diff --git a/design_builder/context.py b/design_builder/context.py index 7c43bcf8..6df67c8f 100644 --- a/design_builder/context.py +++ b/design_builder/context.py @@ -27,7 +27,7 @@ def __init__(self, root: "_Node"): def _compare(self, subscripts, other): """Compare 'other' to the node's data store.""" for i in subscripts: - value = self.__getitem__(i) + value = self[i] if value != other[i]: return False return True diff --git a/design_builder/contrib/ext.py b/design_builder/contrib/ext.py index 6d7dc239..6ac49062 100644 --- a/design_builder/contrib/ext.py +++ b/design_builder/contrib/ext.py @@ -7,14 +7,13 @@ from django.db.models import Q from nautobot.dcim.models import Cable -from nautobot.extras.models import Status from nautobot.ipam.models import Prefix import netaddr from design_builder.design import Builder from design_builder.design import ModelInstance -from design_builder.errors import DesignImplementationError +from design_builder.errors import DesignImplementationError, MultipleObjectsReturnedError, DoesNotExistError from design_builder.ext import Extension from design_builder.jinja2 import network_offset @@ -22,6 +21,8 @@ class LookupMixin: """A helper mixin that provides a way to lookup objects.""" + builder: Builder + def lookup_by_content_type(self, app_label, model_name, query): """Perform a query on a model. @@ -46,29 +47,31 @@ def lookup_by_content_type(self, app_label, model_name, query): return self.lookup(queryset, query) - def lookup(self, queryset, query): # pylint: disable=R0201 + def lookup(self, queryset, query, parent=None): """Perform a single object lookup from a queryset. Args: queryset: Queryset (e.g. Status.objects.all) from which to query. query: Query params to filter by. + parent: Optional field used for better error reporting. Set this + value to the model instance that is semantically the parent so + that DesignModelErrors raised are more easily debugged. Raises: - DesignImplementationError: If either no object is found, or multiple objects are found. + DoesNotExistError: If either no object is found. + MultipleObjectsReturnedError: if multiple objects are found. Returns: Any: The object matching the query. """ - # TODO: Convert this to lookup via ModelInstance - for key, value in query.items(): - if hasattr(value, "instance"): - query[key] = value.instance + self.builder.resolve_values(query, unwrap_model_instances=True) + try: return queryset.get(**query) except ObjectDoesNotExist: - raise DesignImplementationError(f"no {queryset.model.__name__} matching {query}") + raise DoesNotExistError(queryset.model, query_filter=query, parent=parent) except MultipleObjectsReturned: - raise DesignImplementationError(f"Multiple {queryset.model.__name__} objects match {query}") + raise MultipleObjectsReturnedError(queryset.model, query=query, parent=parent) class LookupExtension(Extension, LookupMixin): @@ -143,12 +146,13 @@ def attribute(self, value, model_instance) -> None: """Connect a cable termination to another cable termination. Args: - value: Query for the `termination_b` side. This dictionary must - include a field `status` or `status__` that is either - a reference to a status object (former) or a lookup key/value to - get a status (latter). The query must also include enough - differentiating lookup params to retrieve a single matching termination - of the same type as the `termination_a` side. + value: Dictionary with details about the cable. At a minimum + the dictionary must have a `to` key which includes a query + dictionary that will return exactly one object to be added to the + `termination_b` side of the cable. All other attributes map + directly to the cable attributes. Cables require a status, + so the `status` field is mandatory and follows typical design + builder query lookup. Raises: DesignImplementationError: If no `status` was provided, or no matching @@ -172,38 +176,49 @@ def attribute(self, value, model_instance) -> None: status__name: "Active" "!connect_cable": status__name: "Planned" - device: "!ref:device1" - name: "GigabitEthernet1" + to: + device: "!ref:device1" + name: "GigabitEthernet1" ``` """ - query = {**value} - status = query.pop("status", None) - if status is None: - for key in list(query.keys()): - if key.startswith("status__"): - status_lookup = key[len("status__") :] # noqa: E203 - status = Status.objects.get(**{status_lookup: query.pop(key)}) - break - elif isinstance(status, dict): - status = Status.objects.get(**status) - elif hasattr(status, "instance"): - status = status.instance - - if status is None: - raise DesignImplementationError("No status given for cable connection") - - remote_instance = self.lookup(model_instance.model_class.objects, query) + if "to" not in value: + raise DesignImplementationError( + f"`connect_cable` must have a `to` field indicating what to terminate to. {value}" + ) + + cable_attributes = {**value} + termination_query = cable_attributes.pop("to") + + # status = query.pop("status", None) + # if status is None: + # for key in list(query.keys()): + # if key.startswith("status__"): + # status_lookup = key[len("status__") :] # noqa: E203 + # status = Status.objects.get(**{status_lookup: query.pop(key)}) + # break + # elif isinstance(status, dict): + # status = Status.objects.get(**status) + # elif hasattr(status, "instance"): + # status = status.instance + + # if status is None: + # raise DesignImplementationError("No status given for cable connection") + + remote_instance = self.lookup(model_instance.model_class.objects, termination_query) + cable_attributes.update( + { + "termination_a": model_instance, + "!create_or_update:termination_b_type": ContentType.objects.get_for_model(remote_instance), + "!create_or_update:termination_b_id": remote_instance.id, + } + ) + model_instance.deferred.append("cable") model_instance.deferred_attributes["cable"] = [ model_instance.__class__( - self.object_creator, + self.builder, model_class=Cable, - attributes={ - "status": status, - "termination_a": model_instance, - "!create_or_update:termination_b_type": ContentType.objects.get_for_model(remote_instance), - "!create_or_update:termination_b_id": remote_instance.id, - }, + attributes=cable_attributes, ) ] @@ -276,7 +291,8 @@ def attribute(self, value: dict, model_instance) -> None: prefixes = Prefix.objects.filter(query) return "prefix", self._get_next(prefixes, length) - def _get_next(self, prefixes, length) -> str: # pylint:disable=no-self-use + @staticmethod + def _get_next(prefixes, length) -> str: """Return the next available prefix from a parent prefix. Args: @@ -364,7 +380,7 @@ class BGPPeeringExtension(Extension): attribute_tag = "bgp_peering" - def __init__(self, object_creator: Builder): + def __init__(self, builder: Builder): """Initialize the BGPPeeringExtension. This initializer will import the necessary BGP models. If the @@ -373,7 +389,7 @@ def __init__(self, object_creator: Builder): Raises: DesignImplementationError: Raised when the BGP Models Plugin is not installed. """ - super().__init__(object_creator) + super().__init__(builder) try: from nautobot_bgp_models.models import PeerEndpoint, Peering # pylint:disable=import-outside-toplevel @@ -440,8 +456,8 @@ def attribute(self, value, model_instance) -> None: # copy the value so it won't be modified in later # use retval = {**value} - endpoint_a = ModelInstance(self.object_creator, self.PeerEndpoint, retval.pop("endpoint_a")) - endpoint_z = ModelInstance(self.object_creator, self.PeerEndpoint, retval.pop("endpoint_z")) + endpoint_a = ModelInstance(self.builder, self.PeerEndpoint, retval.pop("endpoint_a")) + endpoint_z = ModelInstance(self.builder, self.PeerEndpoint, retval.pop("endpoint_z")) peering_a = None peering_z = None try: diff --git a/design_builder/contrib/tests/test_ext.py b/design_builder/contrib/tests/test_ext.py index 5d3594bd..8c01f998 100644 --- a/design_builder/contrib/tests/test_ext.py +++ b/design_builder/contrib/tests/test_ext.py @@ -88,8 +88,9 @@ def test_connect_cable(self): status__name: "Active" "!connect_cable": status__name: "Planned" - device: "!ref:device1" - name: "GigabitEthernet1" + to: + device: "!ref:device1" + name: "GigabitEthernet1" """ design_template_v2 = """ @@ -134,8 +135,9 @@ def test_connect_cable(self): status__name: "Active" "!connect_cable": status__name: "Planned" - device: "!ref:device1" - name: "GigabitEthernet1" + to: + device: "!ref:device1" + name: "GigabitEthernet1" """ if nautobot_version < "2.0.0": @@ -330,6 +332,7 @@ def test_creation(self): bgp_routing_instances: - "!create_or_update:autonomous_system__asn": 64500 "!ref": "device1-instance" + "status__name": "Active" - "!create_or_update:name": "device2" status__name: "Active" @@ -346,6 +349,7 @@ def test_creation(self): bgp_routing_instances: - "!create_or_update:autonomous_system__asn": 64500 "!ref": "device2-instance" + "status__name": "Active" bgp_peerings: - "!bgp_peering": diff --git a/design_builder/design.py b/design_builder/design.py index 9239f377..ce80c263 100644 --- a/design_builder/design.py +++ b/design_builder/design.py @@ -471,7 +471,7 @@ def implement_design(self, design, commit=False): self.roll_back() raise ex - def resolve_value(self, value): + def resolve_value(self, value, unwrap_model_instance=False): """Resolve a value using extensions, if needed.""" if isinstance(value, str) and value.startswith("!"): (action, arg) = value.lstrip("!").split(":", 1) @@ -480,9 +480,11 @@ def resolve_value(self, value): value = extn.value(arg) else: raise errors.DesignImplementationError(f"Unknown attribute extension {value}") + if unwrap_model_instance and isinstance(value, ModelInstance): + value = value.instance return value - def resolve_values(self, value): + def resolve_values(self, value, unwrap_model_instances=False): """Resolve a value, or values, using extensions. Args: @@ -492,13 +494,13 @@ def resolve_values(self, value): Any: The resolved value. """ if isinstance(value, str): - value = self.resolve_value(value) + value = self.resolve_value(value, unwrap_model_instances) elif isinstance(value, list): for i, item in enumerate(value): - value[i] = self.resolve_value(item) + value[i] = self.resolve_value(item, unwrap_model_instances) elif isinstance(value, dict): for k, item in value.items(): - value[k] = self.resolve_value(item) + value[k] = self.resolve_value(item, unwrap_model_instances) return value def _create_objects(self, model_cls, objects): diff --git a/design_builder/errors.py b/design_builder/errors.py index 7082c904..c8ea0cd2 100644 --- a/design_builder/errors.py +++ b/design_builder/errors.py @@ -3,6 +3,7 @@ from inspect import isclass from django.core.exceptions import ValidationError +from django.db.models import Model def _error_msg(validation_error): @@ -33,24 +34,35 @@ def __init__(self, message, model=None): class DesignModelError(Exception): """Parent class for all model related design errors.""" - def __init__(self, model=None) -> None: + def __init__(self, model=None, parent=None) -> None: """Initialize a DesignError with optional model_stack. Args: model: The model that generated the error. + parent: If model is a django model (as opposed to a design + builder ModelInstance) then a parent can be specified + in order to better represent the relationship of the + model within the design. """ super().__init__() self.model = model + self.parent = parent @staticmethod def _model_str(model): instance_str = None - if not hasattr(model, "instance"): + if not isinstance(model, Model) and not hasattr(model, "instance"): return str(model) - if model.instance: - instance_str = str(model.instance) - model_str = model.model_class._meta.verbose_name.capitalize() + model_class = model.__class__ + # if it looks like a duck... + if hasattr(model, "instance"): + model_class = model.model_class + model = model.instance + + if model: + instance_str = str(model) + model_str = model_class._meta.verbose_name.capitalize() if instance_str: model_str = f"{model_str} {instance_str}" return model_str @@ -83,6 +95,9 @@ def path_str(self): path_msg.insert(0, DesignModelError._model_str(model)) if hasattr(model, "parent"): model = model.parent + elif self.parent: + model = self.parent + self.parent = None else: model = None # don't include the top level model in the ancestry @@ -137,6 +152,16 @@ def __str__(self) -> str: class DesignQueryError(DesignModelError): """Exception indicating design builder could not find the object.""" + def __init__(self, model=None, query_filter=None, **kwargs): + """Initialize a design query error. + + Args: + model: Model or model class this query error corresponds to. + query_filter: Query filter the generated the error. + """ + super().__init__(model=model, **kwargs) + self.query_filter = query_filter + def __str__(self) -> str: """The string representation of an object of the DoesNotExistError class.""" msg = [] @@ -146,6 +171,8 @@ def __str__(self) -> str: msg.append(f"{indentation}- {self.model_str}:") if hasattr(self.model, "query_filter"): msg.append(DesignModelError._object_to_markdown(self.model.query_filter, indentation=f"{indentation} ")) + elif self.query_filter: + msg.append(DesignModelError._object_to_markdown(self.query_filter, indentation=f"{indentation} ")) else: msg.append(DesignModelError._object_to_markdown(self.model.filter, indentation=f"{indentation} ")) return "\n".join(msg) diff --git a/design_builder/ext.py b/design_builder/ext.py index bc18c959..75b580a3 100644 --- a/design_builder/ext.py +++ b/design_builder/ext.py @@ -64,12 +64,12 @@ class Extension(ABC): tag matching `tag_name` or `value_name` is encountered. Args: - object_creator (ObjectCreator): The object creator that is implementing the + builder (Builder): The object creator that is implementing the current design. """ - def __init__(self, object_creator: "Builder"): # noqa: D107 - self.object_creator = object_creator + def __init__(self, builder: "Builder"): # noqa: D107 + self.builder = builder def attribute(self, value: Any, model_instance: "ModelInstance") -> None: """This method is called when the `attribute_tag` is encountered. @@ -115,15 +115,15 @@ class ReferenceExtension(Extension): stored creator object. Args: - object_creator (ObjectCreator): The object creator that is implementing the + builder (Builder): The object creator that is implementing the current design. """ attribute_tag = "ref" value_tag = "ref" - def __init__(self, object_creator: "Builder"): # noqa: D107 - super().__init__(object_creator) + def __init__(self, builder: "Builder"): # noqa: D107 + super().__init__(builder) self._env = {} def attribute(self, value, model_instance): @@ -180,7 +180,7 @@ class GitContextExtension(Extension): """Provides the "!git_context" attribute extension that will save content to a git repo. Args: - object_creator (ObjectCreator): The object creator that is implementing the + builder (Builder): The object creator that is implementing the current design. Example: @@ -201,14 +201,14 @@ class GitContextExtension(Extension): attribute_tag = "git_context" - def __init__(self, object_creator: "Builder"): # noqa: D107 - super().__init__(object_creator) + def __init__(self, builder: "Builder"): # noqa: D107 + super().__init__(builder) self._env = { "files": [], "directories": [], } slug = DesignBuilderConfig.context_repository - self.context_repo = GitRepo(slug, object_creator.job_result) + self.context_repo = GitRepo(slug, builder.job_result) def attribute(self, value, model_instance): """Provide the attribute tag functionality for git_context. diff --git a/design_builder/jinja2.py b/design_builder/jinja2.py index ad559409..a92733d1 100644 --- a/design_builder/jinja2.py +++ b/design_builder/jinja2.py @@ -91,7 +91,8 @@ def parse(self, parser): args = [nodes.TemplateData(whitespace)] return nodes.CallBlock(self.call_method("_indent_support", args), [], [], body).set_lineno(lineno) - def _indent_support(self, indentation, caller): # pylint: disable=no-self-use + @staticmethod + def _indent_support(indentation, caller): """Perform the block indentation. Args: diff --git a/design_builder/tests/__init__.py b/design_builder/tests/__init__.py index b2d17b00..43e5b3bd 100644 --- a/design_builder/tests/__init__.py +++ b/design_builder/tests/__init__.py @@ -31,7 +31,7 @@ def setUp(self): git_instance_mock.return_value.path = self.git_path self.git_mock.side_effect = git_instance_mock - def get_mocked_job(self, design_class: Type[DesignJob]): # pylint: disable=no-self-use + def get_mocked_job(self, design_class: Type[DesignJob]): """Create an instance of design_class and properly mock request and job_result for testing.""" job = design_class() job.job_result = mock.Mock() diff --git a/design_builder/tests/test_errors.py b/design_builder/tests/test_errors.py index 6f0bfeb5..37fc7243 100644 --- a/design_builder/tests/test_errors.py +++ b/design_builder/tests/test_errors.py @@ -54,6 +54,14 @@ def test_path_str_ancestors(self): got = DesignModelError(child).path_str self.assertEqual(want, got) + def test_explicit_parent(self): + want = (" ", "- Verbose name instance grandparent\n - Verbose name instance parent") + grandparent = self.TestModel("instance grandparent") + parent = self.TestModel("instance parent", parent=grandparent) + child = "instance child" + got = DesignModelError(child, parent=parent).path_str + self.assertEqual(want, got) + class TestDesignValidationError(unittest.TestCase): def test_single_string(self): diff --git a/design_builder/tests/test_ext.py b/design_builder/tests/test_ext.py index 46115d34..dcf093cb 100644 --- a/design_builder/tests/test_ext.py +++ b/design_builder/tests/test_ext.py @@ -4,13 +4,12 @@ from django.test import TestCase -from nautobot.dcim.models import Interface, DeviceType +from nautobot.dcim.models import DeviceType from design_builder import ext -from design_builder.contrib.ext import LookupExtension, CableConnectionExtension +from design_builder.contrib.ext import LookupExtension from design_builder.design import Builder from design_builder.ext import DesignImplementationError -from design_builder.util import nautobot_version class Extension(ext.Extension): @@ -83,100 +82,3 @@ def test_lookup_by_single_attribute(self): builder.implement_design(design, commit=True) device_type = DeviceType.objects.get(model="model") self.assertEqual("Manufacturer", device_type.manufacturer.name) - - -class TestCableConnectionExtension(TestCase): - def test_connect_cable(self): - design_template_v1 = """ - sites: - - name: "Site" - status__name: "Active" - device_roles: - - name: "test-role" - manufacturers: - - name: "test-manufacturer" - device_types: - - manufacturer__name: "test-manufacturer" - model: "test-type" - devices: - - name: "Device 1" - "!ref": "device1" - site__name: "Site" - status__name: "Active" - device_role__name: "test-role" - device_type__model: "test-type" - interfaces: - - name: "GigabitEthernet1" - type: "1000base-t" - status__name: "Active" - - name: "Device 2" - site__name: "Site" - status__name: "Active" - device_role__name: "test-role" - device_type__model: "test-type" - interfaces: - - name: "GigabitEthernet1" - type: "1000base-t" - status__name: "Active" - "!connect_cable": - status__name: "Planned" - device: "!ref:device1" - name: "GigabitEthernet1" - """ - - design_template_v2 = """ - location_types: - - name: "Site" - content_types: - - "!get:app_label": "dcim" - "!get:model": "device" - locations: - - location_type__name: "Site" - name: "Site" - status__name: "Active" - roles: - - name: "test-role" - content_types: - - "!get:app_label": "dcim" - "!get:model": "device" - manufacturers: - - name: "test-manufacturer" - device_types: - - manufacturer__name: "test-manufacturer" - model: "test-type" - devices: - - name: "Device 1" - "!ref": "device1" - location__name: "Site" - status__name: "Active" - role__name: "test-role" - device_type__model: "test-type" - interfaces: - - name: "GigabitEthernet1" - type: "1000base-t" - status__name: "Active" - - name: "Device 2" - location__name: "Site" - status__name: "Active" - role__name: "test-role" - device_type__model: "test-type" - interfaces: - - name: "GigabitEthernet1" - type: "1000base-t" - status__name: "Active" - "!connect_cable": - status__name: "Planned" - device: "!ref:device1" - name: "GigabitEthernet1" - """ - - if nautobot_version < "2.0.0": - design = yaml.safe_load(design_template_v1) - else: - design = yaml.safe_load(design_template_v2) - - builder = Builder(extensions=[CableConnectionExtension]) - builder.implement_design(design, commit=True) - interfaces = Interface.objects.all() - self.assertEqual(2, len(interfaces)) - self.assertEqual(interfaces[0].connected_endpoint, interfaces[1]) diff --git a/development/development.env b/development/development.env index a082a5eb..b572472e 100644 --- a/development/development.env +++ b/development/development.env @@ -13,6 +13,7 @@ NAUTOBOT_LOG_LEVEL=DEBUG NAUTOBOT_METRICS_ENABLED=True NAUTOBOT_NAPALM_TIMEOUT=5 NAUTOBOT_MAX_PAGE_SIZE=0 +NAUTOBOT_INSTALLATION_METRICS_ENABLED = False # Redis Configuration Environment Variables NAUTOBOT_REDIS_HOST=redis @@ -43,4 +44,4 @@ MYSQL_ROOT_HOST=% DESIGN_BUILDER_GIT_SERVER=http://git-server.local:3000 DESIGN_BUILDER_CONTEXT_REPO_SLUG=config-contexts DESIGN_BUILDER_CONTEXT_REPO=config-contexts.git -DESIGN_BUILDER_DESIGN_REPO=designs.git \ No newline at end of file +DESIGN_BUILDER_DESIGN_REPO=designs.git diff --git a/pyproject.toml b/pyproject.toml index 12ffbc7b..93fc2ce5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "design-builder" -version = "0.2.1" +version = "0.3.0" description = "A plugin that uses design templates to easily create data objects in Nautobot with minimal input from a user." authors = ["Network to Code, LLC "] readme = "README.md" @@ -88,7 +88,6 @@ no-docstring-rgx="^(_|test_|Test|Meta$)" # Pylint and Black disagree about how to format multi-line arrays; Black wins. disable = """, line-too-long, - bad-continuation, duplicate-code, too-many-lines, too-many-ancestors,