From c85ea5cfeb3c3eb7ff0a64c6a3c2b6c687525c38 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 12 Aug 2024 15:05:38 -0400 Subject: [PATCH 01/26] Release v0.3.0 --- netbox_branching/__init__.py | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/netbox_branching/__init__.py b/netbox_branching/__init__.py index 22902f3..fc37369 100644 --- a/netbox_branching/__init__.py +++ b/netbox_branching/__init__.py @@ -5,7 +5,7 @@ class AppConfig(PluginConfig): name = 'netbox_branching' verbose_name = 'NetBox Branching' description = 'A git-like branching implementation for NetBox' - version = '0.2.1' + version = '0.3.0' base_url = 'branching' min_version = '4.1' middleware = [ diff --git a/pyproject.toml b/pyproject.toml index 610b4a8..4c27b61 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "netboxlabs-netbox-branching" -version = "0.2.1" +version = "0.3.0" description = "A git-like branching implementation for NetBox" readme = "README.md" requires-python = ">=3.10" From a7d93d3ea439721ec4e50aa6e05d528807a0043a Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Thu, 15 Aug 2024 13:55:48 -0400 Subject: [PATCH 02/26] Add PR template --- .github/PULL_REQUEST_TEMPLATE.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .github/PULL_REQUEST_TEMPLATE.md diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 0000000..3443891 --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,9 @@ + +### Fixes: #1234 + + From e7ed80105b255e5379c5b96825a513829dc2372b Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Thu, 15 Aug 2024 14:59:13 -0400 Subject: [PATCH 03/26] Update GitHub issue labels --- .github/ISSUE_TEMPLATE/01-feature_request.yaml | 2 +- .github/ISSUE_TEMPLATE/02-bug_report.yaml | 2 +- .github/ISSUE_TEMPLATE/03-documentation_change.yaml | 2 +- .github/ISSUE_TEMPLATE/04-housekeeping.yaml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/01-feature_request.yaml b/.github/ISSUE_TEMPLATE/01-feature_request.yaml index 0c9ae7a..6c594b7 100644 --- a/.github/ISSUE_TEMPLATE/01-feature_request.yaml +++ b/.github/ISSUE_TEMPLATE/01-feature_request.yaml @@ -1,7 +1,7 @@ --- name: ✨ Feature Request description: Propose a new NetBox feature or enhancement -labels: ["feature"] +labels: ["type: feature"] body: - type: input attributes: diff --git a/.github/ISSUE_TEMPLATE/02-bug_report.yaml b/.github/ISSUE_TEMPLATE/02-bug_report.yaml index cdb3bc9..a085f62 100644 --- a/.github/ISSUE_TEMPLATE/02-bug_report.yaml +++ b/.github/ISSUE_TEMPLATE/02-bug_report.yaml @@ -1,7 +1,7 @@ --- name: 🐛 Bug Report description: Report a reproducible bug in the current stable release -labels: ["bug"] +labels: ["type: bug"] body: - type: input attributes: diff --git a/.github/ISSUE_TEMPLATE/03-documentation_change.yaml b/.github/ISSUE_TEMPLATE/03-documentation_change.yaml index bc9d983..9f7968e 100644 --- a/.github/ISSUE_TEMPLATE/03-documentation_change.yaml +++ b/.github/ISSUE_TEMPLATE/03-documentation_change.yaml @@ -1,7 +1,7 @@ --- name: 📖 Documentation Change description: Suggest an addition or modification to the plugin's documentation -labels: ["documentation"] +labels: ["type: documentation"] body: - type: dropdown attributes: diff --git a/.github/ISSUE_TEMPLATE/04-housekeeping.yaml b/.github/ISSUE_TEMPLATE/04-housekeeping.yaml index 1fb7a1e..cec1500 100644 --- a/.github/ISSUE_TEMPLATE/04-housekeeping.yaml +++ b/.github/ISSUE_TEMPLATE/04-housekeeping.yaml @@ -1,7 +1,7 @@ --- name: 🏡 Housekeeping description: A change pertaining to the codebase itself (developers only) -labels: ["housekeeping"] +labels: ["type: housekeeping"] body: - type: textarea attributes: From 61bc9420a3df59d9384618e543fe0cb280f05b6e Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Thu, 15 Aug 2024 13:45:48 -0400 Subject: [PATCH 04/26] Fixes #42: Fix exception raised when viewing custom scripts --- netbox_branching/template_content.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/netbox_branching/template_content.py b/netbox_branching/template_content.py index edb3a4a..2d2c154 100644 --- a/netbox_branching/template_content.py +++ b/netbox_branching/template_content.py @@ -24,7 +24,8 @@ def navbar(self): class BranchNotification(PluginTemplateExtension): def alerts(self): - instance = self.context['object'] + if not (instance := self.context['object']): + return '' ct = ContentType.objects.get_for_model(instance) relevant_changes = ChangeDiff.objects.filter( object_type=ct, From af99c928d930e206f63103e016d5a92741f99025 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Thu, 15 Aug 2024 14:39:39 -0400 Subject: [PATCH 05/26] Fixes #44: Handle truncated sequence names to avoid exception during branch provisioning --- netbox_branching/models/branches.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/netbox_branching/models/branches.py b/netbox_branching/models/branches.py index 34105d4..c4b4d65 100644 --- a/netbox_branching/models/branches.py +++ b/netbox_branching/models/branches.py @@ -434,9 +434,14 @@ def provision(self, user): cursor.execute( f"INSERT INTO {schema}.{table} SELECT * FROM public.{table}" ) + # Get the name of the sequence used for object ID allocations + cursor.execute( + f"SELECT pg_get_serial_sequence('{table}', 'id');" + ) + sequence_name = cursor.fetchone()[0] # Set the default value for the ID column to the sequence associated with the source table cursor.execute( - f"ALTER TABLE {schema}.{table} ALTER COLUMN id SET DEFAULT nextval('public.{table}_id_seq')" + f"ALTER TABLE {schema}.{table} ALTER COLUMN id SET DEFAULT nextval('{sequence_name}')" ) # Commit the transaction From d8fad3ac61bc6b24af9045664b8c6041e0ecd796 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Thu, 15 Aug 2024 16:49:30 -0400 Subject: [PATCH 06/26] Fixes #48: Roll back the transaction before re-raising any exceptions during branch provisioning --- netbox_branching/models/branches.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/netbox_branching/models/branches.py b/netbox_branching/models/branches.py index c4b4d65..0dbbda2 100644 --- a/netbox_branching/models/branches.py +++ b/netbox_branching/models/branches.py @@ -388,8 +388,8 @@ def provision(self, user): # Update Branch status Branch.objects.filter(pk=self.pk).update(status=BranchStatusChoices.PROVISIONING) - try: - with connection.cursor() as cursor: + with connection.cursor() as cursor: + try: schema = self.schema_name # Start a transaction @@ -447,10 +447,15 @@ def provision(self, user): # Commit the transaction cursor.execute("COMMIT") - except Exception as e: - logger.error(e) - Branch.objects.filter(pk=self.pk).update(status=BranchStatusChoices.FAILED) - raise e + except Exception as e: + # Abort the transaction + cursor.execute("ROLLBACK") + + # Mark the Branch as failed + logger.error(e) + Branch.objects.filter(pk=self.pk).update(status=BranchStatusChoices.FAILED) + + raise e # Emit branch_provisioned signal branch_provisioned.send(sender=self.__class__, branch=self, user=user) From 2dfcb5d345fd93449064d37e81553dc562551f0b Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Fri, 16 Aug 2024 15:01:23 -0400 Subject: [PATCH 07/26] Remove ability to publish to test PyPI --- .github/workflows/release.yaml | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 30a49a8..fe11654 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -1,15 +1,5 @@ name: Release on: - workflow_dispatch: - inputs: - pypi: - description: 'PyPI instance' - required: true - type: choice - default: 'production' - options: - - production - - testing push: branches: [ release ] @@ -20,8 +10,6 @@ concurrency: env: PYTHON_RUNTIME_VERSION: "3.11" PYTHON_PACKAGE_NAME: netboxlabs-netbox-branching - PYPI_URL_PRODUCTION: "https://upload.pypi.org/legacy/" - PYPI_URL_TESTING: "https://test.pypi.org/legacy/" jobs: get-package-name: @@ -106,12 +94,10 @@ jobs: - name: Publish release distributions to PyPI uses: pypa/gh-action-pypi-publish@release/v1 with: - repository-url: ${{ github.event.inputs.pypi == 'production' && env.PYPI_URL_PRODUCTION || env.PYPI_URL_TESTING }} packages-dir: dist release: name: Release needs: [ get-next-version, get-release-notes, build ] - if: github.event.inputs.pypi == 'production' runs-on: ubuntu-latest timeout-minutes: 5 steps: From 792da7fbba55a3bd659f9405f012c6204a125b61 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Fri, 16 Aug 2024 15:29:22 -0400 Subject: [PATCH 08/26] Fixes #50: Branch state should remain 'merged' after dry-run revert --- netbox_branching/models/branches.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netbox_branching/models/branches.py b/netbox_branching/models/branches.py index 0dbbda2..4fd8146 100644 --- a/netbox_branching/models/branches.py +++ b/netbox_branching/models/branches.py @@ -358,7 +358,7 @@ def revert(self, user, commit=True): logger.error(e) # Disconnect signal receiver & restore original branch status post_save.disconnect(handler, sender=ObjectChange_) - Branch.objects.filter(pk=self.pk).update(status=BranchStatusChoices.READY) + Branch.objects.filter(pk=self.pk).update(status=BranchStatusChoices.MERGED) raise e # Update the Branch's status to "ready" From c16d68fefc5d3e55d44f4e9a5b7a98e1836e2164 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Fri, 16 Aug 2024 15:41:39 -0400 Subject: [PATCH 09/26] Release v0.3.1 --- docs/changelog.md | 13 +++++++++++++ netbox_branching/__init__.py | 2 +- pyproject.toml | 2 +- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index a1de8e4..cea51f1 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -1,5 +1,16 @@ # Change Log +## v0.3.1 + +### Bug Fixes + +* [#42](https://github.com/netboxlabs/nbl-netbox-branching/issues/42) - Fix exception raised when viewing custom scripts +* [#44](https://github.com/netboxlabs/nbl-netbox-branching/issues/44) - Handle truncated SQL sequence names to avoid exceptions during branch provisioning +* [#48](https://github.com/netboxlabs/nbl-netbox-branching/issues/48) - Ensure background job is terminated in the event branch provisioning errors +* [#50](https://github.com/netboxlabs/nbl-netbox-branching/issues/50) - Branch state should remain as "merged" after dry-run revert + +--- + ## v0.3.0 ### Enhancements @@ -22,6 +33,8 @@ * [#30](https://github.com/netboxlabs/nbl-netbox-branching/issues/30) - Include only unmerged branches with relevant changes in object view notifications * [#31](https://github.com/netboxlabs/nbl-netbox-branching/issues/31) - Prevent the deletion of a branch in a transitional state +--- + ## v0.2.0 * Initial private release diff --git a/netbox_branching/__init__.py b/netbox_branching/__init__.py index fc37369..ba35dd7 100644 --- a/netbox_branching/__init__.py +++ b/netbox_branching/__init__.py @@ -5,7 +5,7 @@ class AppConfig(PluginConfig): name = 'netbox_branching' verbose_name = 'NetBox Branching' description = 'A git-like branching implementation for NetBox' - version = '0.3.0' + version = '0.3.1' base_url = 'branching' min_version = '4.1' middleware = [ diff --git a/pyproject.toml b/pyproject.toml index 4c27b61..a858bf6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "netboxlabs-netbox-branching" -version = "0.3.0" +version = "0.3.1" description = "A git-like branching implementation for NetBox" readme = "README.md" requires-python = ">=3.10" From ade37d175fe3fd9bf2244660d85623fc17bdbe35 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Fri, 16 Aug 2024 16:27:50 -0400 Subject: [PATCH 10/26] Closes #47: Clean up raw SQL queries & sanitize arguments to nextval() --- netbox_branching/models/branches.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/netbox_branching/models/branches.py b/netbox_branching/models/branches.py index 4fd8146..acabff5 100644 --- a/netbox_branching/models/branches.py +++ b/netbox_branching/models/branches.py @@ -413,35 +413,39 @@ def provision(self, user): # Create an empty copy of the global change log. Share the ID sequence from the main table to avoid # reusing change record IDs. table = ObjectChange_._meta.db_table - logger.debug(f'Creating table {schema}.{table}') + main_table = f'public.{table}' + schema_table = f'{schema}.{table}' + logger.debug(f'Creating table {schema_table}') cursor.execute( - f"CREATE TABLE {schema}.{table} ( LIKE public.{table} INCLUDING INDEXES )" + f"CREATE TABLE {schema_table} ( LIKE {main_table} INCLUDING INDEXES )" ) # Set the default value for the ID column to the sequence associated with the source table + sequence_name = f'public.{table}_id_seq' cursor.execute( - f"ALTER TABLE {schema}.{table} " - f"ALTER COLUMN id SET DEFAULT nextval('public.{table}_id_seq')" + f"ALTER TABLE {schema_table} ALTER COLUMN id SET DEFAULT nextval(%s)", [sequence_name] ) # Replicate relevant tables from the main schema for table in get_tables_to_replicate(): - logger.debug(f'Creating table {schema}.{table}') + main_table = f'public.{table}' + schema_table = f'{schema}.{table}' + logger.debug(f'Creating table {schema_table}') # Create the table in the new schema cursor.execute( - f"CREATE TABLE {schema}.{table} ( LIKE public.{table} INCLUDING INDEXES )" + f"CREATE TABLE {schema_table} ( LIKE {main_table} INCLUDING INDEXES )" ) # Copy data from the source table cursor.execute( - f"INSERT INTO {schema}.{table} SELECT * FROM public.{table}" + f"INSERT INTO {schema_table} SELECT * FROM {main_table}" ) # Get the name of the sequence used for object ID allocations cursor.execute( - f"SELECT pg_get_serial_sequence('{table}', 'id');" + "SELECT pg_get_serial_sequence(%s, 'id')", [table] ) sequence_name = cursor.fetchone()[0] # Set the default value for the ID column to the sequence associated with the source table cursor.execute( - f"ALTER TABLE {schema}.{table} ALTER COLUMN id SET DEFAULT nextval('{sequence_name}')" + f"ALTER TABLE {schema_table} ALTER COLUMN id SET DEFAULT nextval(%s)", [sequence_name] ) # Commit the transaction From eb783f51d9d6ef95eeb071879d59841cea09faba Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 2 Sep 2024 09:38:31 -0400 Subject: [PATCH 11/26] Adjust migration dependencies for NetBox v4.1.0 --- netbox_branching/migrations/0001_initial.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netbox_branching/migrations/0001_initial.py b/netbox_branching/migrations/0001_initial.py index 47fee29..e40b455 100644 --- a/netbox_branching/migrations/0001_initial.py +++ b/netbox_branching/migrations/0001_initial.py @@ -13,7 +13,7 @@ class Migration(migrations.Migration): dependencies = [ ('contenttypes', '0002_remove_content_type_name'), ('core', '0011_move_objectchange'), - ('extras', '0118_notifications'), + ('extras', '0119_notifications'), migrations.swappable_dependency(settings.AUTH_USER_MODEL), ] From d1ed4c33c606ad278a0219843ee3036dc00e6849 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Tue, 20 Aug 2024 15:56:39 -0400 Subject: [PATCH 12/26] Fixes #57: Record ChangeDiffs only for supported object types --- netbox_branching/__init__.py | 10 +++++++++- netbox_branching/constants.py | 6 ++++++ netbox_branching/signal_receivers.py | 5 +++++ netbox_branching/utilities.py | 5 ++--- 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/netbox_branching/__init__.py b/netbox_branching/__init__.py index ba35dd7..482872a 100644 --- a/netbox_branching/__init__.py +++ b/netbox_branching/__init__.py @@ -1,4 +1,5 @@ from netbox.plugins import PluginConfig +from netbox.registry import registry class AppConfig(PluginConfig): @@ -18,7 +19,14 @@ class AppConfig(PluginConfig): def ready(self): super().ready() - from . import events, search, signal_receivers + from . import constants, events, search, signal_receivers + + # Record all object types which support branching in the NetBox registry + if 'branching' not in registry['model_features']: + registry['model_features']['branching'] = { + k: v for k, v in registry['model_features']['change_logging'].items() + if k not in constants.EXCLUDED_APPS + } config = AppConfig diff --git a/netbox_branching/constants.py b/netbox_branching/constants.py index 853337a..c7743b5 100644 --- a/netbox_branching/constants.py +++ b/netbox_branching/constants.py @@ -9,3 +9,9 @@ # URL query parameter name QUERY_PARAM = '_branch' + +# Apps which are explicitly excluded from branching +EXCLUDED_APPS = ( + 'netbox_branching', + 'netbox_changes', +) diff --git a/netbox_branching/signal_receivers.py b/netbox_branching/signal_receivers.py index 08e460e..702a06b 100644 --- a/netbox_branching/signal_receivers.py +++ b/netbox_branching/signal_receivers.py @@ -10,6 +10,7 @@ from core.models import ObjectChange, ObjectType from extras.events import process_event_rules from extras.models import EventRule +from netbox.registry import registry from utilities.exceptions import AbortRequest from utilities.serialization import serialize_object from .choices import BranchStatusChoices @@ -34,6 +35,10 @@ def record_change_diff(instance, **kwargs): object_type = instance.changed_object_type object_id = instance.changed_object_id + # If this type of object does not support branching, return immediately. + if object_type.model not in registry['model_features']['branching'].get(object_type.app_label, []): + return + # If this is a global change, update the "current" state in any ChangeDiffs for this object. if branch is None: diff --git a/netbox_branching/utilities.py b/netbox_branching/utilities.py index 9a29848..539573b 100644 --- a/netbox_branching/utilities.py +++ b/netbox_branching/utilities.py @@ -73,9 +73,8 @@ def get_branchable_object_types(): Return all object types which are branch-aware; i.e. those which support change logging. """ from core.models import ObjectType - return ObjectType.objects.with_feature('change_logging').exclude( - app_label__in=['netbox_branching', 'netbox_changes'] - ) + + return ObjectType.objects.with_feature('branching') def get_tables_to_replicate(): From ea5ea272444b4b44130129cbc8b48805a6a5a2b0 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Tue, 3 Sep 2024 15:43:27 -0400 Subject: [PATCH 13/26] Closes #52: Introduce the `max_branches` config parameter (#63) * Closes #52: Introduce the max_branches config parameter * Improve documentation for config parameter --- docs/configuration.md | 17 +++++++++++++++++ mkdocs.yml | 1 + netbox_branching/__init__.py | 3 +++ netbox_branching/models/branches.py | 14 ++++++++++++++ netbox_branching/tests/test_branches.py | 21 ++++++++++++++++++++- 5 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 docs/configuration.md diff --git a/docs/configuration.md b/docs/configuration.md new file mode 100644 index 0000000..6e141aa --- /dev/null +++ b/docs/configuration.md @@ -0,0 +1,17 @@ +# Configuration Parameters + +## `max_branches` + +Default: None + +The maximum number of branches that can exist simultaneously, including merged branches that have not been deleted. It may be desirable to limit the total number of provisioned branches to safeguard against excessive database size. + +--- + +## `schema_prefix` + +Default: `branch_` + +The string to prefix to the unique branch ID when provisioning the PostgreSQL schema for a branch. Per [the PostgreSQL documentation](https://www.postgresql.org/docs/16/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS), this string must begin with a letter or underscore. + +Note that a valid prefix is required, as the randomly-generated branch ID alone may begin with a digit, which would not qualify as a valid schema name. diff --git a/mkdocs.yml b/mkdocs.yml index 3cc4368..5697beb 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -43,6 +43,7 @@ nav: - Syncing & Merging: 'using-branches/syncing-merging.md' - Reverting a Branch: 'using-branches/reverting-a-branch.md' - REST API: 'rest-api.md' + - Configuration: 'configuration.md' - Data Model: - Branch: 'models/branch.md' - BranchEvent: 'models/branchevent.md' diff --git a/netbox_branching/__init__.py b/netbox_branching/__init__.py index 482872a..e4261e4 100644 --- a/netbox_branching/__init__.py +++ b/netbox_branching/__init__.py @@ -13,6 +13,9 @@ class AppConfig(PluginConfig): 'netbox_branching.middleware.BranchMiddleware' ] default_settings = { + # The maximum number of branches which can be provisioned simultaneously + 'max_branches': None, + # This string is prefixed to the name of each new branch schema during provisioning 'schema_prefix': 'branch_', } diff --git a/netbox_branching/models/branches.py b/netbox_branching/models/branches.py index acabff5..c119581 100644 --- a/netbox_branching/models/branches.py +++ b/netbox_branching/models/branches.py @@ -5,6 +5,7 @@ from django.conf import settings from django.contrib.auth import get_user_model +from django.core.exceptions import ValidationError from django.db import DEFAULT_DB_ALIAS, connection, models, transaction from django.db.models.signals import post_save from django.db.utils import ProgrammingError @@ -124,6 +125,19 @@ def connection_name(self): def synced_time(self): return self.last_sync or self.created + def clean(self): + + # Check whether we're exceeding the maximum number of Branches + if not self.pk and (max_branches := get_plugin_config('netbox_branching', 'max_branches')): + branch_count = Branch.objects.count() + if branch_count >= max_branches: + raise ValidationError( + _( + "The configured maximum number of branches ({max}) cannot be exceeded. One or more existing " + "branches must be deleted before a new branch may be created." + ).format(max=max_branches) + ) + def save(self, provision=True, *args, **kwargs): """ Args: diff --git a/netbox_branching/tests/test_branches.py b/netbox_branching/tests/test_branches.py index bc6ad85..efcfe28 100644 --- a/netbox_branching/tests/test_branches.py +++ b/netbox_branching/tests/test_branches.py @@ -1,7 +1,8 @@ import re +from django.core.exceptions import ValidationError from django.db import connection -from django.test import TransactionTestCase +from django.test import TransactionTestCase, override_settings from netbox_branching.constants import MAIN_SCHEMA from netbox_branching.models import Branch @@ -75,3 +76,21 @@ def test_branch_schema_id(self): branch.save(provision=False) branch.refresh_from_db() self.assertEqual(branch.schema_id, schema_id, msg="Schema ID was changed during save()") + + @override_settings(PLUGINS_CONFIG={ + 'netbox_branching': { + 'max_branches': 2, + } + }) + def text_max_branches(self): + """ + Verify that the max_branches config parameter is enforced. + """ + Branch.objects.bulk_create(( + Branch(name='Branch 1'), + Branch(name='Branch 2'), + )) + + branch = Branch(name='Branch 3') + with self.assertRaises(ValidationError): + branch.full_clean() From 6644bbb2586314d3ab1bd9f333665a7dc92c13e3 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 2 Sep 2024 13:17:18 -0400 Subject: [PATCH 14/26] Fixes #59: BranchAwareRouter should disregard models which do not support branching --- netbox_branching/database.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/netbox_branching/database.py b/netbox_branching/database.py index c3f5aea..161c6e7 100644 --- a/netbox_branching/database.py +++ b/netbox_branching/database.py @@ -1,3 +1,5 @@ +from netbox.registry import registry + from .contextvars import active_branch @@ -11,15 +13,21 @@ class BranchAwareRouter: A Django database router that returns the appropriate connection/schema for the active branch (if any). """ - def db_for_read(self, model, **hints): + def _get_db(self, model, **hints): + # Bail if the model does not support branching + app_label, model_name = model._meta.label.lower().split('.') + if model_name not in registry['model_features']['branching'].get(app_label, []): + return + + # Return the schema for the active branch (if any) if branch := active_branch.get(): return f'schema_{branch.schema_name}' - return None + + def db_for_read(self, model, **hints): + return self._get_db(model, **hints) def db_for_write(self, model, **hints): - if branch := active_branch.get(): - return f'schema_{branch.schema_name}' - return None + return self._get_db(model, **hints) def allow_relation(self, obj1, obj2, **hints): # Permit relations from the branch schema to the main (public) schema From 2393105e8bab981553d5a021321cac7f768962d2 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Tue, 3 Sep 2024 15:50:49 -0400 Subject: [PATCH 15/26] Update changelog --- docs/changelog.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/changelog.md b/docs/changelog.md index cea51f1..017e28b 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -1,5 +1,18 @@ # Change Log +## FUTURE + +### Enhancements + +* [#52](https://github.com/netboxlabs/nbl-netbox-branching/issues/52) - Introduce the `max_branches` config parameter + +### Bug Fixes + +* [#57](https://github.com/netboxlabs/nbl-netbox-branching/issues/57) - Avoid recording ChangeDiff records for unsupported object types +* [#59](https://github.com/netboxlabs/nbl-netbox-branching/issues/59) - `BranchAwareRouter` should consider branching support for model when determining database connection to use + +--- + ## v0.3.1 ### Bug Fixes From 2b58427f784ab7a4f050a328d580cea2801c022a Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Tue, 3 Sep 2024 16:05:11 -0400 Subject: [PATCH 16/26] Show schema ID under branch details --- netbox_branching/templates/netbox_branching/branch.html | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/netbox_branching/templates/netbox_branching/branch.html b/netbox_branching/templates/netbox_branching/branch.html index ea73310..5d8d6b1 100644 --- a/netbox_branching/templates/netbox_branching/branch.html +++ b/netbox_branching/templates/netbox_branching/branch.html @@ -73,6 +73,10 @@
{% trans "Branch" %}
{% trans "Name" %} {{ object.name }} + + {% trans "Schema ID" %} + {{ object.schema_id }} + {% trans "Status" %} @@ -123,7 +127,7 @@
{% trans "Branch" %}
- {% trans "Schema" %} + {% trans "Database schema" %} {{ object.schema_name }} From 872e1127fb9d95d66d2149769bb8dc1496f41c87 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Tue, 3 Sep 2024 16:35:10 -0400 Subject: [PATCH 17/26] Fixes #61: Fix transaction rollback when performing a dry run sync --- netbox_branching/models/branches.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netbox_branching/models/branches.py b/netbox_branching/models/branches.py index c119581..13dde35 100644 --- a/netbox_branching/models/branches.py +++ b/netbox_branching/models/branches.py @@ -249,7 +249,7 @@ def sync(self, user, commit=True): # Apply each change from the main schema for change in self.get_unsynced_changes().order_by('time'): logger.debug(f'Applying change: {change}') - change.apply(using=self.connection_name) + change.apply() if not commit: raise AbortTransaction() From e9cefe674c49dad50aee0bcfd406ba351c4cb437 Mon Sep 17 00:00:00 2001 From: Michal Fiedorowicz Date: Wed, 4 Sep 2024 13:32:04 +0100 Subject: [PATCH 18/26] Add GHA workflow to run lint and tests (#27) * add pyproject.toml Signed-off-by: Michal Fiedorowicz * add docker-compose setup based on NetBox v4.1-beta1-2.9.1 image with netbox_branching plugin installed Signed-off-by: Michal Fiedorowicz * add Makefile recipes for docker compose Signed-off-by: Michal Fiedorowicz * add GHA workflow to run lint and test Signed-off-by: Michal Fiedorowicz * tidy up docker-compose.yaml Signed-off-by: Michal Fiedorowicz * move workflow into correct directory Signed-off-by: Michal Fiedorowicz * fix mounting of local_settings.py Signed-off-by: Michal Fiedorowicz * ignore missing docstrings checks Signed-off-by: Michal Fiedorowicz * add step for building documentation Signed-off-by: Michal Fiedorowicz * catch make docker-compose-test exit code (test) Signed-off-by: Michal Fiedorowicz * catch make docker-compose-test exit code (test 2) Signed-off-by: Michal Fiedorowicz * tidy up make docker-compose-test Signed-off-by: Michal Fiedorowicz * rename GHA job Signed-off-by: Michal Fiedorowicz * ignore all missing docstrings Signed-off-by: Michal Fiedorowicz * change workflow file ext Signed-off-by: Michal Fiedorowicz * run tests with more native way with matrix of python versions Signed-off-by: Michal Fiedorowicz * fix matrix with python version Signed-off-by: Michal Fiedorowicz * fix test command for plugin Signed-off-by: Michal Fiedorowicz * list tests Signed-off-by: Michal Fiedorowicz * comment out exclude-package-data from pyproject.toml Signed-off-by: Michal Fiedorowicz * tidy up Signed-off-by: Michal Fiedorowicz * keep db when running tests Signed-off-by: Michal Fiedorowicz * Remove v4.1-beta1 tag --------- Signed-off-by: Michal Fiedorowicz Co-authored-by: Jeremy Stretch --- .github/workflows/lint-tests.yaml | 97 +++++++++++++++++++++++++++++++ testing/configuration.py | 38 ++++++++++++ testing/local_settings.py | 12 ++++ 3 files changed, 147 insertions(+) create mode 100644 .github/workflows/lint-tests.yaml create mode 100644 testing/configuration.py create mode 100644 testing/local_settings.py diff --git a/.github/workflows/lint-tests.yaml b/.github/workflows/lint-tests.yaml new file mode 100644 index 0000000..fcedae3 --- /dev/null +++ b/.github/workflows/lint-tests.yaml @@ -0,0 +1,97 @@ +name: Lint and tests +on: + workflow_dispatch: + pull_request: + push: + branches: + - "!release" + +concurrency: + group: ${{ github.workflow }} + cancel-in-progress: false + +permissions: + contents: write + checks: write + pull-requests: write + +jobs: + lint: + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Checkout + uses: actions/checkout@v4 + - name: Setup Python + uses: actions/setup-python@v5 + with: + python-version: "3.10" + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install . + pip install .[dev] + pip install .[test] + - name: Build documentation + run: mkdocs build + - name: Lint with Ruff + run: | + ruff check --output-format=github netbox_branching/ + continue-on-error: true + tests: + runs-on: ubuntu-latest + timeout-minutes: 10 + strategy: + matrix: + python-version: [ "3.10", "3.11", "3.12" ] + services: + redis: + image: redis + ports: + - 6379:6379 + postgres: + image: postgres + env: + POSTGRES_USER: netbox + POSTGRES_PASSWORD: netbox + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + ports: + - 5432:5432 + steps: + - name: Checkout netbox-branching + uses: actions/checkout@v4 + with: + path: netbox-branching + - name: Setup Python ${{ matrix.python-version }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + - name: Checkout netbox + uses: actions/checkout@v4 + with: + repository: "netbox-community/netbox" + path: netbox + - name: Install netbox-branching + working-directory: netbox-branching + run: | + # Include tests directory for test + sed -i 's/exclude-package-data/#exclude-package-data/g' pyproject.toml + python -m pip install --upgrade pip + pip install . + pip install .[test] + - name: Install dependencies & configure plugin + working-directory: netbox + run: | + ln -s $(pwd)/../netbox-branching/testing/configuration.py netbox/netbox/configuration.py + ln -s $(pwd)/../netbox-branching/testing/local_settings.py netbox/netbox/local_settings.py + + python -m pip install --upgrade pip + pip install -r requirements.txt -U + - name: Run tests + working-directory: netbox + run: | + python netbox/manage.py test netbox_branching.tests --keepdb diff --git a/testing/configuration.py b/testing/configuration.py new file mode 100644 index 0000000..2721e48 --- /dev/null +++ b/testing/configuration.py @@ -0,0 +1,38 @@ +################################################################### +# This file serves as a base configuration for testing purposes # +# only. It is not intended for production use. # +################################################################### + +ALLOWED_HOSTS = ["*"] + +DATABASE = { + "NAME": "netbox", + "USER": "netbox", + "PASSWORD": "netbox", + "HOST": "localhost", + "PORT": "", + "CONN_MAX_AGE": 300, +} + +PLUGINS = [ + "netbox_branching", +] + +REDIS = { + "tasks": { + "HOST": "localhost", + "PORT": 6379, + "PASSWORD": "", + "DATABASE": 0, + "SSL": False, + }, + "caching": { + "HOST": "localhost", + "PORT": 6379, + "PASSWORD": "", + "DATABASE": 1, + "SSL": False, + }, +} + +SECRET_KEY = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" diff --git a/testing/local_settings.py b/testing/local_settings.py new file mode 100644 index 0000000..10de74b --- /dev/null +++ b/testing/local_settings.py @@ -0,0 +1,12 @@ +from netbox_branching.utilities import DynamicSchemaDict +from .configuration import DATABASE + +# Wrap DATABASES with DynamicSchemaDict for dynamic schema support +DATABASES = DynamicSchemaDict({ + 'default': DATABASE, +}) + +# Employ our custom database router +DATABASE_ROUTERS = [ + 'netbox_branching.database.BranchAwareRouter', +] From f6db858265d6c7dcb614a0eeeeb7a6aea848515c Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 4 Sep 2024 09:26:48 -0400 Subject: [PATCH 19/26] Fixes #66: Capture object representation on ChangeDiff when creating a new object within a branch --- netbox_branching/signal_receivers.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/netbox_branching/signal_receivers.py b/netbox_branching/signal_receivers.py index 702a06b..c3c9b7a 100644 --- a/netbox_branching/signal_receivers.py +++ b/netbox_branching/signal_receivers.py @@ -75,8 +75,7 @@ def record_change_diff(instance, **kwargs): current_data = serialize_object(obj, exclude=['created', 'last_updated']) diff = ChangeDiff( branch=branch, - object_type=instance.changed_object_type, - object_id=instance.changed_object_id, + object=instance.changed_object, action=instance.action, original=instance.prechange_data_clean, modified=instance.postchange_data_clean, From 80362e6e4523df65da54a44c57f1fbfb6d154318 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 4 Sep 2024 13:17:58 -0400 Subject: [PATCH 20/26] #61: Address a regression in the original fix --- netbox_branching/models/branches.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/netbox_branching/models/branches.py b/netbox_branching/models/branches.py index 13dde35..1779157 100644 --- a/netbox_branching/models/branches.py +++ b/netbox_branching/models/branches.py @@ -245,11 +245,11 @@ def sync(self, user, commit=True): try: with activate_branch(self): - with transaction.atomic(): + with transaction.atomic(using=self.connection_name): # Apply each change from the main schema for change in self.get_unsynced_changes().order_by('time'): logger.debug(f'Applying change: {change}') - change.apply() + change.apply(using=self.connection_name) if not commit: raise AbortTransaction() From 73f713443518b62df2ec256561f9493c43d42df0 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 4 Sep 2024 14:53:28 -0400 Subject: [PATCH 21/26] Closes #68: Replace ruff with pycodestyle --- .github/workflows/lint-tests.yaml | 5 ++--- pyproject.toml | 22 +--------------------- 2 files changed, 3 insertions(+), 24 deletions(-) diff --git a/.github/workflows/lint-tests.yaml b/.github/workflows/lint-tests.yaml index fcedae3..5861dc9 100644 --- a/.github/workflows/lint-tests.yaml +++ b/.github/workflows/lint-tests.yaml @@ -34,10 +34,9 @@ jobs: pip install .[test] - name: Build documentation run: mkdocs build - - name: Lint with Ruff + - name: Run pycodestyle run: | - ruff check --output-format=github netbox_branching/ - continue-on-error: true + pycodestyle --ignore=W504,E501 netbox_branching/ tests: runs-on: ubuntu-latest timeout-minutes: 10 diff --git a/pyproject.toml b/pyproject.toml index a858bf6..1fe5351 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,7 +28,7 @@ dependencies = [ ] [project.optional-dependencies] -dev = ["black", "check-manifest", "ruff", "mkdocs", "mkdocs-material"] +dev = ["check-manifest", "mkdocs", "mkdocs-material", "pycodestyle"] test = ["coverage", "pytest", "pytest-cov"] [project.urls] @@ -47,23 +47,3 @@ license-files = ["LICENSE.md"] [build-system] requires = ["setuptools>=43.0.0", "wheel"] build-backend = "setuptools.build_meta" - -[tool.ruff] -line-length = 140 - -[tool.ruff.format] -quote-style = "double" -indent-style = "space" - -[tool.ruff.lint] -select = ["C", "D", "E", "F", "I", "R", "UP", "W"] -ignore = [ - "F401", - "D203", - "D212", - "D400", - "D401", - "D404", - "RET504", - "D1" -] From 62f4203930c288f5939039d3e769db94b8ab9d03 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 4 Sep 2024 14:15:21 -0400 Subject: [PATCH 22/26] Fixes #73: Ensure all relevant ChangeDiffs are updated when an object is modified in main --- netbox_branching/signal_receivers.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/netbox_branching/signal_receivers.py b/netbox_branching/signal_receivers.py index c3c9b7a..aa1247e 100644 --- a/netbox_branching/signal_receivers.py +++ b/netbox_branching/signal_receivers.py @@ -47,10 +47,14 @@ def record_change_diff(instance, **kwargs): return print(f"Updating change diff for global change to {instance.changed_object}") - if diff := ChangeDiff.objects.filter(object_type=object_type, object_id=object_id, branch__status=BranchStatusChoices.READY).first(): - diff.last_updated = timezone.now() - diff.current = instance.postchange_data_clean - diff.save() + ChangeDiff.objects.filter( + object_type=object_type, + object_id=object_id, + branch__status=BranchStatusChoices.READY + ).update( + last_updated=timezone.now(), + current=instance.postchange_data_clean + ) # If this is a branch-aware change, create or update ChangeDiff for this object. else: From 07904b29bf9377d72307d205f9ebc2a0b9412cea Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 4 Sep 2024 13:54:00 -0400 Subject: [PATCH 23/26] Fixes #69: Represent null values for ChangeDiff fields consistently in REST API --- netbox_branching/signal_receivers.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/netbox_branching/signal_receivers.py b/netbox_branching/signal_receivers.py index aa1247e..817493a 100644 --- a/netbox_branching/signal_receivers.py +++ b/netbox_branching/signal_receivers.py @@ -53,7 +53,7 @@ def record_change_diff(instance, **kwargs): branch__status=BranchStatusChoices.READY ).update( last_updated=timezone.now(), - current=instance.postchange_data_clean + current=instance.postchange_data_clean or None ) # If this is a branch-aware change, create or update ChangeDiff for this object. @@ -65,7 +65,7 @@ def record_change_diff(instance, **kwargs): diff.last_updated = timezone.now() if diff.action != ObjectChangeActionChoices.ACTION_CREATE: diff.action = instance.action - diff.modified = instance.postchange_data_clean + diff.modified = instance.postchange_data_clean or None diff.save() # Creating a new ChangeDiff @@ -81,9 +81,9 @@ def record_change_diff(instance, **kwargs): branch=branch, object=instance.changed_object, action=instance.action, - original=instance.prechange_data_clean, - modified=instance.postchange_data_clean, - current=current_data, + original=instance.prechange_data_clean or None, + modified=instance.postchange_data_clean or None, + current=current_data or None, last_updated=timezone.now(), ) diff.save() From 2b6bd5588d91641935fc75ef0626e6f199dbf731 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Thu, 5 Sep 2024 09:32:28 -0400 Subject: [PATCH 24/26] Closes #76: Check for required settings on init --- netbox_branching/__init__.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/netbox_branching/__init__.py b/netbox_branching/__init__.py index e4261e4..835a969 100644 --- a/netbox_branching/__init__.py +++ b/netbox_branching/__init__.py @@ -1,3 +1,6 @@ +from django.conf import settings +from django.core.exceptions import ImproperlyConfigured + from netbox.plugins import PluginConfig from netbox.registry import registry @@ -23,6 +26,17 @@ class AppConfig(PluginConfig): def ready(self): super().ready() from . import constants, events, search, signal_receivers + from .utilities import DynamicSchemaDict + + # Validate required settings + if type(settings.DATABASES) is not DynamicSchemaDict: + raise ImproperlyConfigured( + "netbox_branching: DATABASES must be a DynamicSchemaDict instance." + ) + if 'netbox_branching.database.BranchAwareRouter' not in settings.DATABASE_ROUTERS: + raise ImproperlyConfigured( + "netbox_branching: DATABASE_ROUTERS must contain 'netbox_branching.database.BranchAwareRouter'." + ) # Record all object types which support branching in the NetBox registry if 'branching' not in registry['model_features']: From 54b32363516bc5d5c31c2b455bc14d538171ba2e Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Thu, 5 Sep 2024 09:21:22 -0400 Subject: [PATCH 25/26] Closes #71: Apply logging consistently for all branch operations --- netbox_branching/models/branches.py | 40 +++++++++++++++++++++++----- netbox_branching/models/changes.py | 17 +++++++----- netbox_branching/signal_receivers.py | 16 ++++++++--- 3 files changed, 57 insertions(+), 16 deletions(-) diff --git a/netbox_branching/models/branches.py b/netbox_branching/models/branches.py index 1779157..b337ab1 100644 --- a/netbox_branching/models/branches.py +++ b/netbox_branching/models/branches.py @@ -240,15 +240,22 @@ def sync(self, user, commit=True): if not self.ready: raise Exception(f"Branch {self} is not ready to sync") + # Retrieve unsynced changes before we update the Branch's status + if changes := self.get_unsynced_changes().order_by('time'): + logger.debug(f"Found {len(changes)} changes to sync") + else: + logger.info(f"No changes found; aborting.") + return + # Update Branch status + logger.debug(f"Setting branch status to {BranchStatusChoices.SYNCING}") Branch.objects.filter(pk=self.pk).update(status=BranchStatusChoices.SYNCING) try: with activate_branch(self): with transaction.atomic(using=self.connection_name): # Apply each change from the main schema - for change in self.get_unsynced_changes().order_by('time'): - logger.debug(f'Applying change: {change}') + for change in changes: change.apply(using=self.connection_name) if not commit: raise AbortTransaction() @@ -260,9 +267,13 @@ def sync(self, user, commit=True): raise e # Record the branch's last_synced time & update its status + logger.debug(f"Setting branch status to {BranchStatusChoices.READY}") self.last_sync = timezone.now() self.status = BranchStatusChoices.READY self.save() + + # Record a branch event for the sync + logger.debug(f"Recording branch event: {BranchEventTypeChoices.SYNCED}") BranchEvent.objects.create(branch=self, user=user, type=BranchEventTypeChoices.SYNCED) # Emit branch_synced signal @@ -284,9 +295,14 @@ def merge(self, user, commit=True): raise Exception(f"Branch {self} is not ready to merge") # Retrieve staged changes before we update the Branch's status - changes = self.get_unmerged_changes().order_by('time') + if changes := self.get_unmerged_changes().order_by('time'): + logger.debug(f"Found {len(changes)} changes to merge") + else: + logger.info(f"No changes found; aborting.") + return # Update Branch status + logger.debug(f"Setting branch status to {BranchStatusChoices.MERGING}") Branch.objects.filter(pk=self.pk).update(status=BranchStatusChoices.MERGING) # Create a dummy request for the event_tracking() context manager @@ -301,7 +317,6 @@ def merge(self, user, commit=True): # Apply each change from the Branch for change in changes: with event_tracking(request): - logger.debug(f'Applying change: {change}') request.id = change.request_id request.user = change.user change.apply(using=DEFAULT_DB_ALIAS) @@ -316,10 +331,14 @@ def merge(self, user, commit=True): raise e # Update the Branch's status to "merged" + logger.debug(f"Setting branch status to {BranchStatusChoices.MERGED}") self.status = BranchStatusChoices.MERGED self.merged_time = timezone.now() self.merged_by = user self.save() + + # Record a branch event for the merge + logger.debug(f"Recording branch event: {BranchEventTypeChoices.MERGED}") BranchEvent.objects.create(branch=self, user=user, type=BranchEventTypeChoices.MERGED) # Emit branch_merged signal @@ -344,9 +363,14 @@ def revert(self, user, commit=True): raise Exception(f"Only merged branches can be reverted.") # Retrieve applied changes before we update the Branch's status - changes = self.get_changes().order_by('-time') + if changes := self.get_changes().order_by('-time'): + logger.debug(f"Found {len(changes)} changes to revert") + else: + logger.info(f"No changes found; aborting.") + return # Update Branch status + logger.debug(f"Setting branch status to {BranchStatusChoices.REVERTING}") Branch.objects.filter(pk=self.pk).update(status=BranchStatusChoices.REVERTING) # Create a dummy request for the event_tracking() context manager @@ -361,7 +385,6 @@ def revert(self, user, commit=True): # Undo each change from the Branch for change in changes: with event_tracking(request): - logger.debug(f'Undoing change: {change}') request.id = change.request_id request.user = change.user change.undo() @@ -376,10 +399,14 @@ def revert(self, user, commit=True): raise e # Update the Branch's status to "ready" + logger.debug(f"Setting branch status to {BranchStatusChoices.READY}") self.status = BranchStatusChoices.READY self.merged_time = None self.merged_by = None self.save() + + # Record a branch event for the merge + logger.debug(f"Recording branch event: {BranchEventTypeChoices.REVERTED}") BranchEvent.objects.create(branch=self, user=user, type=BranchEventTypeChoices.REVERTED) # Emit branch_reverted signal @@ -494,6 +521,7 @@ def deprovision(self): with connection.cursor() as cursor: # Delete the schema and all its tables + logger.debug(f'Deleting schema {self.schema_name}') cursor.execute( f"DROP SCHEMA IF EXISTS {self.schema_name} CASCADE" ) diff --git a/netbox_branching/models/changes.py b/netbox_branching/models/changes.py index b8c7a94..8c894db 100644 --- a/netbox_branching/models/changes.py +++ b/netbox_branching/models/changes.py @@ -1,3 +1,4 @@ +import logging from functools import cached_property from django.contrib.contenttypes.fields import GenericForeignKey @@ -30,13 +31,14 @@ def apply(self, using=DEFAULT_DB_ALIAS): """ Apply the change using the specified database connection. """ + logger = logging.getLogger('netbox_branching.models.ObjectChange.apply') model = self.changed_object_type.model_class() - print(f'Applying change {self} using {using}') + logger.debug(f'Applying change {self} using {using}') # Creating a new object if self.action == ObjectChangeActionChoices.ACTION_CREATE: instance = deserialize_object(model, self.postchange_data, pk=self.changed_object_id) - print(f'Creating {model._meta.verbose_name} {instance}') + logger.debug(f'Creating {model._meta.verbose_name} {instance}') instance.object.full_clean() instance.save(using=using) @@ -49,10 +51,10 @@ def apply(self, using=DEFAULT_DB_ALIAS): elif self.action == ObjectChangeActionChoices.ACTION_DELETE: try: instance = model.objects.get(pk=self.changed_object_id) - print(f'Deleting {model._meta.verbose_name} {instance}') + logger.debug(f'Deleting {model._meta.verbose_name} {instance}') instance.delete(using=using) except model.DoesNotExist: - print(f'{model._meta.verbose_name} ID {self.changed_object_id} already deleted; skipping') + logger.debug(f'{model._meta.verbose_name} ID {self.changed_object_id} already deleted; skipping') # Rebuild the MPTT tree where applicable if issubclass(model, MPTTModel): @@ -64,16 +66,17 @@ def undo(self, using=DEFAULT_DB_ALIAS): """ Revert a previously applied change using the specified database connection. """ + logger = logging.getLogger('netbox_branching.models.ObjectChange.undo') model = self.changed_object_type.model_class() # Deleting a previously created object if self.action == ObjectChangeActionChoices.ACTION_CREATE: try: instance = model.objects.get(pk=self.changed_object_id) - print(f'Undoing creation of {model._meta.verbose_name} {instance}') + logger.debug(f'Undoing creation of {model._meta.verbose_name} {instance}') instance.delete(using=using) except model.DoesNotExist: - print(f'{model._meta.verbose_name} ID {self.changed_object_id} does not exist; skipping') + logger.debug(f'{model._meta.verbose_name} ID {self.changed_object_id} does not exist; skipping') # Reverting a modification to an object elif self.action == ObjectChangeActionChoices.ACTION_UPDATE: @@ -83,7 +86,7 @@ def undo(self, using=DEFAULT_DB_ALIAS): # Restoring a deleted object elif self.action == ObjectChangeActionChoices.ACTION_DELETE: instance = deserialize_object(model, self.prechange_data, pk=self.changed_object_id) - print(f'Restoring {model._meta.verbose_name} {instance}') + logger.debug(f'Restoring {model._meta.verbose_name} {instance}') instance.object.full_clean() instance.save(using=using) diff --git a/netbox_branching/signal_receivers.py b/netbox_branching/signal_receivers.py index 817493a..da59578 100644 --- a/netbox_branching/signal_receivers.py +++ b/netbox_branching/signal_receivers.py @@ -1,3 +1,4 @@ +import logging from functools import partial from django.db import DEFAULT_DB_ALIAS @@ -31,6 +32,8 @@ def record_change_diff(instance, **kwargs): """ When an ObjectChange is created, create or update the relevant ChangeDiff for the active Branch. """ + logger = logging.getLogger('netbox_branching.signal_receivers.record_change_diff') + branch = active_branch.get() object_type = instance.changed_object_type object_id = instance.changed_object_id @@ -46,7 +49,7 @@ def record_change_diff(instance, **kwargs): if instance.action == ObjectChangeActionChoices.ACTION_CREATE: return - print(f"Updating change diff for global change to {instance.changed_object}") + logger.debug(f"Updating change diff for global change to {instance.changed_object}") ChangeDiff.objects.filter( object_type=object_type, object_id=object_id, @@ -61,7 +64,7 @@ def record_change_diff(instance, **kwargs): # Updating the existing ChangeDiff if diff := ChangeDiff.objects.filter(object_type=object_type, object_id=object_id, branch=branch).first(): - print(f"Updating branch change diff for change to {instance.changed_object}") + logger.debug(f"Updating branch change diff for change to {instance.changed_object}") diff.last_updated = timezone.now() if diff.action != ObjectChangeActionChoices.ACTION_CREATE: diff.action = instance.action @@ -70,7 +73,7 @@ def record_change_diff(instance, **kwargs): # Creating a new ChangeDiff else: - print(f"Creating branch change diff for change to {instance.changed_object}") + logger.debug(f"Creating branch change diff for change to {instance.changed_object}") if instance.action == ObjectChangeActionChoices.ACTION_CREATE: current_data = None else: @@ -93,6 +96,9 @@ def handle_branch_event(event_type, branch, user=None, **kwargs): """ Process any EventRules associated with branch events (e.g. syncing or merging). """ + logger = logging.getLogger('netbox_branching.signal_receivers.handle_branch_event') + logger.debug(f"Checking for {event_type} event rules") + # Find any EventRules for this event type object_type = ObjectType.objects.get_by_natural_key('netbox_branching', 'branch') event_rules = EventRule.objects.filter( @@ -100,6 +106,10 @@ def handle_branch_event(event_type, branch, user=None, **kwargs): enabled=True, object_types=object_type ) + if not event_rules: + logger.debug("No matching event rules found") + return + logger.debug(f"Found {len(event_rules)} event rules") # Serialize the branch & process EventRules username = user.username if user else None From c1b44e9e894d6d18d156f7b697bb94444634680c Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Thu, 5 Sep 2024 10:48:37 -0400 Subject: [PATCH 26/26] Release v0.4.0 --- docs/changelog.md | 8 +++++++- netbox_branching/__init__.py | 2 +- pyproject.toml | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index 017e28b..79d43f9 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -1,15 +1,21 @@ # Change Log -## FUTURE +## v0.4.0 ### Enhancements * [#52](https://github.com/netboxlabs/nbl-netbox-branching/issues/52) - Introduce the `max_branches` config parameter +* [#71](https://github.com/netboxlabs/nbl-netbox-branching/issues/71) - Ensure the consistent application of logging messages +* [#76](https://github.com/netboxlabs/nbl-netbox-branching/issues/76) - Validate required configuration items on initialization ### Bug Fixes * [#57](https://github.com/netboxlabs/nbl-netbox-branching/issues/57) - Avoid recording ChangeDiff records for unsupported object types * [#59](https://github.com/netboxlabs/nbl-netbox-branching/issues/59) - `BranchAwareRouter` should consider branching support for model when determining database connection to use +* [#61](https://github.com/netboxlabs/nbl-netbox-branching/issues/61) - Fix transaction rollback when performing a dry run sync +* [#66](https://github.com/netboxlabs/nbl-netbox-branching/issues/66) - Capture object representation on ChangeDiff when creating a new object within a branch +* [#69](https://github.com/netboxlabs/nbl-netbox-branching/issues/69) - Represent null values for ChangeDiff fields consistently in REST API +* [#73](https://github.com/netboxlabs/nbl-netbox-branching/issues/73) - Ensure all relevant branch diffs are updated when an object is modified in main --- diff --git a/netbox_branching/__init__.py b/netbox_branching/__init__.py index 835a969..5174856 100644 --- a/netbox_branching/__init__.py +++ b/netbox_branching/__init__.py @@ -9,7 +9,7 @@ class AppConfig(PluginConfig): name = 'netbox_branching' verbose_name = 'NetBox Branching' description = 'A git-like branching implementation for NetBox' - version = '0.3.1' + version = '0.4.0' base_url = 'branching' min_version = '4.1' middleware = [ diff --git a/pyproject.toml b/pyproject.toml index 1fe5351..af09f1c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "netboxlabs-netbox-branching" -version = "0.3.1" +version = "0.4.0" description = "A git-like branching implementation for NetBox" readme = "README.md" requires-python = ">=3.10"