get() to raise ValueError instead of returning None with bad params; linter fixes

Author: glennmatthews

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Unreleased
 
 - `DiffSync.get_by_uids()` now raises `ObjectNotFound` if any of the provided uids cannot be located.
+- `DiffSync.get()` raises `ObjectNotFound` or `ValueError` on failure, instead of returning `None`.
 - #34 - in diff dicts, change keys `src`/`dst`/`_src`/`_dst` to `-` and `+`
 - #37 - add `sync_complete` callback, triggered on `sync_from` completion with changes.
 - #41 - add `summary` API for Diff and DiffElement objects.

diffsync/__init__.py

@@ -549,7 +549,7 @@ def _sync_from_diff_element(
         log = logger or self._log
         object_class = getattr(self, element.type)
         try:
-            obj = self.get(object_class, element.keys)
+            obj: Optional[DiffSyncModel] = self.get(object_class, element.keys)
         except ObjectNotFound:
             obj = None
         # Get the attributes that actually differ between source and dest
@@ -654,6 +654,7 @@ def get(
             identifier: Unique ID of the object to retrieve, or dict of unique identifier keys/values
 
         Raises:
+            ValueError: if obj is a str and identifier is a dict (can't convert dict into a uid str without a model class)
             ObjectNotFound: if the requested object is not present
         """
         if isinstance(obj, str):
@@ -671,11 +672,10 @@ def get(
         elif object_class:
             uid = object_class.create_unique_id(**identifier)
         else:
-            self._log.warning(
-                f"Tried to look up a {modelname} by identifier {identifier}, "
-                "but don't know how to convert that to a uid string",
+            raise ValueError(
+                f"Invalid args: ({obj}, {identifier}): "
+                f"either {obj} should be a class/instance or {identifier} should be a str"
             )
-            return None
 
         if uid not in self._data[modelname]:
             raise ObjectNotFound(f"{modelname} {uid} not present in {self.name}")

tests/unit/test_diffsync.py

@@ -89,8 +89,9 @@ def test_diffsync_get_with_generic_model(generic_diffsync, generic_diffsync_mode
     # The generic_diffsync_model has an empty identifier/unique-id
     assert generic_diffsync.get(DiffSyncModel, "") == generic_diffsync_model
     assert generic_diffsync.get(DiffSyncModel.get_type(), "") == generic_diffsync_model
-    # DiffSync doesn't know how to construct a uid str for a "diffsyncmodel"
-    assert generic_diffsync.get(DiffSyncModel.get_type(), {}) is None
+    # DiffSync doesn't know how to construct a uid str for a "diffsyncmodel" (it needs the class or instance, not a str)
+    with pytest.raises(ValueError):
+        generic_diffsync.get(DiffSyncModel.get_type(), {})
     # Wrong object-type - no match
     with pytest.raises(ObjectNotFound):
         generic_diffsync.get("", "")
@@ -147,6 +148,7 @@ class BadElementName(DiffSync):
 
 
 def test_diffsync_subclass_validation_missing_top_level():
+    # pylint: disable=unused-variable
     with pytest.raises(AttributeError) as excinfo:
 
         class MissingTopLevel(DiffSync):
@@ -160,6 +162,7 @@ class MissingTopLevel(DiffSync):
 
 
 def test_diffsync_subclass_validation_top_level_not_diffsyncmodel():
+    # pylint: disable=unused-variable
     with pytest.raises(AttributeError) as excinfo:
 
         class TopLevelNotDiffSyncModel(DiffSync):