From 4fa39b5a7b1547e5002d01963f955f5e1317e660 Mon Sep 17 00:00:00 2001 From: Leo Kirchner Date: Fri, 17 Mar 2023 15:43:53 +0100 Subject: [PATCH] Type hinting overhaul. --- diffsync/__init__.py | 189 +++++++++++++++++++------------------ diffsync/diff.py | 83 ++++++++-------- diffsync/exceptions.py | 7 +- diffsync/helpers.py | 22 +++-- diffsync/logging.py | 6 +- diffsync/store/__init__.py | 62 ++++++------ diffsync/store/local.py | 26 ++--- diffsync/store/redis.py | 59 +++++++----- diffsync/utils.py | 23 +++-- poetry.lock | 38 +++++++- pyproject.toml | 5 +- tests/unit/conftest.py | 10 +- 12 files changed, 302 insertions(+), 228 deletions(-) diff --git a/diffsync/__init__.py b/diffsync/__init__.py index 3fbe7186..412270d8 100644 --- a/diffsync/__init__.py +++ b/diffsync/__init__.py @@ -15,7 +15,8 @@ limitations under the License. """ from inspect import isclass -from typing import Callable, ClassVar, Dict, List, Mapping, Optional, Text, Tuple, Type, Union +from typing import Callable, ClassVar, Dict, List, Optional, Tuple, Type, Union, Any, Set +from typing_extensions import Self from pydantic import BaseModel, PrivateAttr import structlog # type: ignore @@ -28,6 +29,10 @@ from diffsync.store.local import LocalStore from diffsync.utils import get_path, set_key, tree_string +# This workaround is used because we are defining a method called `str` in our class definition, which therefore renders +# the builtin `str` type unusable. +StrType = str + class DiffSyncModel(BaseModel): """Base class for all DiffSync object models. @@ -72,7 +77,7 @@ class DiffSyncModel(BaseModel): Note: inclusion in `_attributes` is mutually exclusive from inclusion in `_identifiers`; a field cannot be in both! """ - _children: ClassVar[Mapping[str, str]] = {} + _children: ClassVar[Dict[str, str]] = {} """Optional: dict of `{_modelname: field_name}` entries describing how to store "child" models in this model. When calculating a Diff or performing a sync, DiffSync will automatically recurse into these child models. @@ -101,7 +106,7 @@ class Config: # pylint: disable=too-few-public-methods # Let us have a DiffSync as an instance variable even though DiffSync is not a Pydantic model itself. arbitrary_types_allowed = True - def __init_subclass__(cls): + def __init_subclass__(cls) -> None: """Validate that the various class attribute declarations correspond to actual instance fields. Called automatically on subclass declaration. @@ -132,19 +137,19 @@ def __init_subclass__(cls): if attr_child_overlap: raise AttributeError(f"Fields {attr_child_overlap} are included in both _attributes and _children.") - def __repr__(self): + def __repr__(self) -> str: return f'{self.get_type()} "{self.get_unique_id()}"' - def __str__(self): + def __str__(self) -> str: return self.get_unique_id() - def dict(self, **kwargs) -> dict: + def dict(self, **kwargs: Any) -> Dict: """Convert this DiffSyncModel to a dict, excluding the diffsync field by default as it is not serializable.""" if "exclude" not in kwargs: kwargs["exclude"] = {"diffsync"} return super().dict(**kwargs) - def json(self, **kwargs) -> str: + def json(self, **kwargs: Any) -> StrType: """Convert this DiffSyncModel to a JSON string, excluding the diffsync field by default as it is not serializable.""" if "exclude" not in kwargs: kwargs["exclude"] = {"diffsync"} @@ -152,7 +157,7 @@ def json(self, **kwargs) -> str: kwargs["exclude_defaults"] = True return super().json(**kwargs) - def str(self, include_children: bool = True, indent: int = 0) -> str: + def str(self, include_children: bool = True, indent: int = 0) -> StrType: """Build a detailed string representation of this DiffSyncModel and optionally its children.""" margin = " " * indent output = f"{margin}{self.get_type()}: {self.get_unique_id()}: {self.get_attrs()}" @@ -172,13 +177,13 @@ def str(self, include_children: bool = True, indent: int = 0) -> str: output += f"\n{margin} {child_id} (ERROR: details unavailable)" return output - def set_status(self, status: DiffSyncStatus, message: Text = ""): + def set_status(self, status: DiffSyncStatus, message: StrType = "") -> None: """Update the status (and optionally status message) of this model in response to a create/update/delete call.""" self._status = status self._status_message = message @classmethod - def create_base(cls, diffsync: "DiffSync", ids: Mapping, attrs: Mapping) -> Optional["DiffSyncModel"]: + def create_base(cls, diffsync: "DiffSync", ids: Dict, attrs: Dict) -> Optional[Self]: """Instantiate this class, along with any platform-specific data creation. This method is not meant to be subclassed, users should redefine create() instead. @@ -196,7 +201,7 @@ def create_base(cls, diffsync: "DiffSync", ids: Mapping, attrs: Mapping) -> Opti return model @classmethod - def create(cls, diffsync: "DiffSync", ids: Mapping, attrs: Mapping) -> Optional["DiffSyncModel"]: + def create(cls, diffsync: "DiffSync", ids: Dict, attrs: Dict) -> Optional[Self]: """Instantiate this class, along with any platform-specific data creation. Subclasses must call `super().create()` or `self.create_base()`; they may wish to then override the default status information @@ -216,7 +221,7 @@ def create(cls, diffsync: "DiffSync", ids: Mapping, attrs: Mapping) -> Optional[ """ return cls.create_base(diffsync=diffsync, ids=ids, attrs=attrs) - def update_base(self, attrs: Mapping) -> Optional["DiffSyncModel"]: + def update_base(self, attrs: Dict) -> Optional[Self]: """Base Update method to update the attributes of this instance, along with any platform-specific data updates. This method is not meant to be subclassed, users should redefine update() instead. @@ -234,7 +239,7 @@ def update_base(self, attrs: Mapping) -> Optional["DiffSyncModel"]: self.set_status(DiffSyncStatus.SUCCESS, "Updated successfully") return self - def update(self, attrs: Mapping) -> Optional["DiffSyncModel"]: + def update(self, attrs: Dict) -> Optional[Self]: """Update the attributes of this instance, along with any platform-specific data updates. Subclasses must call `super().update()` or `self.update_base()`; they may wish to then override the default status information @@ -252,7 +257,7 @@ def update(self, attrs: Mapping) -> Optional["DiffSyncModel"]: """ return self.update_base(attrs=attrs) - def delete_base(self) -> Optional["DiffSyncModel"]: + def delete_base(self) -> Optional[Self]: """Base delete method Delete any platform-specific data corresponding to this instance. This method is not meant to be subclassed, users should redefine delete() instead. @@ -263,7 +268,7 @@ def delete_base(self) -> Optional["DiffSyncModel"]: self.set_status(DiffSyncStatus.SUCCESS, "Deleted successfully") return self - def delete(self) -> Optional["DiffSyncModel"]: + def delete(self) -> Optional[Self]: """Delete any platform-specific data corresponding to this instance. Subclasses must call `super().delete()` or `self.delete_base()`; they may wish to then override the default status information @@ -279,7 +284,7 @@ def delete(self) -> Optional["DiffSyncModel"]: return self.delete_base() @classmethod - def get_type(cls) -> Text: + def get_type(cls) -> StrType: """Return the type AKA modelname of the object or the class Returns: @@ -288,7 +293,7 @@ def get_type(cls) -> Text: return cls._modelname @classmethod - def create_unique_id(cls, **identifiers) -> Text: + def create_unique_id(cls, **identifiers: Dict[StrType, Any]) -> StrType: """Construct a unique identifier for this model class. Args: @@ -297,11 +302,11 @@ def create_unique_id(cls, **identifiers) -> Text: return "__".join(str(identifiers[key]) for key in cls._identifiers) @classmethod - def get_children_mapping(cls) -> Mapping[Text, Text]: + def get_children_mapping(cls) -> Dict[StrType, StrType]: """Get the mapping of types to fieldnames for child models of this model.""" return cls._children - def get_identifiers(self) -> Mapping: + def get_identifiers(self) -> Dict: """Get a dict of all identifiers (primary keys) and their values for this object. Returns: @@ -309,7 +314,7 @@ def get_identifiers(self) -> Mapping: """ return self.dict(include=set(self._identifiers)) - def get_attrs(self) -> Mapping: + def get_attrs(self) -> Dict: """Get all the non-primary-key attributes or parameters for this object. Similar to Pydantic's `BaseModel.dict()` method, with the following key differences: @@ -322,7 +327,7 @@ def get_attrs(self) -> Mapping: """ return self.dict(include=set(self._attributes)) - def get_unique_id(self) -> Text: + def get_unique_id(self) -> StrType: """Get the unique ID of an object. By default the unique ID is built based on all the primary keys defined in `_identifiers`. @@ -332,7 +337,7 @@ def get_unique_id(self) -> Text: """ return self.create_unique_id(**self.get_identifiers()) - def get_shortname(self) -> Text: + def get_shortname(self) -> StrType: """Get the (not guaranteed-unique) shortname of an object, if any. By default the shortname is built based on all the keys defined in `_shortname`. @@ -345,11 +350,11 @@ def get_shortname(self) -> Text: return "__".join([str(getattr(self, key)) for key in self._shortname]) return self.get_unique_id() - def get_status(self) -> Tuple[DiffSyncStatus, Text]: + def get_status(self) -> Tuple[DiffSyncStatus, StrType]: """Get the status of the last create/update/delete operation on this object, and any associated message.""" - return (self._status, self._status_message) + return self._status, self._status_message - def add_child(self, child: "DiffSyncModel"): + def add_child(self, child: "DiffSyncModel") -> None: """Add a child reference to an object. The child object isn't stored, only its unique id. @@ -373,7 +378,7 @@ def add_child(self, child: "DiffSyncModel"): raise ObjectAlreadyExists(f"Already storing a {child_type} with unique_id {child.get_unique_id()}", child) childs.append(child.get_unique_id()) - def remove_child(self, child: "DiffSyncModel"): + def remove_child(self, child: "DiffSyncModel") -> None: """Remove a child reference from an object. The name of the storage attribute is defined in `_children` per object type. @@ -404,13 +409,15 @@ class DiffSync: # pylint: disable=too-many-public-methods # modelname1 = MyModelClass1 # modelname2 = MyModelClass2 - type: ClassVar[Optional[str]] = None + type: Optional[str] = None """Type of the object, will default to the name of the class if not provided.""" top_level: ClassVar[List[str]] = [] """List of top-level modelnames to begin from when diffing or synchronizing.""" - def __init__(self, name=None, internal_storage_engine=LocalStore): + def __init__( + self, name: Optional[str] = None, internal_storage_engine: Union[Type[BaseStore], BaseStore] = LocalStore + ) -> None: """Generic initialization function. Subclasses should be careful to call super().__init__() if they override this method. @@ -429,7 +436,7 @@ def __init__(self, name=None, internal_storage_engine=LocalStore): # If the name has not been provided, use the type as the name self.name = name if name else self.type - def __init_subclass__(cls): + def __init_subclass__(cls) -> None: """Validate that references to specific DiffSyncModels use the correct modelnames. Called automatically on subclass declaration. @@ -448,16 +455,16 @@ def __init_subclass__(cls): if not isclass(value) or not issubclass(value, DiffSyncModel): raise AttributeError(f'top_level references attribute "{name}" but it is not a DiffSyncModel subclass!') - def __str__(self): + def __str__(self) -> StrType: """String representation of a DiffSync.""" if self.type != self.name: return f'{self.type} "{self.name}"' return self.type - def __repr__(self): + def __repr__(self) -> StrType: return f"<{str(self)}>" - def __len__(self): + def __len__(self) -> int: """Total number of elements stored.""" return self.store.count() @@ -481,11 +488,11 @@ def _get_initial_value_order(cls) -> List[str]: value_order.append(item) return value_order - def load(self): + def load(self) -> None: """Load all desired data from whatever backend data source into this instance.""" # No-op in this generic class - def dict(self, exclude_defaults: bool = True, **kwargs) -> Mapping: + def dict(self, exclude_defaults: bool = True, **kwargs: Any) -> Dict[str, Dict[str, Dict]]: """Represent the DiffSync contents as a dict, as if it were a Pydantic model.""" data: Dict[str, Dict[str, Dict]] = {} for modelname in self.store.get_all_model_names(): @@ -494,7 +501,7 @@ def dict(self, exclude_defaults: bool = True, **kwargs) -> Mapping: data[obj.get_type()][obj.get_unique_id()] = obj.dict(exclude_defaults=exclude_defaults, **kwargs) return data - def str(self, indent: int = 0) -> str: + def str(self, indent: int = 0) -> StrType: """Build a detailed string representation of this DiffSync.""" margin = " " * indent output = "" @@ -510,7 +517,7 @@ def str(self, indent: int = 0) -> str: output += "\n" + model.str(indent=indent + 2) return output - def load_from_dict(self, data: Dict): + def load_from_dict(self, data: Dict) -> None: """The reverse of `dict` method, taking a dictionary and loading into the inventory. Args: @@ -531,20 +538,20 @@ def sync_from( # pylint: disable=too-many-arguments source: "DiffSync", diff_class: Type[Diff] = Diff, flags: DiffSyncFlags = DiffSyncFlags.NONE, - callback: Optional[Callable[[Text, int, int], None]] = None, + callback: Optional[Callable[[StrType, int, int], None]] = None, diff: Optional[Diff] = None, ) -> Diff: """Synchronize data from the given source DiffSync object into the current DiffSync object. Args: - source (DiffSync): object to sync data from into this one - diff_class (class): Diff or subclass thereof to use to calculate the diffs to use for synchronization - flags (DiffSyncFlags): Flags influencing the behavior of this sync. - callback (function): Function with parameters (stage, current, total), to be called at intervals as the - calculation of the diff and subsequent sync proceed. - diff (Diff): An existing diff to be used rather than generating a completely new diff. + source: object to sync data from into this one + diff_class: Diff or subclass thereof to use to calculate the diffs to use for synchronization + flags: Flags influencing the behavior of this sync. + callback: Function with parameters (stage, current, total), to be called at intervals as the calculation of + the diff and subsequent sync proceed. + diff: An existing diff to be used rather than generating a completely new diff. Returns: - Diff: Diff between origin object and source + Diff between origin object and source Raises: DiffClassMismatch: The provided diff's class does not match the diff_class """ @@ -569,20 +576,20 @@ def sync_to( # pylint: disable=too-many-arguments target: "DiffSync", diff_class: Type[Diff] = Diff, flags: DiffSyncFlags = DiffSyncFlags.NONE, - callback: Optional[Callable[[Text, int, int], None]] = None, + callback: Optional[Callable[[StrType, int, int], None]] = None, diff: Optional[Diff] = None, ) -> Diff: """Synchronize data from the current DiffSync object into the given target DiffSync object. Args: - target (DiffSync): object to sync data into from this one. - diff_class (class): Diff or subclass thereof to use to calculate the diffs to use for synchronization - flags (DiffSyncFlags): Flags influencing the behavior of this sync. - callback (function): Function with parameters (stage, current, total), to be called at intervals as the - calculation of the diff and subsequent sync proceed. - diff (Diff): An existing diff that will be used when determining what needs to be synced. + target: object to sync data into from this one. + diff_class: Diff or subclass thereof to use to calculate the diffs to use for synchronization + flags: Flags influencing the behavior of this sync. + callback: Function with parameters (stage, current, total), to be called at intervals as the calculation of + the diff and subsequent sync proceed. + diff: An existing diff that will be used when determining what needs to be synced. Returns: - Diff: Diff between origin object and target + Diff between origin object and target Raises: DiffClassMismatch: The provided diff's class does not match the diff_class """ @@ -594,7 +601,7 @@ def sync_complete( diff: Diff, flags: DiffSyncFlags = DiffSyncFlags.NONE, logger: Optional[structlog.BoundLogger] = None, - ): + ) -> None: """Callback triggered after a `sync_from` operation has completed and updated the model data of this instance. Note that this callback is **only** triggered if the sync actually resulted in data changes. If there are no @@ -619,15 +626,15 @@ def diff_from( source: "DiffSync", diff_class: Type[Diff] = Diff, flags: DiffSyncFlags = DiffSyncFlags.NONE, - callback: Optional[Callable[[Text, int, int], None]] = None, + callback: Optional[Callable[[StrType, int, int], None]] = None, ) -> Diff: """Generate a Diff describing the difference from the other DiffSync to this one. Args: - source (DiffSync): Object to diff against. - diff_class (class): Diff or subclass thereof to use for diff calculation and storage. - flags (DiffSyncFlags): Flags influencing the behavior of this diff operation. - callback (function): Function with parameters (stage, current, total), to be called at intervals as the + source: Object to diff against. + diff_class: Diff or subclass thereof to use for diff calculation and storage. + flags: Flags influencing the behavior of this diff operation. + callback: Function with parameters (stage, current, total), to be called at intervals as the calculation of the diff proceeds. """ differ = DiffSyncDiffer( @@ -640,15 +647,15 @@ def diff_to( target: "DiffSync", diff_class: Type[Diff] = Diff, flags: DiffSyncFlags = DiffSyncFlags.NONE, - callback: Optional[Callable[[Text, int, int], None]] = None, + callback: Optional[Callable[[StrType, int, int], None]] = None, ) -> Diff: """Generate a Diff describing the difference from this DiffSync to another one. Args: - target (DiffSync): Object to diff against. - diff_class (class): Diff or subclass thereof to use for diff calculation and storage. - flags (DiffSyncFlags): Flags influencing the behavior of this diff operation. - callback (function): Function with parameters (stage, current, total), to be called at intervals as the + target: Object to diff against. + diff_class: Diff or subclass thereof to use for diff calculation and storage. + flags: Flags influencing the behavior of this diff operation. + callback: Function with parameters (stage, current, total), to be called at intervals as the calculation of the diff proceeds. """ return target.diff_from(self, diff_class=diff_class, flags=flags, callback=callback) @@ -657,16 +664,16 @@ def diff_to( # Object Storage Management # ------------------------------------------------------------------------------ - def get_all_model_names(self): + def get_all_model_names(self) -> Set[StrType]: """Get all model names. Returns: - List[str]: List of model names + List of model names """ return self.store.get_all_model_names() def get( - self, obj: Union[Text, DiffSyncModel, Type[DiffSyncModel]], identifier: Union[Text, Mapping] + self, obj: Union[StrType, DiffSyncModel, Type[DiffSyncModel]], identifier: Union[StrType, Dict] ) -> DiffSyncModel: """Get one object from the data store based on its unique id. @@ -681,7 +688,7 @@ def get( return self.store.get(model=obj, identifier=identifier) def get_or_none( - self, obj: Union[Text, DiffSyncModel, Type[DiffSyncModel]], identifier: Union[Text, Mapping] + self, obj: Union[StrType, DiffSyncModel, Type[DiffSyncModel]], identifier: Union[StrType, Dict] ) -> Optional[DiffSyncModel]: """Get one object from the data store based on its unique id or get a None @@ -700,19 +707,19 @@ def get_or_none( except ObjectNotFound: return None - def get_all(self, obj: Union[Text, DiffSyncModel, Type[DiffSyncModel]]) -> List[DiffSyncModel]: + def get_all(self, obj: Union[StrType, DiffSyncModel, Type[DiffSyncModel]]) -> List[DiffSyncModel]: """Get all objects of a given type. Args: obj: DiffSyncModel class or instance, or modelname string, that defines the type of the objects to retrieve Returns: - List[DiffSyncModel]: List of Object + List of Object """ return self.store.get_all(model=obj) def get_by_uids( - self, uids: List[Text], obj: Union[Text, DiffSyncModel, Type[DiffSyncModel]] + self, uids: List[StrType], obj: Union[StrType, DiffSyncModel, Type[DiffSyncModel]] ) -> List[DiffSyncModel]: """Get multiple objects from the store by their unique IDs/Keys and type. @@ -726,11 +733,11 @@ def get_by_uids( return self.store.get_by_uids(uids=uids, model=obj) @classmethod - def get_tree_traversal(cls, as_dict: bool = False) -> Union[Text, Mapping]: + def get_tree_traversal(cls, as_dict: bool = False) -> Union[StrType, Dict]: """Get a string describing the tree traversal for the diffsync object. Args: - as_dict: Whether or not to return as a dictionary + as_dict: Whether to return as a dictionary Returns: A string or dictionary representation of tree @@ -751,34 +758,34 @@ def get_tree_traversal(cls, as_dict: bool = False) -> Union[Text, Mapping]: return output_dict return tree_string(output_dict, cls.__name__) - def add(self, obj: DiffSyncModel): + def add(self, obj: DiffSyncModel) -> None: """Add a DiffSyncModel object to the store. Args: - obj (DiffSyncModel): Object to store + obj: Object to store Raises: ObjectAlreadyExists: if a different object with the same uid is already present. """ return self.store.add(obj=obj) - def update(self, obj: DiffSyncModel): + def update(self, obj: DiffSyncModel) -> None: """Update a DiffSyncModel object to the store. Args: - obj (DiffSyncModel): Object to store + obj: Object to store Raises: ObjectAlreadyExists: if a different object with the same uid is already present. """ return self.store.update(obj=obj) - def remove(self, obj: DiffSyncModel, remove_children: bool = False): + def remove(self, obj: DiffSyncModel, remove_children: bool = False) -> None: """Remove a DiffSyncModel object from the store. Args: - obj (DiffSyncModel): object to remove - remove_children (bool): If True, also recursively remove any children of this object + obj: object to remove + remove_children: If True, also recursively remove any children of this object Raises: ObjectNotFound: if the object is not present @@ -791,12 +798,12 @@ def get_or_instantiate( """Attempt to get the object with provided identifiers 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. + model: The DiffSyncModel to get or create. + ids: Identifiers for the DiffSyncModel to get or create with. + attrs: Attributes when creating an object if it doesn't exist. Defaults to None. Returns: - Tuple[DiffSyncModel, bool]: Provides the existing or new object and whether it was created or not. + Provides the existing or new object and whether it was created or not. """ return self.store.get_or_instantiate(model=model, ids=ids, attrs=attrs) @@ -815,12 +822,12 @@ def update_or_instantiate(self, model: Type[DiffSyncModel], ids: Dict, attrs: Di """Attempt to update an existing object with provided ids/attrs or instantiate it with provided identifiers and attrs. Args: - model (DiffSyncModel): The DiffSyncModel to update or create. - ids (Dict): Identifiers for the DiffSyncModel to update or create with. - attrs (Dict): Attributes when creating/updating an object if it doesn't exist. Pass in empty dict, if no specific attrs. + model: The DiffSyncModel to update or create. + ids: Identifiers for the DiffSyncModel to update or create with. + attrs: 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. + Provides the existing or new object and whether it was created or not. """ return self.store.update_or_instantiate(model=model, ids=ids, attrs=attrs) @@ -828,21 +835,21 @@ def update_or_add_model_instance(self, obj: DiffSyncModel) -> Tuple[DiffSyncMode """Attempt to update an existing object with provided obj ids/attrs or instantiate obj. Args: - instance: An instance of the DiffSyncModel to update or create. + obj: An instance of the DiffSyncModel to update or create. Returns: Provides the existing or new object and whether it was created or not. """ return self.store.update_or_add_model_instance(obj=obj) - def count(self, model: Union[Text, "DiffSyncModel", Type["DiffSyncModel"], None] = None): + def count(self, model: Union[StrType, "DiffSyncModel", Type["DiffSyncModel"], None] = None) -> int: """Count how many objects of one model type exist in the backend store. Args: - model (DiffSyncModel): The DiffSyncModel to check the number of elements. If not provided, default to all. + model: The DiffSyncModel to check the number of elements. If not provided, default to all. Returns: - Int: Number of elements of the model type + Number of elements of the model type """ return self.store.count(model=model) diff --git a/diffsync/diff.py b/diffsync/diff.py index afeea436..c85117a6 100644 --- a/diffsync/diff.py +++ b/diffsync/diff.py @@ -16,40 +16,44 @@ """ from functools import total_ordering -from typing import Any, Iterator, Iterable, Mapping, Optional, Text, Type +from typing import Any, Iterator, Optional, Type, List, Dict, Iterable from .exceptions import ObjectAlreadyExists from .utils import intersection, OrderedDefaultDict from .enum import DiffSyncActions +# This workaround is used because we are defining a method called `str` in our class definition, which therefore renders +# the builtin `str` type unusable. +StrType = str + class Diff: """Diff Object, designed to store multiple DiffElement object and organize them in a group.""" - def __init__(self): + def __init__(self) -> None: """Initialize a new, empty Diff object.""" - self.children = OrderedDefaultDict(dict) + self.children = OrderedDefaultDict[StrType, Dict[StrType, DiffElement]](dict) """DefaultDict for storing DiffElement objects. `self.children[group][unique_id] == DiffElement(...)` """ self.models_processed = 0 - def __len__(self): + def __len__(self) -> int: """Total number of DiffElements stored herein.""" total = 0 for child in self.get_children(): total += len(child) return total - def complete(self): + def complete(self) -> None: """Method to call when this Diff has been fully populated with data and is "complete". The default implementation does nothing, but a subclass could use this, for example, to save the completed Diff to a file or database record. """ - def add(self, element: "DiffElement"): + def add(self, element: "DiffElement") -> None: """Add a new DiffElement to the changeset of this Diff. Raises: @@ -61,15 +65,15 @@ def add(self, element: "DiffElement"): self.children[element.type][element.name] = element - def groups(self): + def groups(self) -> List[StrType]: """Get the list of all group keys in self.children.""" - return self.children.keys() + return list(self.children.keys()) def has_diffs(self) -> bool: """Indicate if at least one of the child elements contains some diff. Returns: - bool: True if at least one child element contains some diff + True if at least one child element contains some diff """ for group in self.groups(): for child in self.children[group].values(): @@ -96,7 +100,7 @@ def get_children(self) -> Iterator["DiffElement"]: yield from order_method(self.children[group]) @classmethod - def order_children_default(cls, children: Mapping) -> Iterator["DiffElement"]: + def order_children_default(cls, children: Dict[StrType, "DiffElement"]) -> Iterator["DiffElement"]: """Default method to an Iterator for children. Since children is already an OrderedDefaultDict, this method is not doing anything special. @@ -104,7 +108,7 @@ def order_children_default(cls, children: Mapping) -> Iterator["DiffElement"]: for child in children.values(): yield child - def summary(self) -> Mapping[Text, int]: + def summary(self) -> Dict[StrType, int]: """Build a dict summary of this Diff and its child DiffElements.""" summary = { DiffSyncActions.CREATE: 0, @@ -127,7 +131,7 @@ def summary(self) -> Mapping[Text, int]: ) return summary - def str(self, indent: int = 0): + def str(self, indent: int = 0) -> StrType: """Build a detailed string representation of this Diff and its child DiffElements.""" margin = " " * indent output = [] @@ -144,9 +148,9 @@ def str(self, indent: int = 0): result = "(no diffs)" return result - def dict(self) -> Mapping[Text, Mapping[Text, Mapping]]: + def dict(self) -> Dict[StrType, Dict[StrType, Dict]]: """Build a dictionary representation of this Diff.""" - result = OrderedDefaultDict(dict) + result = OrderedDefaultDict[str, Dict](dict) for child in self.get_children(): if child.has_diffs(include_children=True): result[child.type][child.name] = child.dict() @@ -159,11 +163,11 @@ class DiffElement: # pylint: disable=too-many-instance-attributes def __init__( self, - obj_type: Text, - name: Text, - keys: Mapping, - source_name: Text = "source", - dest_name: Text = "dest", + obj_type: StrType, + name: StrType, + keys: Dict, + source_name: StrType = "source", + dest_name: StrType = "dest", diff_class: Type[Diff] = Diff, ): # pylint: disable=too-many-arguments """Instantiate a DiffElement. @@ -177,10 +181,10 @@ def __init__( dest_name: Name of the destination DiffSync object diff_class: Diff or subclass thereof to use to calculate the diffs to use for synchronization """ - if not isinstance(obj_type, str): + if not isinstance(obj_type, StrType): raise ValueError(f"obj_type must be a string (not {type(obj_type)})") - if not isinstance(name, str): + if not isinstance(name, StrType): raise ValueError(f"name must be a string (not {type(name)})") self.type = obj_type @@ -189,18 +193,18 @@ def __init__( self.source_name = source_name self.dest_name = dest_name # Note: *_attrs == None if no target object exists; it'll be an empty dict if it exists but has no _attributes - self.source_attrs: Optional[Mapping] = None - self.dest_attrs: Optional[Mapping] = None + self.source_attrs: Optional[Dict] = None + self.dest_attrs: Optional[Dict] = None self.child_diff = diff_class() - def __lt__(self, other): + def __lt__(self, other: "DiffElement") -> bool: """Logical ordering of DiffElements. Other comparison methods (__gt__, __le__, __ge__, etc.) are created by our use of the @total_ordering decorator. """ return (self.type, self.name) < (other.type, other.name) - def __eq__(self, other): + def __eq__(self, other: object) -> bool: """Logical equality of DiffElements. Other comparison methods (__gt__, __le__, __ge__, etc.) are created by our use of the @total_ordering decorator. @@ -216,14 +220,14 @@ def __eq__(self, other): # TODO also check that self.child_diff == other.child_diff, needs Diff to implement __eq__(). ) - def __str__(self): + def __str__(self) -> StrType: """Basic string representation of a DiffElement.""" return ( f'{self.type} "{self.name}" : {self.keys} : ' f"{self.source_name} → {self.dest_name} : {self.get_attrs_diffs()}" ) - def __len__(self): + def __len__(self) -> int: """Total number of DiffElements in this one, including itself.""" total = 1 # self for child in self.get_children(): @@ -231,11 +235,11 @@ def __len__(self): return total @property - def action(self) -> Optional[Text]: + def action(self) -> Optional[StrType]: """Action, if any, that should be taken to remediate the diffs described by this element. Returns: - str: DiffSyncActions ("create", "update", "delete", or None) + "create", "update", "delete", or None) """ if self.source_attrs is not None and self.dest_attrs is None: return DiffSyncActions.CREATE @@ -251,7 +255,7 @@ def action(self) -> Optional[Text]: return None # TODO: separate into set_source_attrs() and set_dest_attrs() methods, or just use direct property access instead? - def add_attrs(self, source: Optional[Mapping] = None, dest: Optional[Mapping] = None): + def add_attrs(self, source: Optional[Dict] = None, dest: Optional[Dict] = None) -> None: """Set additional attributes of a source and/or destination item that may result in diffs.""" # TODO: should source_attrs and dest_attrs be "write-once" properties, or is it OK to overwrite them once set? if source is not None: @@ -260,7 +264,7 @@ def add_attrs(self, source: Optional[Mapping] = None, dest: Optional[Mapping] = if dest is not None: self.dest_attrs = dest - def get_attrs_keys(self) -> Iterable[Text]: + def get_attrs_keys(self) -> Iterable[StrType]: """Get the list of shared attrs between source and dest, or the attrs of source or dest if only one is present. - If source_attrs is not set, return the keys of dest_attrs @@ -268,18 +272,18 @@ def get_attrs_keys(self) -> Iterable[Text]: - If both are defined, return the intersection of both keys """ if self.source_attrs is not None and self.dest_attrs is not None: - return intersection(self.dest_attrs.keys(), self.source_attrs.keys()) + return intersection(list(self.dest_attrs.keys()), list(self.source_attrs.keys())) if self.source_attrs is None and self.dest_attrs is not None: return self.dest_attrs.keys() if self.source_attrs is not None and self.dest_attrs is None: return self.source_attrs.keys() return [] - def get_attrs_diffs(self) -> Mapping[Text, Mapping[Text, Any]]: + def get_attrs_diffs(self) -> Dict[StrType, Dict[StrType, Any]]: """Get the dict of actual attribute diffs between source_attrs and dest_attrs. Returns: - dict: of the form `{"-": {key1: , key2: ...}, "+": {key1: , key2: ...}}`, + Dictionary of the form `{"-": {key1: , key2: ...}, "+": {key1: , key2: ...}}`, where the `"-"` or `"+"` dicts may be absent. """ if self.source_attrs is not None and self.dest_attrs is not None: @@ -301,13 +305,10 @@ def get_attrs_diffs(self) -> Mapping[Text, Mapping[Text, Any]]: return {"+": {key: self.source_attrs[key] for key in self.get_attrs_keys()}} return {} - def add_child(self, element: "DiffElement"): + def add_child(self, element: "DiffElement") -> None: """Attach a child object of type DiffElement. Childs are saved in a Diff object and are organized by type and name. - - Args: - element: DiffElement """ self.child_diff.add(element) @@ -336,7 +337,7 @@ def has_diffs(self, include_children: bool = True) -> bool: return False - def summary(self) -> Mapping[Text, int]: + def summary(self) -> Dict[StrType, int]: """Build a summary of this DiffElement and its children.""" summary = { DiffSyncActions.CREATE: 0, @@ -353,7 +354,7 @@ def summary(self) -> Mapping[Text, int]: summary[key] += child_summary[key] return summary - def str(self, indent: int = 0): + def str(self, indent: int = 0) -> StrType: """Build a detailed string representation of this DiffElement and its children.""" margin = " " * indent result = f"{margin}{self.type}: {self.name}" @@ -377,7 +378,7 @@ def str(self, indent: int = 0): result += " (no diffs)" return result - def dict(self) -> Mapping[Text, Mapping[Text, Any]]: + def dict(self) -> Dict[StrType, Dict[StrType, Any]]: """Build a dictionary representation of this DiffElement and its children.""" attrs_diffs = self.get_attrs_diffs() result = {} diff --git a/diffsync/exceptions.py b/diffsync/exceptions.py index b604c74e..fd02c0c8 100644 --- a/diffsync/exceptions.py +++ b/diffsync/exceptions.py @@ -14,6 +14,11 @@ See the License for the specific language governing permissions and limitations under the License. """ +from typing import TYPE_CHECKING, Union, Any + +if TYPE_CHECKING: + from diffsync import DiffSyncModel + from diffsync.diff import DiffElement class ObjectCrudException(Exception): @@ -39,7 +44,7 @@ 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): + def __init__(self, message: str, existing_object: Union["DiffSyncModel", "DiffElement"], *args: Any, **kwargs: Any): """Add existing_object to the exception to provide user with existing object.""" self.existing_object = existing_object super().__init__(message, existing_object, *args, **kwargs) diff --git a/diffsync/helpers.py b/diffsync/helpers.py index 8da7fd5c..96882bae 100644 --- a/diffsync/helpers.py +++ b/diffsync/helpers.py @@ -15,7 +15,7 @@ limitations under the License. """ from collections.abc import Iterable as ABCIterable, Mapping as ABCMapping -from typing import Callable, Iterable, List, Mapping, Optional, Tuple, Type, TYPE_CHECKING +from typing import Callable, List, Optional, Tuple, Type, TYPE_CHECKING, Dict, Iterable import structlog # type: ignore @@ -57,7 +57,7 @@ def __init__( # pylint: disable=too-many-arguments self.total_models = len(src_diffsync) + len(dst_diffsync) self.logger.debug(f"Diff calculation between these two datasets will involve {self.total_models} models") - def incr_models_processed(self, delta: int = 1): + def incr_models_processed(self, delta: int = 1) -> None: """Increment self.models_processed, then call self.callback if present.""" if delta: self.models_processed += delta @@ -136,7 +136,9 @@ def diff_object_list(self, src: List["DiffSyncModel"], dst: List["DiffSyncModel" return diff_elements @staticmethod - def validate_objects_for_diff(object_pairs: Iterable[Tuple[Optional["DiffSyncModel"], Optional["DiffSyncModel"]]]): + def validate_objects_for_diff( + object_pairs: Iterable[Tuple[Optional["DiffSyncModel"], Optional["DiffSyncModel"]]] + ) -> None: """Check whether all DiffSyncModels in the given dictionary are valid for comparison to one another. Helper method for `diff_object_list`. @@ -234,7 +236,7 @@ def diff_child_objects( diff_element: DiffElement, src_obj: Optional["DiffSyncModel"], dst_obj: Optional["DiffSyncModel"], - ): + ) -> DiffElement: """For all children of the given DiffSyncModel pair, diff recursively, adding diffs to the given diff_element. Helper method to `calculate_diffs`, usually doesn't need to be called directly. @@ -242,7 +244,7 @@ def diff_child_objects( These helper methods work in a recursive cycle: diff_object_list -> diff_object_pair -> diff_child_objects -> diff_object_list -> etc. """ - children_mapping: Mapping[str, str] + children_mapping: Dict[str, str] if src_obj and dst_obj: # Get the subset of child types common to both src_obj and dst_obj src_mapping = src_obj.get_children_mapping() @@ -308,7 +310,7 @@ def __init__( # pylint: disable=too-many-arguments self.model_class: Type["DiffSyncModel"] self.action: Optional[str] = None - def incr_elements_processed(self, delta: int = 1): + def incr_elements_processed(self, delta: int = 1) -> None: """Increment self.elements_processed, then call self.callback if present.""" if delta: self.elements_processed += delta @@ -319,7 +321,7 @@ def perform_sync(self) -> bool: """Perform data synchronization based on the provided diff. Returns: - bool: True if any changes were actually performed, else False. + True if any changes were actually performed, else False. """ changed = False self.base_logger.info("Beginning sync") @@ -401,14 +403,14 @@ def sync_diff_element(self, element: DiffElement, parent_model: Optional["DiffSy return changed def sync_model( # pylint: disable=too-many-branches, unused-argument - self, src_model: Optional["DiffSyncModel"], dst_model: Optional["DiffSyncModel"], ids: Mapping, attrs: Mapping + self, src_model: Optional["DiffSyncModel"], dst_model: Optional["DiffSyncModel"], ids: Dict, attrs: Dict ) -> Tuple[bool, Optional["DiffSyncModel"]]: """Create/update/delete the current DiffSyncModel with current ids/attrs, and update self.status and self.message. Helper method to `sync_diff_element`. Returns: - tuple: (changed, model) where model may be None if an error occurred + (changed, model) where model may be None if an error occurred """ if self.action is None: status = DiffSyncStatus.SUCCESS @@ -451,7 +453,7 @@ def sync_model( # pylint: disable=too-many-branches, unused-argument return (True, dst_model) - def log_sync_status(self, action: Optional[str], status: DiffSyncStatus, message: str): + def log_sync_status(self, action: Optional[str], status: DiffSyncStatus, message: str) -> None: """Log the current sync status at the appropriate verbosity with appropriate context. Helper method to `sync_diff_element`/`sync_model`. diff --git a/diffsync/logging.py b/diffsync/logging.py index f96cef4c..904d196e 100644 --- a/diffsync/logging.py +++ b/diffsync/logging.py @@ -22,7 +22,7 @@ from packaging import version -def enable_console_logging(verbosity=0): +def enable_console_logging(verbosity: int = 0) -> None: """Enable formatted logging to console with the specified verbosity. See https://www.structlog.org/en/stable/development.html as a reference @@ -52,7 +52,7 @@ def enable_console_logging(verbosity=0): processors.append(structlog.dev.ConsoleRenderer()) structlog.configure( - processors=processors, + processors=processors, # type: ignore[arg-type] context_class=dict, logger_factory=structlog.stdlib.LoggerFactory(), wrapper_class=structlog.stdlib.BoundLogger, @@ -60,7 +60,7 @@ def enable_console_logging(verbosity=0): ) -def _structlog_exception_formatter_required(): +def _structlog_exception_formatter_required() -> bool: """Determine if structlog exception formatter is needed. Return True if structlog exception formatter should be loaded diff --git a/diffsync/store/__init__.py b/diffsync/store/__init__.py index 0b750594..77f1a9f4 100644 --- a/diffsync/store/__init__.py +++ b/diffsync/store/__init__.py @@ -1,5 +1,5 @@ """BaseStore module.""" -from typing import Dict, List, Mapping, Text, Tuple, Type, Union, TYPE_CHECKING, Optional, Set +from typing import Dict, List, Tuple, Type, Union, TYPE_CHECKING, Optional, Set, Any import structlog # type: ignore from diffsync.exceptions import ObjectNotFound @@ -13,14 +13,18 @@ class BaseStore: """Reference store to be implemented in different backends.""" def __init__( - self, *args, diffsync: Optional["DiffSync"] = None, name: str = "", **kwargs # pylint: disable=unused-argument + self, # pylint: disable=unused-argument + *args: Any, # pylint: disable=unused-argument + diffsync: Optional["DiffSync"] = None, + name: str = "", + **kwargs: Any, # pylint: disable=unused-argument ) -> None: """Init method for BaseStore.""" self.diffsync = diffsync self.name = name or self.__class__.__name__ self._log = structlog.get_logger().new(store=self) - def __str__(self): + def __str__(self) -> str: """Render store name.""" return self.name @@ -28,12 +32,12 @@ def get_all_model_names(self) -> Set[str]: """Get all the model names stored. Return: - Set[str]: Set of all the model names. + Set of all the model names. """ raise NotImplementedError def get( - self, *, model: Union[Text, "DiffSyncModel", Type["DiffSyncModel"]], identifier: Union[Text, Mapping] + self, *, model: Union[str, "DiffSyncModel", Type["DiffSyncModel"]], identifier: Union[str, Dict] ) -> "DiffSyncModel": """Get one object from the data store based on its unique id. @@ -47,19 +51,19 @@ def get( """ raise NotImplementedError - def get_all(self, *, model: Union[Text, "DiffSyncModel", Type["DiffSyncModel"]]) -> List["DiffSyncModel"]: + def get_all(self, *, model: Union[str, "DiffSyncModel", Type["DiffSyncModel"]]) -> List["DiffSyncModel"]: """Get all objects of a given type. Args: model: DiffSyncModel class or instance, or modelname string, that defines the type of the objects to retrieve Returns: - List[DiffSyncModel]: List of Object + List of Object """ raise NotImplementedError def get_by_uids( - self, *, uids: List[Text], model: Union[Text, "DiffSyncModel", Type["DiffSyncModel"]] + self, *, uids: List[str], model: Union[str, "DiffSyncModel", Type["DiffSyncModel"]] ) -> List["DiffSyncModel"]: """Get multiple objects from the store by their unique IDs/Keys and type. @@ -72,16 +76,16 @@ def get_by_uids( """ raise NotImplementedError - def remove_item(self, modelname: str, uid: str): + def remove_item(self, modelname: str, uid: str) -> None: """Remove one item from store.""" raise NotImplementedError - def remove(self, *, obj: "DiffSyncModel", remove_children: bool = False): + def remove(self, *, obj: "DiffSyncModel", remove_children: bool = False) -> None: """Remove a DiffSyncModel object from the store. Args: - obj (DiffSyncModel): object to remove - remove_children (bool): If True, also recursively remove any children of this object + obj: object to remove + remove_children: If True, also recursively remove any children of this object Raises: ObjectNotFound: if the object is not present @@ -110,26 +114,26 @@ def remove(self, *, obj: "DiffSyncModel", remove_children: bool = False): parent_id=uid, ) - def add(self, *, obj: "DiffSyncModel"): + def add(self, *, obj: "DiffSyncModel") -> None: """Add a DiffSyncModel object to the store. Args: - obj (DiffSyncModel): Object to store + obj: Object to store Raises: ObjectAlreadyExists: if a different object with the same uid is already present. """ raise NotImplementedError - def update(self, *, obj: "DiffSyncModel"): + def update(self, *, obj: "DiffSyncModel") -> None: """Update a DiffSyncModel object to the store. Args: - obj (DiffSyncModel): Object to update + obj: Object to update """ raise NotImplementedError - def count(self, *, model: Union[Text, "DiffSyncModel", Type["DiffSyncModel"], None] = None) -> int: + def count(self, *, model: Union[str, "DiffSyncModel", Type["DiffSyncModel"], None] = None) -> int: """Returns the number of elements of a specific model, or all elements in the store if not specified.""" raise NotImplementedError @@ -139,12 +143,12 @@ def get_or_instantiate( """Attempt to get the object with provided identifiers 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. + model: The DiffSyncModel to get or create. + ids: Identifiers for the DiffSyncModel to get or create with. + attrs: Attributes when creating an object if it doesn't exist. Defaults to None. Returns: - Tuple[DiffSyncModel, bool]: Provides the existing or new object and whether it was created or not. + Provides the existing or new object and whether it was created or not. """ created = False try: @@ -183,12 +187,12 @@ def update_or_instantiate( """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 (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. + model: The DiffSyncModel to get or create. + ids: Identifiers for the DiffSyncModel to get or create with. + attrs: 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. + Provides the existing or new object and whether it was created or not. """ created = False try: @@ -210,7 +214,7 @@ def update_or_add_model_instance(self, obj: "DiffSyncModel") -> Tuple["DiffSyncM """Attempt to update an existing object with provided ids/attrs or instantiate obj. Args: - instance: An instance of the DiffSyncModel to update or create. + obj: An instance of the DiffSyncModel to update or create. Returns: Provides the existing or new object and whether it was added or not. @@ -234,7 +238,7 @@ def update_or_add_model_instance(self, obj: "DiffSyncModel") -> Tuple["DiffSyncM return obj, added def _get_object_class_and_model( - self, model: Union[Text, "DiffSyncModel", Type["DiffSyncModel"]] + self, model: Union[str, "DiffSyncModel", Type["DiffSyncModel"]] ) -> Tuple[Union["DiffSyncModel", Type["DiffSyncModel"], None], str]: """Get object class and model name for a model.""" if isinstance(model, str): @@ -250,9 +254,9 @@ def _get_object_class_and_model( @staticmethod def _get_uid( - model: Union[Text, "DiffSyncModel", Type["DiffSyncModel"]], + model: Union[str, "DiffSyncModel", Type["DiffSyncModel"]], object_class: Union["DiffSyncModel", Type["DiffSyncModel"], None], - identifier: Union[Text, Mapping], + identifier: Union[str, Dict], ) -> str: """Get the related uid for a model and an identifier.""" if isinstance(identifier, str): diff --git a/diffsync/store/local.py b/diffsync/store/local.py index eaf6fe5a..82bb69ba 100644 --- a/diffsync/store/local.py +++ b/diffsync/store/local.py @@ -1,7 +1,7 @@ """LocalStore module.""" from collections import defaultdict -from typing import List, Mapping, Text, Type, Union, TYPE_CHECKING, Dict, Set +from typing import List, Type, Union, TYPE_CHECKING, Dict, Set, Any from diffsync.exceptions import ObjectNotFound, ObjectAlreadyExists from diffsync.store import BaseStore @@ -14,7 +14,7 @@ class LocalStore(BaseStore): """LocalStore class.""" - def __init__(self, *args, **kwargs) -> None: + def __init__(self, *args: Any, **kwargs: Any) -> None: """Init method for LocalStore.""" super().__init__(*args, **kwargs) @@ -24,12 +24,12 @@ def get_all_model_names(self) -> Set[str]: """Get all the model names stored. Return: - Set[str]: Set of all the model names. + Set of all the model names. """ return set(self._data.keys()) def get( - self, *, model: Union[Text, "DiffSyncModel", Type["DiffSyncModel"]], identifier: Union[Text, Mapping] + self, *, model: Union[str, "DiffSyncModel", Type["DiffSyncModel"]], identifier: Union[str, Dict] ) -> "DiffSyncModel": """Get one object from the data store based on its unique id. @@ -49,14 +49,14 @@ def get( raise ObjectNotFound(f"{modelname} {uid} not present in {str(self)}") return self._data[modelname][uid] - def get_all(self, *, model: Union[Text, "DiffSyncModel", Type["DiffSyncModel"]]) -> List["DiffSyncModel"]: + def get_all(self, *, model: Union[str, "DiffSyncModel", Type["DiffSyncModel"]]) -> List["DiffSyncModel"]: """Get all objects of a given type. Args: model: DiffSyncModel class or instance, or modelname string, that defines the type of the objects to retrieve Returns: - List[DiffSyncModel]: List of Object + List of Object """ if isinstance(model, str): modelname = model @@ -66,7 +66,7 @@ def get_all(self, *, model: Union[Text, "DiffSyncModel", Type["DiffSyncModel"]]) return list(self._data[modelname].values()) def get_by_uids( - self, *, uids: List[Text], model: Union[Text, "DiffSyncModel", Type["DiffSyncModel"]] + self, *, uids: List[str], model: Union[str, "DiffSyncModel", Type["DiffSyncModel"]] ) -> List["DiffSyncModel"]: """Get multiple objects from the store by their unique IDs/Keys and type. @@ -89,11 +89,11 @@ def get_by_uids( results.append(self._data[modelname][uid]) return results - def add(self, *, obj: "DiffSyncModel"): + def add(self, *, obj: "DiffSyncModel") -> None: """Add a DiffSyncModel object to the store. Args: - obj (DiffSyncModel): Object to store + obj: Object to store Raises: ObjectAlreadyExists: if a different object with the same uid is already present. @@ -113,11 +113,11 @@ def add(self, *, obj: "DiffSyncModel"): self._data[modelname][uid] = obj - def update(self, *, obj: "DiffSyncModel"): + def update(self, *, obj: "DiffSyncModel") -> None: """Update a DiffSyncModel object to the store. Args: - obj (DiffSyncModel): Object to update + obj: Object to update """ modelname = obj.get_type() uid = obj.get_unique_id() @@ -128,13 +128,13 @@ def update(self, *, obj: "DiffSyncModel"): self._data[modelname][uid] = obj - def remove_item(self, modelname: str, uid: str): + def remove_item(self, modelname: str, uid: str) -> None: """Remove one item from store.""" if uid not in self._data[modelname]: raise ObjectNotFound(f"{modelname} {uid} not present in {str(self)}") del self._data[modelname][uid] - def count(self, *, model: Union[Text, "DiffSyncModel", Type["DiffSyncModel"], None] = None) -> int: + def count(self, *, model: Union[str, "DiffSyncModel", Type["DiffSyncModel"], None] = None) -> int: """Returns the number of elements of a specific model, or all elements in the store if unspecified.""" if not model: return sum(len(entries) for entries in self._data.values()) diff --git a/diffsync/store/redis.py b/diffsync/store/redis.py index 83ca68cb..927dbf5e 100644 --- a/diffsync/store/redis.py +++ b/diffsync/store/redis.py @@ -2,7 +2,7 @@ import copy import uuid from pickle import loads, dumps # nosec -from typing import List, Mapping, Text, Type, Union, TYPE_CHECKING, Set +from typing import List, Type, Union, TYPE_CHECKING, Set, Any, Optional, Dict try: from redis import Redis @@ -23,7 +23,16 @@ class RedisStore(BaseStore): """RedisStore class.""" - def __init__(self, *args, store_id=None, host=None, port=6379, url=None, db=0, **kwargs): + def __init__( + self, + *args: Any, + store_id: Optional[str] = None, + host: Optional[str] = None, + port: int = 6379, + url: Optional[str] = None, + db: int = 0, + **kwargs: Any, + ): """Init method for RedisStore.""" super().__init__(*args, **kwargs) @@ -33,11 +42,13 @@ def __init__(self, *args, store_id=None, host=None, port=6379, url=None, db=0, * try: if url: self._store = Redis.from_url(url, db=db) - else: + elif host: self._store = Redis(host=host, port=port, db=db) + else: + raise RedisConnectionError("Neither 'host' nor 'url' were specified.") if not self._store.ping(): - raise RedisConnectionError + raise RedisConnectionError() except RedisConnectionError: raise ObjectStoreException("Redis store is unavailable.") from RedisConnectionError @@ -45,24 +56,24 @@ def __init__(self, *args, store_id=None, host=None, port=6379, url=None, db=0, * self._store_label = f"{REDIS_DIFFSYNC_ROOT_LABEL}:{self._store_id}" - def __str__(self): + def __str__(self) -> str: """Render store name.""" return f"{self.name} ({self._store_id})" - def _get_object_from_redis_key(self, key): + def _get_object_from_redis_key(self, key: str) -> "DiffSyncModel": """Get the object from Redis key.""" - try: - obj_result = loads(self._store.get(key)) # nosec + pickled_object = self._store.get(key) + if pickled_object: + obj_result = loads(pickled_object) # nosec obj_result.diffsync = self.diffsync return obj_result - except TypeError as exc: - raise ObjectNotFound(f"{key} not present in Cache") from exc + raise ObjectNotFound(f"{key} not present in Cache") def get_all_model_names(self) -> Set[str]: """Get all the model names stored. Return: - Set[str]: Set of all the model names. + Set of all the model names. """ # TODO: optimize it all_model_names = set() @@ -74,11 +85,11 @@ def get_all_model_names(self) -> Set[str]: return all_model_names - def _get_key_for_object(self, modelname, uid): + def _get_key_for_object(self, modelname: str, uid: str) -> str: return f"{self._store_label}:{modelname}:{uid}" def get( - self, *, model: Union[Text, "DiffSyncModel", Type["DiffSyncModel"]], identifier: Union[Text, Mapping] + self, *, model: Union[str, "DiffSyncModel", Type["DiffSyncModel"]], identifier: Union[str, Dict] ) -> "DiffSyncModel": """Get one object from the data store based on its unique id. @@ -96,28 +107,28 @@ def get( return self._get_object_from_redis_key(self._get_key_for_object(modelname, uid)) - def get_all(self, *, model: Union[Text, "DiffSyncModel", Type["DiffSyncModel"]]) -> List["DiffSyncModel"]: + def get_all(self, *, model: Union[str, "DiffSyncModel", Type["DiffSyncModel"]]) -> List["DiffSyncModel"]: """Get all objects of a given type. Args: model: DiffSyncModel class or instance, or modelname string, that defines the type of the objects to retrieve Returns: - List[DiffSyncModel]: List of Object + List of Object """ if isinstance(model, str): modelname = model else: modelname = model.get_type() - results = [] + results: List["DiffSyncModel"] = [] for key in self._store.scan_iter(f"{self._store_label}:{modelname}:*"): - results.append(self._get_object_from_redis_key(key)) + results.append(self._get_object_from_redis_key(key)) # type: ignore[arg-type] return results def get_by_uids( - self, *, uids: List[Text], model: Union[Text, "DiffSyncModel", Type["DiffSyncModel"]] + self, *, uids: List[str], model: Union[str, "DiffSyncModel", Type["DiffSyncModel"]] ) -> List["DiffSyncModel"]: """Get multiple objects from the store by their unique IDs/Keys and type. @@ -139,11 +150,11 @@ def get_by_uids( return results - def add(self, *, obj: "DiffSyncModel"): + def add(self, *, obj: "DiffSyncModel") -> None: """Add a DiffSyncModel object to the store. Args: - obj (DiffSyncModel): Object to store + obj: Object to store Raises: ObjectAlreadyExists: if a different object with the same uid is already present. @@ -171,11 +182,11 @@ def add(self, *, obj: "DiffSyncModel"): self._store.set(object_key, dumps(obj_copy)) - def update(self, *, obj: "DiffSyncModel"): + def update(self, *, obj: "DiffSyncModel") -> None: """Update a DiffSyncModel object to the store. Args: - obj (DiffSyncModel): Object to update + obj: Object to update """ modelname = obj.get_type() uid = obj.get_unique_id() @@ -186,7 +197,7 @@ def update(self, *, obj: "DiffSyncModel"): self._store.set(object_key, dumps(obj_copy)) - def remove_item(self, modelname: str, uid: str): + def remove_item(self, modelname: str, uid: str) -> None: """Remove one item from store.""" object_key = self._get_key_for_object(modelname, uid) @@ -195,7 +206,7 @@ def remove_item(self, modelname: str, uid: str): self._store.delete(object_key) - def count(self, *, model: Union[Text, "DiffSyncModel", Type["DiffSyncModel"], None] = None) -> int: + def count(self, *, model: Union[str, "DiffSyncModel", Type["DiffSyncModel"], None] = None) -> int: """Returns the number of elements of a specific model, or all elements in the store if unspecified.""" search_pattern = f"{self._store_label}:*" if model is not None: diff --git a/diffsync/utils.py b/diffsync/utils.py index 94b0aa94..07462ff7 100644 --- a/diffsync/utils.py +++ b/diffsync/utils.py @@ -14,36 +14,41 @@ See the License for the specific language governing permissions and limitations under the License. """ - from collections import OrderedDict -from typing import Iterator, List, Dict, Optional +from typing import Iterator, List, Dict, Optional, TypeVar, Callable, Generic SPACE = " " BRANCH = "│ " TEE = "├── " LAST = "└── " +T = TypeVar("T") +K = TypeVar("K") +V = TypeVar("V") + -def intersection(lst1, lst2) -> List: +def intersection(lst1: List[T], lst2: List[T]) -> List[T]: """Calculate the intersection of two lists, with ordering based on the first list.""" lst3 = [value for value in lst1 if value in lst2] return lst3 -def symmetric_difference(lst1, lst2) -> List: +def symmetric_difference(lst1: List[T], lst2: List[T]) -> List[T]: """Calculate the symmetric difference of two lists.""" - return sorted(set(lst1) ^ set(lst2)) + # Type hinting this correct is kind of hard to do. _Probably_ the solution is using typing.Protocol to ensure the + # set members support the ^ operator / set.symmetric_difference, but I decided this wasn't worth figuring out. + return sorted(set(lst1) ^ set(lst2)) # type: ignore[type-var] -class OrderedDefaultDict(OrderedDict): +class OrderedDefaultDict(OrderedDict, Generic[K, V]): """A combination of collections.OrderedDict and collections.DefaultDict behavior.""" - def __init__(self, dict_type): + def __init__(self, dict_type: Callable[[], V]) -> None: """Create a new OrderedDefaultDict.""" self.factory = dict_type super().__init__(self) - def __missing__(self, key): + def __missing__(self, key: K) -> V: """When trying to access a nonexistent key, initialize the key value based on the internal factory.""" self[key] = value = self.factory() return value @@ -71,7 +76,7 @@ def tree_string(data: Dict, root: str) -> str: return "\n".join([root, *_tree(data)]) -def set_key(data: Dict, keys: List): +def set_key(data: Dict, keys: List) -> None: """Set a nested dictionary key given a list of keys.""" current_level = data for key in keys: diff --git a/poetry.lock b/poetry.lock index ecd569b1..e639d21d 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1572,6 +1572,17 @@ files = [ [package.dependencies] cryptography = ">=35.0.0" +[[package]] +name = "types-python-slugify" +version = "8.0.0.1" +description = "Typing stubs for python-slugify" +optional = false +python-versions = "*" +files = [ + {file = "types-python-slugify-8.0.0.1.tar.gz", hash = "sha256:f2177d2e65ecf2eca742d28fbf236a8d919a72e68272d1d748c3fe965e5ea3ab"}, + {file = "types_python_slugify-8.0.0.1-py3-none-any.whl", hash = "sha256:1de288ce45c85627ef632c2b5098dedccfc7ebedb241d181ba69402f03704449"}, +] + [[package]] name = "types-redis" version = "4.5.1.4" @@ -1587,6 +1598,20 @@ files = [ cryptography = ">=35.0.0" types-pyOpenSSL = "*" +[[package]] +name = "types-requests" +version = "2.28.11.15" +description = "Typing stubs for requests" +optional = false +python-versions = "*" +files = [ + {file = "types-requests-2.28.11.15.tar.gz", hash = "sha256:fc8eaa09cc014699c6b63c60c2e3add0c8b09a410c818b5ac6e65f92a26dde09"}, + {file = "types_requests-2.28.11.15-py3-none-any.whl", hash = "sha256:a05e4c7bc967518fba5789c341ea8b0c942776ee474c7873129a61161978e586"}, +] + +[package.dependencies] +types-urllib3 = "<1.27" + [[package]] name = "types-toml" version = "0.10.8.5" @@ -1598,6 +1623,17 @@ files = [ {file = "types_toml-0.10.8.5-py3-none-any.whl", hash = "sha256:2432017febe43174af0f3c65f03116e3d3cf43e7e1406b8200e106da8cf98992"}, ] +[[package]] +name = "types-urllib3" +version = "1.26.25.8" +description = "Typing stubs for urllib3" +optional = false +python-versions = "*" +files = [ + {file = "types-urllib3-1.26.25.8.tar.gz", hash = "sha256:ecf43c42d8ee439d732a1110b4901e9017a79a38daca26f08e42c8460069392c"}, + {file = "types_urllib3-1.26.25.8-py3-none-any.whl", hash = "sha256:95ea847fbf0bf675f50c8ae19a665baedcf07e6b4641662c4c3c72e7b2edf1a9"}, +] + [[package]] name = "typing-extensions" version = "4.5.0" @@ -1746,4 +1782,4 @@ redis = ["redis"] [metadata] lock-version = "2.0" python-versions = "^3.7" -content-hash = "37892839cc3207d00936d4f2d47780725bc0c299e1b1372a0bb4aba8907fe379" +content-hash = "2213a75a00f27293bb23f2a74792c2f8e46b97148e9197ddf620c9239d88b634" diff --git a/pyproject.toml b/pyproject.toml index e7191dae..22e1066d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,7 +26,7 @@ redis = {version = "^4.3", optional = true} [tool.poetry.extras] redis = ["redis"] -[tool.poetry.dev-dependencies] +[tool.poetry.group.dev.dependencies] pytest = "*" pyyaml = "*" black = "*" @@ -47,6 +47,8 @@ toml = "*" types-toml = "*" types-redis = "*" pytest-redis = "^2.4.0" +types-requests = "^2.28.11.15" +types-python-slugify = "^8.0.0.1" [tool.black] line-length = 120 @@ -101,6 +103,7 @@ testpaths = [ [tool.mypy] warn_unused_configs = true +disallow_untyped_defs = true ignore_missing_imports = true [build-system] diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 07b8fc8d..e0dd85c7 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. """ -from typing import ClassVar, List, Mapping, Optional, Tuple +from typing import ClassVar, List, Optional, Tuple, Dict import pytest @@ -35,7 +35,7 @@ class ErrorProneModelMixin: _counter: ClassVar[int] = 0 @classmethod - def create(cls, diffsync: DiffSync, ids: Mapping, attrs: Mapping): + def create(cls, diffsync: DiffSync, ids: Dict, attrs: Dict): """As DiffSyncModel.create(), but periodically throw exceptions.""" cls._counter += 1 if not cls._counter % 5: @@ -44,7 +44,7 @@ def create(cls, diffsync: DiffSync, ids: Mapping, attrs: Mapping): return None # non-fatal error return super().create(diffsync, ids, attrs) # type: ignore - def update(self, attrs: Mapping): + def update(self, attrs: Dict): """As DiffSyncModel.update(), but periodically throw exceptions.""" # pylint: disable=protected-access self.__class__._counter += 1 @@ -69,11 +69,11 @@ class ExceptionModelMixin: """Test class that always throws exceptions when creating/updating/deleting instances.""" @classmethod - def create(cls, diffsync: DiffSync, ids: Mapping, attrs: Mapping): + def create(cls, diffsync: DiffSync, ids: Dict, attrs: Dict): """As DiffSyncModel.create(), but always throw exceptions.""" raise NotImplementedError - def update(self, attrs: Mapping): + def update(self, attrs: Dict): """As DiffSyncModel.update(), but always throw exceptions.""" raise NotImplementedError