Require attrs for update_or, Set existing_object as attribute in Obje…

…ctAlreadyExists, other feedback.

diffsync/__init__.py

@@ -722,15 +722,13 @@ def get_or_instantiate(
 
         return obj, created
 
-    def update_or_instantiate(
-        self, model: Type[DiffSyncModel], ids: Dict, attrs: Dict = None
-    ) -> Tuple[DiffSyncModel, bool]:
+    def update_or_instantiate(self, model: Type[DiffSyncModel], ids: Dict, attrs: Dict) -> Tuple[DiffSyncModel, bool]:
         """Attempt to update an existing object with provided ids/attrs or instantiate it with provided identifiers and attrs.
 
         Args:
             model (DiffSyncModel): The DiffSyncModel to get or create.
-            ids (Mapping): Identifiers for the DiffSyncModel to get or create with.
-            attrs (Mapping, optional): Attributes when creating an object if it doesn't exist. Defaults to None.
+            ids (Dict): Identifiers for the DiffSyncModel to get or create with.
+            attrs (Dict): Attributes when creating/updating an object if it doesn't exist. Pass in empty dict, if no specific attrs.
 
         Returns:
             Tuple[DiffSyncModel, bool]: Provides the existing or new object and whether it was created or not.
@@ -739,20 +737,15 @@ def update_or_instantiate(
         try:
             obj = self.get(model, ids)
         except ObjectNotFound:
-            if not attrs:
-                attrs = {}
             obj = model(**ids, **attrs)
             # Add the object to diffsync adapter
             self.add(obj)
             created = True
 
         # Update existing obj with attrs
-        if attrs:
-            existing_attrs = obj.get_attrs()
-            mutual_attrs = set(existing_attrs).intersection(attrs)
-            for attr in mutual_attrs:
-                if existing_attrs[attr] != attrs[attr]:
-                    setattr(obj, attr, attrs[attr])
+        for attr, value in attrs.items():
+            if getattr(obj, attr) != value:
+                setattr(obj, attr, value)
 
         return obj, created
 

diffsync/diff.py

@@ -55,7 +55,7 @@ def add(self, element: "DiffElement"):
         """
         # Note that element.name is usually a DiffSyncModel.shortname() -- i.e., NOT guaranteed globally unique!!
         if element.name in self.children[element.type]:
-            raise ObjectAlreadyExists(f"Already storing a {element.type} named {element.name}")
+            raise ObjectAlreadyExists(f"Already storing a {element.type} named {element.name}", element)
 
         self.children[element.type][element.name] = element
 

diffsync/exceptions.py

@@ -39,6 +39,11 @@ class ObjectStoreException(Exception):
 class ObjectAlreadyExists(ObjectStoreException):
     """Exception raised when trying to store a DiffSyncModel or DiffElement that is already being stored."""
 
+    def __init__(self, message, existing_object, *args, **kwargs):
+        """Add existing_object to the exception to provide user with existing object."""
+        self.existing_object = existing_object
+        super().__init__(message, existing_object, *args, **kwargs)
+
 
 class ObjectNotFound(ObjectStoreException):
     """Exception raised when trying to access a DiffSyncModel that isn't in storage."""

examples/04-get-update-instantiate/backends.py

@@ -15,9 +15,8 @@
 limitations under the License.
 """
 
-# pylint: disable=wrong-import-order
+from models import Site, Device, Interface
 from diffsync import DiffSync
-from models import Site, Device, Interface  # pylint: disable=no-name-in-module
 
 BACKEND_DATA_A = [
     {

examples/04-get-update-instantiate/main.py

@@ -15,16 +15,15 @@
 See the License for the specific language governing permissions and
 limitations under the License.
 """
-# pylint: disable=wrong-import-order
 
 import argparse
 import pprint
 
-from diffsync.logging import enable_console_logging
-
 from backends import BackendA
 from backends import BackendB
 
+from diffsync.logging import enable_console_logging
+
 
 def main():
     """Demonstrate DiffSync behavior using the example backends provided."""

tests/unit/test_diffsync.py

@@ -107,21 +107,19 @@ def test_diffsync_add_raises_already_exists_with_updated_object(generic_diffsync
     new_intf = Interface(device_name="device1", name="eth0", interface_type="1000base-t")
     with pytest.raises(ObjectAlreadyExists) as error:
         generic_diffsync.add(new_intf)
-    error_model = error.value.args[1]
+    error_model = error.value.existing_object
     assert isinstance(error_model, DiffSyncModel)
     assert new_intf is error_model
 
 
 def test_diffsync_get_or_instantiate_create_non_existent_object(generic_diffsync):
     intf_identifiers = {"device_name": "device1", "name": "eth1"}
-    intf = Interface(**intf_identifiers)
 
     # Assert that the object does not currently exist.
     with pytest.raises(ObjectNotFound):
         generic_diffsync.get(Interface, intf_identifiers)
 
     obj, created = generic_diffsync.get_or_instantiate(Interface, intf_identifiers)
-    assert obj == intf
     assert created
     assert obj is generic_diffsync.get(Interface, intf_identifiers)
 
@@ -136,6 +134,19 @@ def test_diffsync_get_or_instantiate_retrieve_existing_object(generic_diffsync):
     assert not created
 
 
+def test_diffsync_get_or_instantiate_retrieve_existing_object_w_attrs(generic_diffsync):
+    intf_identifiers = {"device_name": "device1", "name": "eth1"}
+    intf_attrs = {"interface_type": "1000base-t", "description": "Testing"}
+    intf = Interface(**intf_identifiers)
+    generic_diffsync.add(intf)
+
+    obj, created = generic_diffsync.get_or_instantiate(Interface, intf_identifiers, intf_attrs)
+    assert obj is intf
+    assert not created
+    assert obj.interface_type == "ethernet"
+    assert obj.description is None
+
+
 def test_diffsync_get_or_instantiate_retrieve_create_non_existent_w_attrs(generic_diffsync):
     intf_identifiers = {"device_name": "device1", "name": "eth1"}
     intf_attrs = {"interface_type": "1000base-t", "description": "Testing"}
@@ -159,18 +170,6 @@ def test_diffsync_get_or_instantiate_retrieve_existing_object_wo_attrs(generic_d
     assert obj.description is None
 
 
-def test_diffsync_update_or_instantiate_retrieve_existing_object(generic_diffsync):
-    intf_identifiers = {"device_name": "device1", "name": "eth1"}
-    intf = Interface(**intf_identifiers)
-    generic_diffsync.add(intf)
-
-    obj, created = generic_diffsync.update_or_instantiate(Interface, intf_identifiers)
-    assert obj is intf
-    assert not created
-    assert obj.interface_type == "ethernet"
-    assert obj.description is None
-
-
 def test_diffsync_update_or_instantiate_retrieve_existing_object_w_updated_attrs(generic_diffsync):
     intf_identifiers = {"device_name": "device1", "name": "eth1"}
     intf_attrs = {"interface_type": "1000base-t", "description": "Testing"}
@@ -187,7 +186,7 @@ def test_diffsync_update_or_instantiate_retrieve_existing_object_w_updated_attrs
 def test_diffsync_update_or_instantiate_create_object(generic_diffsync):
     intf_identifiers = {"device_name": "device1", "name": "eth1"}
 
-    obj, created = generic_diffsync.update_or_instantiate(Interface, intf_identifiers)
+    obj, created = generic_diffsync.update_or_instantiate(Interface, intf_identifiers, {})
     assert created
     assert obj.interface_type == "ethernet"
     assert obj.description is None