From 8284d870a5a227798797e038a48bdbf2f3c64e7a Mon Sep 17 00:00:00 2001 From: Damien Garros Date: Thu, 23 Dec 2021 16:59:31 -0500 Subject: [PATCH 1/3] Convert actions to enum --- diffsync/diff.py | 21 +++++++++++---------- diffsync/enum.py | 9 +++++++++ diffsync/helpers.py | 12 ++++++------ 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/diffsync/diff.py b/diffsync/diff.py index 412924ea..de153cef 100644 --- a/diffsync/diff.py +++ b/diffsync/diff.py @@ -20,6 +20,7 @@ from .exceptions import ObjectAlreadyExists from .utils import intersection, OrderedDefaultDict +from .enum import DiffSyncActions class Diff: @@ -105,9 +106,9 @@ def order_children_default(cls, children: Mapping) -> Iterator["DiffElement"]: def summary(self) -> Mapping[Text, int]: """Build a dict summary of this Diff and its child DiffElements.""" summary = { - "create": 0, - "update": 0, - "delete": 0, + DiffSyncActions.CREATE: 0, + DiffSyncActions.UPDATE: 0, + DiffSyncActions.DELETE: 0, "no-change": 0, } for child in self.get_children(): @@ -224,18 +225,18 @@ def action(self) -> Optional[Text]: """Action, if any, that should be taken to remediate the diffs described by this element. Returns: - str: "create", "update", "delete", or None + str: DiffSyncActions ("create", "update", "delete", or None) """ if self.source_attrs is not None and self.dest_attrs is None: - return "create" + return DiffSyncActions.CREATE if self.source_attrs is None and self.dest_attrs is not None: - return "delete" + return DiffSyncActions.DELETE if ( self.source_attrs is not None and self.dest_attrs is not None and any(self.source_attrs[attr_key] != self.dest_attrs[attr_key] for attr_key in self.get_attrs_keys()) ): - return "update" + return DiffSyncActions.UPDATE return None @@ -328,9 +329,9 @@ def has_diffs(self, include_children: bool = True) -> bool: def summary(self) -> Mapping[Text, int]: """Build a summary of this DiffElement and its children.""" summary = { - "create": 0, - "update": 0, - "delete": 0, + DiffSyncActions.CREATE: 0, + DiffSyncActions.UPDATE: 0, + DiffSyncActions.DELETE: 0, "no-change": 0, } if self.action: diff --git a/diffsync/enum.py b/diffsync/enum.py index 29c1046e..ce4d3ee7 100644 --- a/diffsync/enum.py +++ b/diffsync/enum.py @@ -73,3 +73,12 @@ class DiffSyncStatus(enum.Enum): SUCCESS = "success" FAILURE = "failure" ERROR = "error" + + +class DiffSyncActions: # pylint: disable=too-few-public-methods + """List of valid Action for DiffSyncModel.""" + + CREATE = "create" + UPDATE = "update" + DELETE = "delete" + NO_CHANGE = None diff --git a/diffsync/helpers.py b/diffsync/helpers.py index 9d509226..3a3ef3f9 100644 --- a/diffsync/helpers.py +++ b/diffsync/helpers.py @@ -20,7 +20,7 @@ import structlog # type: ignore from .diff import Diff, DiffElement -from .enum import DiffSyncModelFlags, DiffSyncFlags, DiffSyncStatus +from .enum import DiffSyncModelFlags, DiffSyncFlags, DiffSyncStatus, DiffSyncActions from .exceptions import ObjectNotFound, ObjectNotCreated, ObjectNotUpdated, ObjectNotDeleted, ObjectCrudException from .utils import intersection, symmetric_difference @@ -353,11 +353,11 @@ def sync_diff_element(self, element: DiffElement, parent_model: "DiffSyncModel" self.logger.warning("No object resulted from sync, will not process child objects.") return changed - if self.action == "create": + if self.action == DiffSyncActions.CREATE: if parent_model: parent_model.add_child(model) self.dst_diffsync.add(model) - elif self.action == "delete": + elif self.action == DiffSyncActions.DELETE: if parent_model: parent_model.remove_child(model) if model.model_flags & DiffSyncModelFlags.SKIP_CHILDREN_ON_DELETE: @@ -391,15 +391,15 @@ def sync_model( try: self.logger.debug(f"Attempting model {self.action}") - if self.action == "create": + if self.action == DiffSyncActions.CREATE: if model is not None: raise ObjectNotCreated(f"Failed to create {self.model_class.get_type()} {ids} - it already exists!") model = self.model_class.create(diffsync=self.dst_diffsync, ids=ids, attrs=attrs) - elif self.action == "update": + elif self.action == DiffSyncActions.UPDATE: if model is None: raise ObjectNotUpdated(f"Failed to update {self.model_class.get_type()} {ids} - not found!") model = model.update(attrs=attrs) - elif self.action == "delete": + elif self.action == DiffSyncActions.DELETE: if model is None: raise ObjectNotDeleted(f"Failed to delete {self.model_class.get_type()} {ids} - not found!") model = model.delete() From 0d9aa65e49b774000025d0522ff82375d6cb8d4d Mon Sep 17 00:00:00 2001 From: Damien Garros Date: Fri, 21 Jan 2022 13:59:05 -0500 Subject: [PATCH 2/3] Refactor to use proper Enum --- diffsync/diff.py | 18 +++++++++--------- diffsync/enum.py | 2 +- diffsync/helpers.py | 10 +++++----- tests/unit/test_diffsync.py | 4 ++-- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/diffsync/diff.py b/diffsync/diff.py index de153cef..70029c84 100644 --- a/diffsync/diff.py +++ b/diffsync/diff.py @@ -106,9 +106,9 @@ def order_children_default(cls, children: Mapping) -> Iterator["DiffElement"]: def summary(self) -> Mapping[Text, int]: """Build a dict summary of this Diff and its child DiffElements.""" summary = { - DiffSyncActions.CREATE: 0, - DiffSyncActions.UPDATE: 0, - DiffSyncActions.DELETE: 0, + DiffSyncActions.CREATE.value: 0, + DiffSyncActions.UPDATE.value: 0, + DiffSyncActions.DELETE.value: 0, "no-change": 0, } for child in self.get_children(): @@ -221,11 +221,11 @@ def __len__(self): return total @property - def action(self) -> Optional[Text]: + def action(self) -> Optional[DiffSyncActions]: """Action, if any, that should be taken to remediate the diffs described by this element. Returns: - str: DiffSyncActions ("create", "update", "delete", or None) + DiffSyncActions ("create", "update", "delete", or None) """ if self.source_attrs is not None and self.dest_attrs is None: return DiffSyncActions.CREATE @@ -329,13 +329,13 @@ def has_diffs(self, include_children: bool = True) -> bool: def summary(self) -> Mapping[Text, int]: """Build a summary of this DiffElement and its children.""" summary = { - DiffSyncActions.CREATE: 0, - DiffSyncActions.UPDATE: 0, - DiffSyncActions.DELETE: 0, + DiffSyncActions.CREATE.value: 0, + DiffSyncActions.UPDATE.value: 0, + DiffSyncActions.DELETE.value: 0, "no-change": 0, } if self.action: - summary[self.action] += 1 + summary[self.action.value] += 1 else: summary["no-change"] += 1 child_summary = self.child_diff.summary() diff --git a/diffsync/enum.py b/diffsync/enum.py index ce4d3ee7..5cd60ff7 100644 --- a/diffsync/enum.py +++ b/diffsync/enum.py @@ -75,7 +75,7 @@ class DiffSyncStatus(enum.Enum): ERROR = "error" -class DiffSyncActions: # pylint: disable=too-few-public-methods +class DiffSyncActions(enum.Enum): """List of valid Action for DiffSyncModel.""" CREATE = "create" diff --git a/diffsync/helpers.py b/diffsync/helpers.py index 3a3ef3f9..79edf1c6 100644 --- a/diffsync/helpers.py +++ b/diffsync/helpers.py @@ -296,7 +296,7 @@ def __init__( # pylint: disable=too-many-arguments # Local state maintained during synchronization self.logger: structlog.BoundLogger = self.base_logger self.model_class: Type["DiffSyncModel"] - self.action: Optional[str] = None + self.action: Optional[DiffSyncActions] = None def incr_elements_processed(self, delta: int = 1): """Increment self.elements_processed, then call self.callback if present.""" @@ -390,7 +390,7 @@ def sync_model( return (False, model) try: - self.logger.debug(f"Attempting model {self.action}") + self.logger.debug(f"Attempting model {self.action.value}") if self.action == DiffSyncActions.CREATE: if model is not None: raise ObjectNotCreated(f"Failed to create {self.model_class.get_type()} {ids} - it already exists!") @@ -404,13 +404,13 @@ def sync_model( raise ObjectNotDeleted(f"Failed to delete {self.model_class.get_type()} {ids} - not found!") model = model.delete() else: - raise ObjectCrudException(f'Unknown action "{self.action}"!') + raise ObjectCrudException(f'Unknown action "{self.action.value}"!') if model is not None: status, message = model.get_status() else: status = DiffSyncStatus.FAILURE - message = f"{self.model_class.get_type()} {self.action} did not return the model object." + message = f"{self.model_class.get_type()} {self.action.value} did not return the model object." except ObjectCrudException as exception: status = DiffSyncStatus.ERROR @@ -424,7 +424,7 @@ def sync_model( return (True, model) - def log_sync_status(self, action: Optional[str], status: DiffSyncStatus, message: str): + def log_sync_status(self, action: Optional[DiffSyncActions], status: DiffSyncStatus, message: str): """Log the current sync status at the appropriate verbosity with appropriate context. Helper method to `sync_diff_element`/`sync_model`. diff --git a/tests/unit/test_diffsync.py b/tests/unit/test_diffsync.py index 43068ecc..ea15da1e 100644 --- a/tests/unit/test_diffsync.py +++ b/tests/unit/test_diffsync.py @@ -619,7 +619,7 @@ def check_sync_logs_against_diff(diffsync, diff, log, errors_permitted=False): assert { ("action", element.action), - ("event", f"Attempting model {element.action}"), + ("event", f"Attempting model {element.action.value}"), ("level", "debug"), } <= begin_event.items() # attrs_diffs dict is unhashable so we can't include it in the above set comparison @@ -632,7 +632,7 @@ def check_sync_logs_against_diff(diffsync, diff, log, errors_permitted=False): if complete_event["status"] == "success": assert { ("action", element.action), - ("event", f"{element.action.title()}d successfully"), + ("event", f"{element.action.value.title()}d successfully"), ("level", "info"), ("status", "success"), } <= complete_event.items() From 443d8339cd4266ca8d6c9e1d5638dcac2f1bc101 Mon Sep 17 00:00:00 2001 From: Damien Garros Date: Fri, 21 Jan 2022 15:52:00 -0500 Subject: [PATCH 3/3] Remove value from ObjectCrudException exception message --- diffsync/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diffsync/helpers.py b/diffsync/helpers.py index 79edf1c6..11886407 100644 --- a/diffsync/helpers.py +++ b/diffsync/helpers.py @@ -404,7 +404,7 @@ def sync_model( raise ObjectNotDeleted(f"Failed to delete {self.model_class.get_type()} {ids} - not found!") model = model.delete() else: - raise ObjectCrudException(f'Unknown action "{self.action.value}"!') + raise ObjectCrudException(f'Unknown action "{self.action}"!') if model is not None: status, message = model.get_status()