Skip to content

Fixes #163: Ensure changelog records for non-branching models are created in main schema #164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 4 additions & 19 deletions netbox_branching/__init__.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
from django.conf import settings
from django.core.exceptions import ImproperlyConfigured

from netbox.plugins import PluginConfig, get_plugin_config
from netbox.registry import registry
from netbox.plugins import PluginConfig
from .utilities import register_models


class AppConfig(PluginConfig):
Expand Down Expand Up @@ -44,23 +44,8 @@ def ready(self):
"netbox_branching: DATABASE_ROUTERS must contain 'netbox_branching.database.BranchAwareRouter'."
)

# Record all object types which support branching in the NetBox registry
exempt_models = (
*constants.EXEMPT_MODELS,
*get_plugin_config('netbox_branching', 'exempt_models'),
)
branching_models = {}
for app_label, models in registry['model_features']['change_logging'].items():
# Wildcard exclusion for all models in this app
if f'{app_label}.*' in exempt_models:
continue
models = [
model for model in models
if f'{app_label}.{model}' not in exempt_models
]
if models:
branching_models[app_label] = models
registry['model_features']['branching'] = branching_models
# Register models which support branching
register_models()


config = AppConfig
8 changes: 4 additions & 4 deletions netbox_branching/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@
# URL query parameter name
QUERY_PARAM = '_branch'

# Tables which must be replicated within a branch even though their
# models don't directly support branching.
REPLICATE_TABLES = (
'dcim_cablepath',
# Models which do not support change logging, but whose database tables
# must be replicated for each branch to ensure proper functionality
INCLUDE_MODELS = (
'dcim.cablepath',
)

# Models for which branching support is explicitly disabled
Expand Down
5 changes: 5 additions & 0 deletions netbox_branching/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ def _get_db(self, model, **hints):
warnings.warn(f"Routing database query for {model} before branching support is initialized.")
return

# Bail if the model does not support branching
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for this PR, but might be nice in the future to make this a utility function: model_supports_branching - I could see it might be useful in the future to potentially check elsewhere and it isolates the logic in a nice place / makes it more readable.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. and it would definitely be useful if a plugin could declare its models branching-aware (or not). That way I could avoid having to tell users it's either branching or NetBox DNS at the moment.

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}'
Expand Down
38 changes: 36 additions & 2 deletions netbox_branching/utilities.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import datetime
import logging
from collections import defaultdict
from contextlib import contextmanager
from dataclasses import dataclass

from django.db.models import ForeignKey, ManyToManyField
from django.urls import reverse

from .constants import REPLICATE_TABLES
from netbox.plugins import get_plugin_config
from netbox.registry import registry
from .constants import EXEMPT_MODELS, INCLUDE_MODELS
from .contextvars import active_branch

__all__ = (
Expand All @@ -19,6 +22,7 @@
'get_tables_to_replicate',
'is_api_request',
'record_applied_change',
'register_models',
'update_object',
)

Expand Down Expand Up @@ -80,11 +84,41 @@ def get_branchable_object_types():
return ObjectType.objects.with_feature('branching')


def register_models():
"""
Register all models which support branching in the NetBox registry.
"""
# Compile a list of exempt models (those for which change logging may
# be enabled, but branching is not supported)
exempt_models = (
*EXEMPT_MODELS,
*get_plugin_config('netbox_branching', 'exempt_models'),
)

# Register all models which support change logging and are not exempt
branching_models = defaultdict(list)
for app_label, models in registry['model_features']['change_logging'].items():
# Wildcard exclusion for all models in this app
if f'{app_label}.*' in exempt_models:
continue
for model in models:
if f'{app_label}.{model}' not in exempt_models:
branching_models[app_label].append(model)

# Register additional included models
# TODO: Allow plugins to declare additional models?
for label in INCLUDE_MODELS:
app_label, model = label.split('.')
branching_models[app_label].append(model)

registry['model_features']['branching'] = dict(branching_models)


def get_tables_to_replicate():
"""
Return an ordered list of database tables to replicate when provisioning a new schema.
"""
tables = set(REPLICATE_TABLES)
tables = set()

branch_aware_models = [
ot.model_class() for ot in get_branchable_object_types()
Expand Down