From cb413f44bcac92179b9bca5a4eaace6bc460a2a9 Mon Sep 17 00:00:00 2001 From: Adam Byczkowski <38091261+qduk@users.noreply.github.com> Date: Mon, 16 Sep 2024 14:27:15 -0500 Subject: [PATCH 1/8] Updated for dryrun bug --- nautobot_ssot/jobs/base.py | 92 ++++++++++++++++++++++++-------- nautobot_ssot/tests/test_jobs.py | 33 ++++++++++-- 2 files changed, 99 insertions(+), 26 deletions(-) diff --git a/nautobot_ssot/jobs/base.py b/nautobot_ssot/jobs/base.py index 9e087baed..95496c0a0 100644 --- a/nautobot_ssot/jobs/base.py +++ b/nautobot_ssot/jobs/base.py @@ -19,7 +19,9 @@ from nautobot_ssot.choices import SyncLogEntryActionChoices from nautobot_ssot.models import BaseModel, Sync, SyncLogEntry -DataMapping = namedtuple("DataMapping", ["source_name", "source_url", "target_name", "target_url"]) +DataMapping = namedtuple( + "DataMapping", ["source_name", "source_url", "target_name", "target_url"] +) """Entry in the list returned by a job's data_mappings() API. The idea here is to provide insight into how various classes of data are mapped between Nautobot @@ -45,8 +47,13 @@ class DataSyncBaseJob(Job): # pylint: disable=too-many-instance-attributes - `data_source_icon` and `data_target_icon` """ - dryrun = DryRunVar(description="Perform a dry-run, making no actual changes to Nautobot data.", default=True) - memory_profiling = BooleanVar(description="Perform a memory profiling analysis.", default=False) + dryrun = DryRunVar( + description="Perform a dry-run, making no actual changes to Nautobot data.", + default=True, + ) + memory_profiling = BooleanVar( + description="Perform a memory profiling analysis.", default=False + ) def load_source_adapter(self): """Method to instantiate and load the SOURCE adapter into `self.source_adapter`. @@ -72,7 +79,9 @@ def calculate_diff(self): This is a generic implementation that you could overwrite completely in your custom logic. """ if self.source_adapter is not None and self.target_adapter is not None: - self.diff = self.source_adapter.diff_to(self.target_adapter, flags=self.diffsync_flags) + self.diff = self.source_adapter.diff_to( + self.target_adapter, flags=self.diffsync_flags + ) self.sync.diff = {} self.sync.summary = self.diff.summary() self.sync.save() @@ -80,11 +89,15 @@ def calculate_diff(self): self.sync.diff = self.diff.dict() self.sync.save() except OperationalError: - self.logger.warning("Unable to save JSON diff to the database; likely the diff is too large.") + self.logger.warning( + "Unable to save JSON diff to the database; likely the diff is too large." + ) self.sync.refresh_from_db() self.logger.info(self.diff.summary()) else: - self.logger.warning("Not both adapters were properly initialized prior to diff calculation.") + self.logger.warning( + "Not both adapters were properly initialized prior to diff calculation." + ) def execute_sync(self): """Method to synchronize the difference from `self.diff`, from SOURCE to TARGET adapter. @@ -94,7 +107,9 @@ def execute_sync(self): if self.source_adapter is not None and self.target_adapter is not None: self.source_adapter.sync_to(self.target_adapter, flags=self.diffsync_flags) else: - self.logger.warning("Not both adapters were properly initialized prior to synchronization.") + self.logger.warning( + "Not both adapters were properly initialized prior to synchronization." + ) def sync_data(self, memory_profiling): """Method to load data from adapters, calculate diffs and sync (if not dry-run). @@ -117,10 +132,16 @@ def format_size(size): # pylint: disable=inconsistent-return-statements for unit in ("B", "KiB", "MiB", "GiB", "TiB"): if abs(size) < 100 and unit != "B": # 3 digits (xx.x UNIT) - return "%.1f %s" % (size, unit) # pylint: disable=consider-using-f-string + return "%.1f %s" % ( + size, + unit, + ) # pylint: disable=consider-using-f-string if abs(size) < 10 * 1024 or unit == "TiB": # 4 or 5 digits (xxxx UNIT) - return "%.0f %s" % (size, unit) # pylint: disable=consider-using-f-string + return "%.0f %s" % ( + size, + unit, + ) # pylint: disable=consider-using-f-string size /= 1024 def record_memory_trace(step: str): @@ -150,7 +171,11 @@ def record_memory_trace(step: str): load_source_adapter_time = datetime.now() self.sync.source_load_time = load_source_adapter_time - start_time self.sync.save() - self.logger.info("Source Load Time from %s: %s", self.source_adapter, self.sync.source_load_time) + self.logger.info( + "Source Load Time from %s: %s", + self.source_adapter, + self.sync.source_load_time, + ) if memory_profiling: record_memory_trace("source_load") @@ -159,7 +184,11 @@ def record_memory_trace(step: str): load_target_adapter_time = datetime.now() self.sync.target_load_time = load_target_adapter_time - load_source_adapter_time self.sync.save() - self.logger.info("Target Load Time from %s: %s", self.target_adapter, self.sync.target_load_time) + self.logger.info( + "Target Load Time from %s: %s", + self.target_adapter, + self.sync.target_load_time, + ) if memory_profiling: record_memory_trace("target_load") @@ -172,10 +201,12 @@ def record_memory_trace(step: str): if memory_profiling: record_memory_trace("diff") - if self.dryrun: + if self.sync.dry_run: self.logger.info("As `dryrun` is set, skipping the actual data sync.") else: - self.logger.info("Syncing from %s to %s...", self.source_adapter, self.target_adapter) + self.logger.info( + "Syncing from %s to %s...", self.source_adapter, self.target_adapter + ) self.execute_sync() execute_sync_time = datetime.now() self.sync.sync_time = execute_sync_time - calculate_diff_time @@ -185,7 +216,9 @@ def record_memory_trace(step: str): if memory_profiling: record_memory_trace("sync") - def lookup_object(self, model_name, unique_id) -> Optional[BaseModel]: # pylint: disable=unused-argument + def lookup_object( + self, model_name, unique_id # pylint: disable=unused-argument + ) -> Optional[BaseModel]: """Look up the Nautobot record, if any, identified by the args. Optional helper method used to build more detailed/accurate SyncLogEntry records from DiffSync logs. @@ -238,15 +271,23 @@ def sync_log( # pylint: disable=too-many-arguments def _structlog_to_sync_log_entry(self, _logger, _log_method, event_dict): """Capture certain structlog messages from DiffSync into the Nautobot database.""" - if all(key in event_dict for key in ("src", "dst", "action", "model", "unique_id", "diffs", "status")): + if all( + key in event_dict + for key in ("src", "dst", "action", "model", "unique_id", "diffs", "status") + ): # The DiffSync log gives us a model name (string) and unique_id (string). # Try to look up the actual Nautobot object that this describes. synced_object = self.lookup_object( # pylint: disable=assignment-from-none event_dict["model"], event_dict["unique_id"] ) - object_repr = repr(synced_object) if synced_object else f"{event_dict['model']} {event_dict['unique_id']}" + object_repr = ( + repr(synced_object) + if synced_object + else f"{event_dict['model']} {event_dict['unique_id']}" + ) self.sync_log( - action=event_dict["action"] or SyncLogEntryActionChoices.ACTION_NO_CHANGE, + action=event_dict["action"] + or SyncLogEntryActionChoices.ACTION_NO_CHANGE, diff=event_dict["diffs"] if event_dict["action"] else None, status=event_dict["status"], message=event_dict["event"], @@ -278,12 +319,16 @@ def __init__(self): self.source_adapter = None self.target_adapter = None # Default diffsync flags. You can overwrite them at any time. - self.diffsync_flags = DiffSyncFlags.CONTINUE_ON_FAILURE | DiffSyncFlags.LOG_UNCHANGED_RECORDS + self.diffsync_flags = ( + DiffSyncFlags.CONTINUE_ON_FAILURE | DiffSyncFlags.LOG_UNCHANGED_RECORDS + ) @classmethod def as_form(cls, data=None, files=None, initial=None, approval_view=False): """Render this instance as a Django form for user inputs, including a "Dry run" field.""" - form = super().as_form(data=data, files=files, initial=initial, approval_view=approval_view) + form = super().as_form( + data=data, files=files, initial=initial, approval_view=approval_view + ) # Set the "dryrun" widget's initial value based on our Meta attribute, if any form.fields["dryrun"].initial = getattr(cls.Meta, "dryrun_default", True) return form @@ -308,7 +353,9 @@ def data_target_icon(cls): """Icon corresponding to the data_target.""" return getattr(cls.Meta, "data_target_icon", None) - def run(self, dryrun, memory_profiling, *args, **kwargs): # pylint:disable=arguments-differ + def run( + self, dryrun, memory_profiling, *args, **kwargs + ): # pylint:disable=arguments-differ """Job entry point from Nautobot - do not override!""" self.sync = Sync.objects.create( source=self.data_source, @@ -321,7 +368,10 @@ def run(self, dryrun, memory_profiling, *args, **kwargs): # pylint:disable=argu # Add _structlog_to_sync_log_entry as a processor for structlog calls from DiffSync structlog.configure( - processors=[self._structlog_to_sync_log_entry, structlog.stdlib.render_to_log_kwargs], + processors=[ + self._structlog_to_sync_log_entry, + structlog.stdlib.render_to_log_kwargs, + ], context_class=dict, logger_factory=structlog.stdlib.LoggerFactory(), wrapper_class=structlog.stdlib.BoundLogger, diff --git a/nautobot_ssot/tests/test_jobs.py b/nautobot_ssot/tests/test_jobs.py index b4052b242..3c49a4fc8 100644 --- a/nautobot_ssot/tests/test_jobs.py +++ b/nautobot_ssot/tests/test_jobs.py @@ -1,7 +1,7 @@ """Test the Job classes in nautobot_ssot.""" import os.path -from unittest.mock import Mock, call +from unittest.mock import Mock, call, patch from django.db.utils import IntegrityError, OperationalError from django.test import override_settings @@ -90,9 +90,35 @@ def test_run(self): self.assertIsNone(self.job.sync.sync_time) self.assertEqual(self.job.sync.source, self.job.data_source) self.assertEqual(self.job.sync.target, self.job.data_target) - self.assertTrue(self.job.dryrun) + self.assertTrue(self.job.sync.dry_run) self.assertEqual(self.job.job_result, self.job.sync.job_result) + @patch.object(DataSyncBaseJob, "execute_sync") + def test_job_dryrun_false(self, mock_execute_sync): + """Test the job is ran in dryrun mode.""" + self.job.run(dryrun=False, memory_profiling=False) + self.assertFalse(self.job.sync.dry_run) + mock_execute_sync.assert_called() + + @patch.object(DataSyncBaseJob, "execute_sync") + def test_job_dryrun_true(self, mock_execute_sync): + """Test the job is ran in dryrun mode.""" + self.job.run(dryrun=True, memory_profiling=False) + self.assertTrue(self.job.sync.dry_run) + mock_execute_sync.assert_not_called() + + @patch("tracemalloc.start") + def test_job_memory_profiling_true(self, mock_malloc_start): + """Test the job is ran in dryrun mode.""" + self.job.run(dryrun=False, memory_profiling=True) + mock_malloc_start.assert_called() + + @patch("tracemalloc.start") + def test_job_memory_profiling_false(self, mock_malloc_start): + """Test the job is ran in dryrun mode.""" + self.job.run(dryrun=False, memory_profiling=False) + mock_malloc_start.assert_not_called() + def test_calculate_diff(self): """Test calculate_diff() method.""" self.job.sync = Mock() @@ -113,9 +139,6 @@ def test_calculate_diff_fail_diff_save_too_large(self): self.job.logger.warning = Mock() self.job.source_adapter.diff_to().dict.return_value = {} self.job.calculate_diff() - self.job.logger.warning.assert_any_call( - "Unable to save JSON diff to the database; likely the diff is too large." - ) def test_calculate_diff_fail_diff_save_generic(self): """Test calculate_diff() method logs failure.""" From b876d3244a28f901b103f7f1a407b44c41681b13 Mon Sep 17 00:00:00 2001 From: Adam Byczkowski <38091261+qduk@users.noreply.github.com> Date: Mon, 16 Sep 2024 14:39:41 -0500 Subject: [PATCH 2/8] Added changelog --- changes/548.fixed | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/548.fixed diff --git a/changes/548.fixed b/changes/548.fixed new file mode 100644 index 000000000..6b904abdc --- /dev/null +++ b/changes/548.fixed @@ -0,0 +1 @@ +Fixed SSoT jobs not respecting DryRun variable \ No newline at end of file From bb3e22e69dafe84b81816a89005b0ef5737a65fd Mon Sep 17 00:00:00 2001 From: Adam Byczkowski <38091261+qduk@users.noreply.github.com> Date: Thu, 26 Sep 2024 09:34:10 -0500 Subject: [PATCH 3/8] Update changes/548.fixed Co-authored-by: Gary Snider <75227981+gsnider2195@users.noreply.github.com> --- changes/548.fixed | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changes/548.fixed b/changes/548.fixed index 6b904abdc..05924edd1 100644 --- a/changes/548.fixed +++ b/changes/548.fixed @@ -1 +1 @@ -Fixed SSoT jobs not respecting DryRun variable \ No newline at end of file +Fixed SSoT jobs not respecting DryRun variable. \ No newline at end of file From ffe7690440811145e8de326a8b49c2089186e12f Mon Sep 17 00:00:00 2001 From: Adam Byczkowski <38091261+qduk@users.noreply.github.com> Date: Thu, 26 Sep 2024 09:49:45 -0500 Subject: [PATCH 4/8] Ruffed --- development/app_config_schema.py | 4 +-- development/nautobot_config.py | 12 ++----- nautobot_ssot/jobs/base.py | 58 +++++++++---------------------- nautobot_ssot/tests/test_basic.py | 18 +++------- nautobot_ssot/tests/test_jobs.py | 3 ++ tasks.py | 58 ++++++++----------------------- 6 files changed, 41 insertions(+), 112 deletions(-) diff --git a/development/app_config_schema.py b/development/app_config_schema.py index e52e24786..a779b14ef 100644 --- a/development/app_config_schema.py +++ b/development/app_config_schema.py @@ -40,9 +40,7 @@ def _main(): **SchemaBuilder().to_json_schema(app_config), # type: ignore } app_config = import_module(package_name).config - _enrich_object_schema( - schema, app_config.default_settings, app_config.required_settings - ) + _enrich_object_schema(schema, app_config.default_settings, app_config.required_settings) schema_path.write_text(json.dumps(schema, indent=4) + "\n") print(f"\n==================\nGenerated schema:\n\n{schema_path}\n") print( diff --git a/development/nautobot_config.py b/development/nautobot_config.py index 33bb80280..123ad7ec2 100644 --- a/development/nautobot_config.py +++ b/development/nautobot_config.py @@ -18,12 +18,8 @@ if "debug_toolbar" not in INSTALLED_APPS: # noqa: F405 INSTALLED_APPS.append("debug_toolbar") # noqa: F405 - if ( - "debug_toolbar.middleware.DebugToolbarMiddleware" not in MIDDLEWARE - ): # noqa: F405 - MIDDLEWARE.insert( - 0, "debug_toolbar.middleware.DebugToolbarMiddleware" - ) # noqa: F405 + if "debug_toolbar.middleware.DebugToolbarMiddleware" not in MIDDLEWARE: # noqa: F405 + MIDDLEWARE.insert(0, "debug_toolbar.middleware.DebugToolbarMiddleware") # noqa: F405 # # Misc. settings @@ -55,9 +51,7 @@ "NAUTOBOT_DB_PORT", default_db_settings[nautobot_db_engine]["NAUTOBOT_DB_PORT"], ), # Database port, default to postgres - "CONN_MAX_AGE": int( - os.getenv("NAUTOBOT_DB_TIMEOUT", "300") - ), # Database timeout + "CONN_MAX_AGE": int(os.getenv("NAUTOBOT_DB_TIMEOUT", "300")), # Database timeout "ENGINE": nautobot_db_engine, } } diff --git a/nautobot_ssot/jobs/base.py b/nautobot_ssot/jobs/base.py index 95496c0a0..25036a1f0 100644 --- a/nautobot_ssot/jobs/base.py +++ b/nautobot_ssot/jobs/base.py @@ -19,9 +19,7 @@ from nautobot_ssot.choices import SyncLogEntryActionChoices from nautobot_ssot.models import BaseModel, Sync, SyncLogEntry -DataMapping = namedtuple( - "DataMapping", ["source_name", "source_url", "target_name", "target_url"] -) +DataMapping = namedtuple("DataMapping", ["source_name", "source_url", "target_name", "target_url"]) """Entry in the list returned by a job's data_mappings() API. The idea here is to provide insight into how various classes of data are mapped between Nautobot @@ -51,9 +49,7 @@ class DataSyncBaseJob(Job): # pylint: disable=too-many-instance-attributes description="Perform a dry-run, making no actual changes to Nautobot data.", default=True, ) - memory_profiling = BooleanVar( - description="Perform a memory profiling analysis.", default=False - ) + memory_profiling = BooleanVar(description="Perform a memory profiling analysis.", default=False) def load_source_adapter(self): """Method to instantiate and load the SOURCE adapter into `self.source_adapter`. @@ -79,9 +75,7 @@ def calculate_diff(self): This is a generic implementation that you could overwrite completely in your custom logic. """ if self.source_adapter is not None and self.target_adapter is not None: - self.diff = self.source_adapter.diff_to( - self.target_adapter, flags=self.diffsync_flags - ) + self.diff = self.source_adapter.diff_to(self.target_adapter, flags=self.diffsync_flags) self.sync.diff = {} self.sync.summary = self.diff.summary() self.sync.save() @@ -89,15 +83,11 @@ def calculate_diff(self): self.sync.diff = self.diff.dict() self.sync.save() except OperationalError: - self.logger.warning( - "Unable to save JSON diff to the database; likely the diff is too large." - ) + self.logger.warning("Unable to save JSON diff to the database; likely the diff is too large.") self.sync.refresh_from_db() self.logger.info(self.diff.summary()) else: - self.logger.warning( - "Not both adapters were properly initialized prior to diff calculation." - ) + self.logger.warning("Not both adapters were properly initialized prior to diff calculation.") def execute_sync(self): """Method to synchronize the difference from `self.diff`, from SOURCE to TARGET adapter. @@ -107,9 +97,7 @@ def execute_sync(self): if self.source_adapter is not None and self.target_adapter is not None: self.source_adapter.sync_to(self.target_adapter, flags=self.diffsync_flags) else: - self.logger.warning( - "Not both adapters were properly initialized prior to synchronization." - ) + self.logger.warning("Not both adapters were properly initialized prior to synchronization.") def sync_data(self, memory_profiling): """Method to load data from adapters, calculate diffs and sync (if not dry-run). @@ -204,9 +192,7 @@ def record_memory_trace(step: str): if self.sync.dry_run: self.logger.info("As `dryrun` is set, skipping the actual data sync.") else: - self.logger.info( - "Syncing from %s to %s...", self.source_adapter, self.target_adapter - ) + self.logger.info("Syncing from %s to %s...", self.source_adapter, self.target_adapter) self.execute_sync() execute_sync_time = datetime.now() self.sync.sync_time = execute_sync_time - calculate_diff_time @@ -217,7 +203,9 @@ def record_memory_trace(step: str): record_memory_trace("sync") def lookup_object( - self, model_name, unique_id # pylint: disable=unused-argument + self, + model_name, + unique_id, # pylint: disable=unused-argument ) -> Optional[BaseModel]: """Look up the Nautobot record, if any, identified by the args. @@ -271,23 +259,15 @@ def sync_log( # pylint: disable=too-many-arguments def _structlog_to_sync_log_entry(self, _logger, _log_method, event_dict): """Capture certain structlog messages from DiffSync into the Nautobot database.""" - if all( - key in event_dict - for key in ("src", "dst", "action", "model", "unique_id", "diffs", "status") - ): + if all(key in event_dict for key in ("src", "dst", "action", "model", "unique_id", "diffs", "status")): # The DiffSync log gives us a model name (string) and unique_id (string). # Try to look up the actual Nautobot object that this describes. synced_object = self.lookup_object( # pylint: disable=assignment-from-none event_dict["model"], event_dict["unique_id"] ) - object_repr = ( - repr(synced_object) - if synced_object - else f"{event_dict['model']} {event_dict['unique_id']}" - ) + object_repr = repr(synced_object) if synced_object else f"{event_dict['model']} {event_dict['unique_id']}" self.sync_log( - action=event_dict["action"] - or SyncLogEntryActionChoices.ACTION_NO_CHANGE, + action=event_dict["action"] or SyncLogEntryActionChoices.ACTION_NO_CHANGE, diff=event_dict["diffs"] if event_dict["action"] else None, status=event_dict["status"], message=event_dict["event"], @@ -319,16 +299,12 @@ def __init__(self): self.source_adapter = None self.target_adapter = None # Default diffsync flags. You can overwrite them at any time. - self.diffsync_flags = ( - DiffSyncFlags.CONTINUE_ON_FAILURE | DiffSyncFlags.LOG_UNCHANGED_RECORDS - ) + self.diffsync_flags = DiffSyncFlags.CONTINUE_ON_FAILURE | DiffSyncFlags.LOG_UNCHANGED_RECORDS @classmethod def as_form(cls, data=None, files=None, initial=None, approval_view=False): """Render this instance as a Django form for user inputs, including a "Dry run" field.""" - form = super().as_form( - data=data, files=files, initial=initial, approval_view=approval_view - ) + form = super().as_form(data=data, files=files, initial=initial, approval_view=approval_view) # Set the "dryrun" widget's initial value based on our Meta attribute, if any form.fields["dryrun"].initial = getattr(cls.Meta, "dryrun_default", True) return form @@ -353,9 +329,7 @@ def data_target_icon(cls): """Icon corresponding to the data_target.""" return getattr(cls.Meta, "data_target_icon", None) - def run( - self, dryrun, memory_profiling, *args, **kwargs - ): # pylint:disable=arguments-differ + def run(self, dryrun, memory_profiling, *args, **kwargs): # pylint:disable=arguments-differ """Job entry point from Nautobot - do not override!""" self.sync = Sync.objects.create( source=self.data_source, diff --git a/nautobot_ssot/tests/test_basic.py b/nautobot_ssot/tests/test_basic.py index 83142face..d72f2d02e 100644 --- a/nautobot_ssot/tests/test_basic.py +++ b/nautobot_ssot/tests/test_basic.py @@ -11,21 +11,11 @@ class TestDocsPackaging(unittest.TestCase): def test_version(self): """Verify that pyproject.toml dev dependencies have the same versions as in the docs requirements.txt.""" - parent_path = os.path.dirname( - os.path.dirname(os.path.dirname(os.path.realpath(__file__))) - ) + parent_path = os.path.dirname(os.path.dirname(os.path.dirname(os.path.realpath(__file__)))) poetry_path = os.path.join(parent_path, "pyproject.toml") - poetry_details = toml.load(poetry_path)["tool"]["poetry"]["group"]["dev"][ - "dependencies" - ] - with open( - f"{parent_path}/docs/requirements.txt", "r", encoding="utf-8" - ) as file: - requirements = [ - line - for line in file.read().splitlines() - if (len(line) > 0 and not line.startswith("#")) - ] + poetry_details = toml.load(poetry_path)["tool"]["poetry"]["group"]["dev"]["dependencies"] + with open(f"{parent_path}/docs/requirements.txt", "r", encoding="utf-8") as file: + requirements = [line for line in file.read().splitlines() if (len(line) > 0 and not line.startswith("#"))] for pkg in requirements: package_name = pkg if len(pkg.split("==")) == 2: # noqa: PLR2004 diff --git a/nautobot_ssot/tests/test_jobs.py b/nautobot_ssot/tests/test_jobs.py index 3c49a4fc8..270b8f279 100644 --- a/nautobot_ssot/tests/test_jobs.py +++ b/nautobot_ssot/tests/test_jobs.py @@ -139,6 +139,9 @@ def test_calculate_diff_fail_diff_save_too_large(self): self.job.logger.warning = Mock() self.job.source_adapter.diff_to().dict.return_value = {} self.job.calculate_diff() + self.job.logger.warning.assert_any_call( + "Unable to save JSON diff to the database; likely the diff is too large." + ) def test_calculate_diff_fail_diff_save_generic(self): """Test calculate_diff() method logs failure.""" diff --git a/tasks.py b/tasks.py index 3006f401b..58a4484fd 100644 --- a/tasks.py +++ b/tasks.py @@ -72,9 +72,7 @@ def _is_compose_included(context, name): def _await_healthy_service(context, service): - container_id = docker_compose( - context, f"ps -q -- {service}", pty=False, echo=False, hide=True - ).stdout.strip() + container_id = docker_compose(context, f"ps -q -- {service}", pty=False, echo=False, hide=True).stdout.strip() _await_healthy_container(context, container_id) @@ -166,9 +164,7 @@ def docker_compose(context, command, **kwargs): ] for compose_file in context.nautobot_ssot.compose_files: - compose_file_path = os.path.join( - context.nautobot_ssot.compose_dir, compose_file - ) + compose_file_path = os.path.join(context.nautobot_ssot.compose_dir, compose_file) compose_command_tokens.append(f' -f "{compose_file_path}"') compose_command_tokens.append(command) @@ -244,20 +240,10 @@ def _get_docker_nautobot_version(context, nautobot_ver=None, python_ver=None): if python_ver is None: python_ver = context.nautobot_ssot.python_ver dockerfile_path = os.path.join(context.nautobot_ssot.compose_dir, "Dockerfile") - base_image = ( - context.run(f"grep --max-count=1 '^FROM ' {dockerfile_path}", hide=True) - .stdout.strip() - .split(" ")[1] - ) - base_image = base_image.replace(r"${NAUTOBOT_VER}", nautobot_ver).replace( - r"${PYTHON_VER}", python_ver - ) - pip_nautobot_ver = context.run( - f"docker run --rm --entrypoint '' {base_image} pip show nautobot", hide=True - ) - match_version = re.search( - r"^Version: (.+)$", pip_nautobot_ver.stdout.strip(), flags=re.MULTILINE - ) + base_image = context.run(f"grep --max-count=1 '^FROM ' {dockerfile_path}", hide=True).stdout.strip().split(" ")[1] + base_image = base_image.replace(r"${NAUTOBOT_VER}", nautobot_ver).replace(r"${PYTHON_VER}", python_ver) + pip_nautobot_ver = context.run(f"docker run --rm --entrypoint '' {base_image} pip show nautobot", hide=True) + match_version = re.search(r"^Version: (.+)$", pip_nautobot_ver.stdout.strip(), flags=re.MULTILINE) if match_version: return match_version.group(1) else: @@ -282,9 +268,7 @@ def _get_docker_nautobot_version(context, nautobot_ver=None, python_ver=None): ), } ) -def lock( - context, check=False, constrain_nautobot_ver=False, constrain_python_ver=False -): +def lock(context, check=False, constrain_nautobot_ver=False, constrain_python_ver=False): """Generate poetry.lock file.""" if constrain_nautobot_ver: docker_nautobot_version = _get_docker_nautobot_version(context) @@ -324,9 +308,7 @@ def restart(context, service=""): def stop(context, service=""): """Stop specified or all services, if service is not specified, remove all containers.""" print("Stopping Nautobot...") - docker_compose( - context, "stop" if service else "down --remove-orphans", service=service - ) + docker_compose(context, "stop" if service else "down --remove-orphans", service=service) @task( @@ -345,9 +327,7 @@ def destroy(context, volumes=True, import_db_file=""): return if not volumes: - raise ValueError( - "Cannot specify `--no-volumes` and `--import-db-file` arguments at the same time." - ) + raise ValueError("Cannot specify `--no-volumes` and `--import-db-file` arguments at the same time.") print(f"Importing database file: {import_db_file}...") @@ -364,16 +344,12 @@ def destroy(context, volumes=True, import_db_file=""): "db", ] - container_id = docker_compose( - context, " ".join(command), pty=False, echo=False, hide=True - ).stdout.strip() + container_id = docker_compose(context, " ".join(command), pty=False, echo=False, hide=True).stdout.strip() _await_healthy_container(context, container_id) print("Stopping database container...") context.run(f"docker stop {container_id}", pty=False, echo=False, hide=True) - print( - "Database import complete, you can start Nautobot with the following command:" - ) + print("Database import complete, you can start Nautobot with the following command:") print("invoke start") @@ -549,9 +525,7 @@ def dbshell(context, db_name="", input_file="", output_file="", query=""): if input_file and query: raise ValueError("Cannot specify both, `input_file` and `query` arguments") if output_file and not (input_file or query): - raise ValueError( - "`output_file` argument requires `input_file` or `query` argument" - ) + raise ValueError("`output_file` argument requires `input_file` or `query` argument") env = {} if query: @@ -689,9 +663,7 @@ def backup_db(context, db_name="", output_file="dump.sql", readable=True): docker_compose(context, " ".join(command), pty=False) print(50 * "=") - print( - "The database backup has been successfully completed and saved to the following file:" - ) + print("The database backup has been successfully completed and saved to the following file:") print(output_file) print("You can import this database backup with the following command:") print(f"invoke import-db --input-file '{output_file}'") @@ -860,9 +832,7 @@ def unittest( # noqa: PLR0913 @task def unittest_coverage(context): """Report on code test coverage as measured by 'invoke unittest'.""" - command = ( - "coverage report --skip-covered --include 'nautobot_ssot/*' --omit *migrations*" - ) + command = "coverage report --skip-covered --include 'nautobot_ssot/*' --omit *migrations*" run_command(context, command) From f63e88d21d2bba29412f422a712d856ce4122c13 Mon Sep 17 00:00:00 2001 From: Adam Byczkowski <38091261+qduk@users.noreply.github.com> Date: Thu, 26 Sep 2024 10:01:56 -0500 Subject: [PATCH 5/8] ruffed #2 --- nautobot_ssot/jobs/base.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/nautobot_ssot/jobs/base.py b/nautobot_ssot/jobs/base.py index 25036a1f0..5dfd849c6 100644 --- a/nautobot_ssot/jobs/base.py +++ b/nautobot_ssot/jobs/base.py @@ -120,16 +120,16 @@ def format_size(size): # pylint: disable=inconsistent-return-statements for unit in ("B", "KiB", "MiB", "GiB", "TiB"): if abs(size) < 100 and unit != "B": # 3 digits (xx.x UNIT) - return "%.1f %s" % ( + return "%.1f %s" % ( # pylint: disable=consider-using-f-string size, unit, - ) # pylint: disable=consider-using-f-string + ) if abs(size) < 10 * 1024 or unit == "TiB": # 4 or 5 digits (xxxx UNIT) - return "%.0f %s" % ( + return "%.0f %s" % ( # pylint: disable=consider-using-f-string size, unit, - ) # pylint: disable=consider-using-f-string + ) size /= 1024 def record_memory_trace(step: str): @@ -202,10 +202,10 @@ def record_memory_trace(step: str): if memory_profiling: record_memory_trace("sync") - def lookup_object( + def lookup_object( # pylint: disable=unused-argument self, model_name, - unique_id, # pylint: disable=unused-argument + unique_id, ) -> Optional[BaseModel]: """Look up the Nautobot record, if any, identified by the args. From fb3f7cc4ed7b1adea90d1670e0620e9cd7f7d37b Mon Sep 17 00:00:00 2001 From: Adam Byczkowski <38091261+qduk@users.noreply.github.com> Date: Fri, 27 Sep 2024 12:09:25 -0500 Subject: [PATCH 6/8] Fixed testing --- nautobot_ssot/jobs/base.py | 61 +++++++++++++++++++++++--------- nautobot_ssot/tests/test_jobs.py | 42 +++++++++++++++------- 2 files changed, 75 insertions(+), 28 deletions(-) diff --git a/nautobot_ssot/jobs/base.py b/nautobot_ssot/jobs/base.py index 5dfd849c6..52ef49430 100644 --- a/nautobot_ssot/jobs/base.py +++ b/nautobot_ssot/jobs/base.py @@ -19,7 +19,9 @@ from nautobot_ssot.choices import SyncLogEntryActionChoices from nautobot_ssot.models import BaseModel, Sync, SyncLogEntry -DataMapping = namedtuple("DataMapping", ["source_name", "source_url", "target_name", "target_url"]) +DataMapping = namedtuple( + "DataMapping", ["source_name", "source_url", "target_name", "target_url"] +) """Entry in the list returned by a job's data_mappings() API. The idea here is to provide insight into how various classes of data are mapped between Nautobot @@ -49,7 +51,9 @@ class DataSyncBaseJob(Job): # pylint: disable=too-many-instance-attributes description="Perform a dry-run, making no actual changes to Nautobot data.", default=True, ) - memory_profiling = BooleanVar(description="Perform a memory profiling analysis.", default=False) + memory_profiling = BooleanVar( + description="Perform a memory profiling analysis.", default=False + ) def load_source_adapter(self): """Method to instantiate and load the SOURCE adapter into `self.source_adapter`. @@ -75,7 +79,9 @@ def calculate_diff(self): This is a generic implementation that you could overwrite completely in your custom logic. """ if self.source_adapter is not None and self.target_adapter is not None: - self.diff = self.source_adapter.diff_to(self.target_adapter, flags=self.diffsync_flags) + self.diff = self.source_adapter.diff_to( + self.target_adapter, flags=self.diffsync_flags + ) self.sync.diff = {} self.sync.summary = self.diff.summary() self.sync.save() @@ -83,11 +89,15 @@ def calculate_diff(self): self.sync.diff = self.diff.dict() self.sync.save() except OperationalError: - self.logger.warning("Unable to save JSON diff to the database; likely the diff is too large.") + self.logger.warning( + "Unable to save JSON diff to the database; likely the diff is too large." + ) self.sync.refresh_from_db() self.logger.info(self.diff.summary()) else: - self.logger.warning("Not both adapters were properly initialized prior to diff calculation.") + self.logger.warning( + "Not both adapters were properly initialized prior to diff calculation." + ) def execute_sync(self): """Method to synchronize the difference from `self.diff`, from SOURCE to TARGET adapter. @@ -97,7 +107,9 @@ def execute_sync(self): if self.source_adapter is not None and self.target_adapter is not None: self.source_adapter.sync_to(self.target_adapter, flags=self.diffsync_flags) else: - self.logger.warning("Not both adapters were properly initialized prior to synchronization.") + self.logger.warning( + "Not both adapters were properly initialized prior to synchronization." + ) def sync_data(self, memory_profiling): """Method to load data from adapters, calculate diffs and sync (if not dry-run). @@ -120,13 +132,13 @@ def format_size(size): # pylint: disable=inconsistent-return-statements for unit in ("B", "KiB", "MiB", "GiB", "TiB"): if abs(size) < 100 and unit != "B": # 3 digits (xx.x UNIT) - return "%.1f %s" % ( # pylint: disable=consider-using-f-string + return "%.1f %s" % ( # pylint: disable=consider-using-f-string size, unit, ) if abs(size) < 10 * 1024 or unit == "TiB": # 4 or 5 digits (xxxx UNIT) - return "%.0f %s" % ( # pylint: disable=consider-using-f-string + return "%.0f %s" % ( # pylint: disable=consider-using-f-string size, unit, ) @@ -192,7 +204,10 @@ def record_memory_trace(step: str): if self.sync.dry_run: self.logger.info("As `dryrun` is set, skipping the actual data sync.") else: - self.logger.info("Syncing from %s to %s...", self.source_adapter, self.target_adapter) + self.logger.info( + "Syncing from %s to %s...", self.source_adapter, self.target_adapter + ) + print("I'm executing the sync now") self.execute_sync() execute_sync_time = datetime.now() self.sync.sync_time = execute_sync_time - calculate_diff_time @@ -202,7 +217,7 @@ def record_memory_trace(step: str): if memory_profiling: record_memory_trace("sync") - def lookup_object( # pylint: disable=unused-argument + def lookup_object( # pylint: disable=unused-argument self, model_name, unique_id, @@ -259,15 +274,23 @@ def sync_log( # pylint: disable=too-many-arguments def _structlog_to_sync_log_entry(self, _logger, _log_method, event_dict): """Capture certain structlog messages from DiffSync into the Nautobot database.""" - if all(key in event_dict for key in ("src", "dst", "action", "model", "unique_id", "diffs", "status")): + if all( + key in event_dict + for key in ("src", "dst", "action", "model", "unique_id", "diffs", "status") + ): # The DiffSync log gives us a model name (string) and unique_id (string). # Try to look up the actual Nautobot object that this describes. synced_object = self.lookup_object( # pylint: disable=assignment-from-none event_dict["model"], event_dict["unique_id"] ) - object_repr = repr(synced_object) if synced_object else f"{event_dict['model']} {event_dict['unique_id']}" + object_repr = ( + repr(synced_object) + if synced_object + else f"{event_dict['model']} {event_dict['unique_id']}" + ) self.sync_log( - action=event_dict["action"] or SyncLogEntryActionChoices.ACTION_NO_CHANGE, + action=event_dict["action"] + or SyncLogEntryActionChoices.ACTION_NO_CHANGE, diff=event_dict["diffs"] if event_dict["action"] else None, status=event_dict["status"], message=event_dict["event"], @@ -299,12 +322,16 @@ def __init__(self): self.source_adapter = None self.target_adapter = None # Default diffsync flags. You can overwrite them at any time. - self.diffsync_flags = DiffSyncFlags.CONTINUE_ON_FAILURE | DiffSyncFlags.LOG_UNCHANGED_RECORDS + self.diffsync_flags = ( + DiffSyncFlags.CONTINUE_ON_FAILURE | DiffSyncFlags.LOG_UNCHANGED_RECORDS + ) @classmethod def as_form(cls, data=None, files=None, initial=None, approval_view=False): """Render this instance as a Django form for user inputs, including a "Dry run" field.""" - form = super().as_form(data=data, files=files, initial=initial, approval_view=approval_view) + form = super().as_form( + data=data, files=files, initial=initial, approval_view=approval_view + ) # Set the "dryrun" widget's initial value based on our Meta attribute, if any form.fields["dryrun"].initial = getattr(cls.Meta, "dryrun_default", True) return form @@ -329,7 +356,9 @@ def data_target_icon(cls): """Icon corresponding to the data_target.""" return getattr(cls.Meta, "data_target_icon", None) - def run(self, dryrun, memory_profiling, *args, **kwargs): # pylint:disable=arguments-differ + def run( + self, dryrun, memory_profiling, *args, **kwargs + ): # pylint:disable=arguments-differ """Job entry point from Nautobot - do not override!""" self.sync = Sync.objects.create( source=self.data_source, diff --git a/nautobot_ssot/tests/test_jobs.py b/nautobot_ssot/tests/test_jobs.py index 270b8f279..de2f09cca 100644 --- a/nautobot_ssot/tests/test_jobs.py +++ b/nautobot_ssot/tests/test_jobs.py @@ -93,19 +93,37 @@ def test_run(self): self.assertTrue(self.job.sync.dry_run) self.assertEqual(self.job.job_result, self.job.sync.job_result) - @patch.object(DataSyncBaseJob, "execute_sync") - def test_job_dryrun_false(self, mock_execute_sync): + def test_job_dryrun_false(self): + """Test the job is not ran in dryrun mode.""" + with patch.object(DataSyncBaseJob, "execute_sync") as mock_execute_sync: + isolated_job = DataSyncBaseJob() + + isolated_job.job_result = JobResult.objects.create( + name="fake job no dryrun", + task_name="fake job no dryrun", + worker="default", + ) + isolated_job.load_source_adapter = lambda *x, **y: None + isolated_job.load_target_adapter = lambda *x, **y: None + isolated_job.run(dryrun=False, memory_profiling=False) + self.assertFalse(isolated_job.sync.dry_run) + mock_execute_sync.assert_called() + + def test_job_dryrun_true(self): """Test the job is ran in dryrun mode.""" - self.job.run(dryrun=False, memory_profiling=False) - self.assertFalse(self.job.sync.dry_run) - mock_execute_sync.assert_called() - - @patch.object(DataSyncBaseJob, "execute_sync") - def test_job_dryrun_true(self, mock_execute_sync): - """Test the job is ran in dryrun mode.""" - self.job.run(dryrun=True, memory_profiling=False) - self.assertTrue(self.job.sync.dry_run) - mock_execute_sync.assert_not_called() + with patch.object(DataSyncBaseJob, "execute_sync") as mock_execute_sync: + isolated_job = DataSyncBaseJob() + + isolated_job.job_result = JobResult.objects.create( + name="fake job", + task_name="fake job", + worker="default", + ) + isolated_job.load_source_adapter = lambda *x, **y: None + isolated_job.load_target_adapter = lambda *x, **y: None + isolated_job.run(dryrun=True, memory_profiling=False) + self.assertTrue(isolated_job.sync.dry_run) + mock_execute_sync.assert_not_called() @patch("tracemalloc.start") def test_job_memory_profiling_true(self, mock_malloc_start): From 4bb2b03b06cd30b1cb504d300887e3028ad4a2e2 Mon Sep 17 00:00:00 2001 From: Adam Byczkowski <38091261+qduk@users.noreply.github.com> Date: Fri, 27 Sep 2024 12:17:58 -0500 Subject: [PATCH 7/8] Fixed test --- nautobot_ssot/jobs/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nautobot_ssot/jobs/base.py b/nautobot_ssot/jobs/base.py index 52ef49430..18f8c3798 100644 --- a/nautobot_ssot/jobs/base.py +++ b/nautobot_ssot/jobs/base.py @@ -111,7 +111,7 @@ def execute_sync(self): "Not both adapters were properly initialized prior to synchronization." ) - def sync_data(self, memory_profiling): + def sync_data(self, memory_profiling): # pylint: disable=too-many-statements """Method to load data from adapters, calculate diffs and sync (if not dry-run). It is composed by 4 methods: From b6af59137a6b2ec485c3bf828e510f4f18189c29 Mon Sep 17 00:00:00 2001 From: Adam Byczkowski <38091261+qduk@users.noreply.github.com> Date: Fri, 27 Sep 2024 13:05:41 -0500 Subject: [PATCH 8/8] ruffed --- nautobot_ssot/jobs/base.py | 54 +++++++++----------------------------- 1 file changed, 13 insertions(+), 41 deletions(-) diff --git a/nautobot_ssot/jobs/base.py b/nautobot_ssot/jobs/base.py index 18f8c3798..972f8952d 100644 --- a/nautobot_ssot/jobs/base.py +++ b/nautobot_ssot/jobs/base.py @@ -19,9 +19,7 @@ from nautobot_ssot.choices import SyncLogEntryActionChoices from nautobot_ssot.models import BaseModel, Sync, SyncLogEntry -DataMapping = namedtuple( - "DataMapping", ["source_name", "source_url", "target_name", "target_url"] -) +DataMapping = namedtuple("DataMapping", ["source_name", "source_url", "target_name", "target_url"]) """Entry in the list returned by a job's data_mappings() API. The idea here is to provide insight into how various classes of data are mapped between Nautobot @@ -51,9 +49,7 @@ class DataSyncBaseJob(Job): # pylint: disable=too-many-instance-attributes description="Perform a dry-run, making no actual changes to Nautobot data.", default=True, ) - memory_profiling = BooleanVar( - description="Perform a memory profiling analysis.", default=False - ) + memory_profiling = BooleanVar(description="Perform a memory profiling analysis.", default=False) def load_source_adapter(self): """Method to instantiate and load the SOURCE adapter into `self.source_adapter`. @@ -79,9 +75,7 @@ def calculate_diff(self): This is a generic implementation that you could overwrite completely in your custom logic. """ if self.source_adapter is not None and self.target_adapter is not None: - self.diff = self.source_adapter.diff_to( - self.target_adapter, flags=self.diffsync_flags - ) + self.diff = self.source_adapter.diff_to(self.target_adapter, flags=self.diffsync_flags) self.sync.diff = {} self.sync.summary = self.diff.summary() self.sync.save() @@ -89,15 +83,11 @@ def calculate_diff(self): self.sync.diff = self.diff.dict() self.sync.save() except OperationalError: - self.logger.warning( - "Unable to save JSON diff to the database; likely the diff is too large." - ) + self.logger.warning("Unable to save JSON diff to the database; likely the diff is too large.") self.sync.refresh_from_db() self.logger.info(self.diff.summary()) else: - self.logger.warning( - "Not both adapters were properly initialized prior to diff calculation." - ) + self.logger.warning("Not both adapters were properly initialized prior to diff calculation.") def execute_sync(self): """Method to synchronize the difference from `self.diff`, from SOURCE to TARGET adapter. @@ -107,9 +97,7 @@ def execute_sync(self): if self.source_adapter is not None and self.target_adapter is not None: self.source_adapter.sync_to(self.target_adapter, flags=self.diffsync_flags) else: - self.logger.warning( - "Not both adapters were properly initialized prior to synchronization." - ) + self.logger.warning("Not both adapters were properly initialized prior to synchronization.") def sync_data(self, memory_profiling): # pylint: disable=too-many-statements """Method to load data from adapters, calculate diffs and sync (if not dry-run). @@ -204,9 +192,7 @@ def record_memory_trace(step: str): if self.sync.dry_run: self.logger.info("As `dryrun` is set, skipping the actual data sync.") else: - self.logger.info( - "Syncing from %s to %s...", self.source_adapter, self.target_adapter - ) + self.logger.info("Syncing from %s to %s...", self.source_adapter, self.target_adapter) print("I'm executing the sync now") self.execute_sync() execute_sync_time = datetime.now() @@ -274,23 +260,15 @@ def sync_log( # pylint: disable=too-many-arguments def _structlog_to_sync_log_entry(self, _logger, _log_method, event_dict): """Capture certain structlog messages from DiffSync into the Nautobot database.""" - if all( - key in event_dict - for key in ("src", "dst", "action", "model", "unique_id", "diffs", "status") - ): + if all(key in event_dict for key in ("src", "dst", "action", "model", "unique_id", "diffs", "status")): # The DiffSync log gives us a model name (string) and unique_id (string). # Try to look up the actual Nautobot object that this describes. synced_object = self.lookup_object( # pylint: disable=assignment-from-none event_dict["model"], event_dict["unique_id"] ) - object_repr = ( - repr(synced_object) - if synced_object - else f"{event_dict['model']} {event_dict['unique_id']}" - ) + object_repr = repr(synced_object) if synced_object else f"{event_dict['model']} {event_dict['unique_id']}" self.sync_log( - action=event_dict["action"] - or SyncLogEntryActionChoices.ACTION_NO_CHANGE, + action=event_dict["action"] or SyncLogEntryActionChoices.ACTION_NO_CHANGE, diff=event_dict["diffs"] if event_dict["action"] else None, status=event_dict["status"], message=event_dict["event"], @@ -322,16 +300,12 @@ def __init__(self): self.source_adapter = None self.target_adapter = None # Default diffsync flags. You can overwrite them at any time. - self.diffsync_flags = ( - DiffSyncFlags.CONTINUE_ON_FAILURE | DiffSyncFlags.LOG_UNCHANGED_RECORDS - ) + self.diffsync_flags = DiffSyncFlags.CONTINUE_ON_FAILURE | DiffSyncFlags.LOG_UNCHANGED_RECORDS @classmethod def as_form(cls, data=None, files=None, initial=None, approval_view=False): """Render this instance as a Django form for user inputs, including a "Dry run" field.""" - form = super().as_form( - data=data, files=files, initial=initial, approval_view=approval_view - ) + form = super().as_form(data=data, files=files, initial=initial, approval_view=approval_view) # Set the "dryrun" widget's initial value based on our Meta attribute, if any form.fields["dryrun"].initial = getattr(cls.Meta, "dryrun_default", True) return form @@ -356,9 +330,7 @@ def data_target_icon(cls): """Icon corresponding to the data_target.""" return getattr(cls.Meta, "data_target_icon", None) - def run( - self, dryrun, memory_profiling, *args, **kwargs - ): # pylint:disable=arguments-differ + def run(self, dryrun, memory_profiling, *args, **kwargs): # pylint:disable=arguments-differ """Job entry point from Nautobot - do not override!""" self.sync = Sync.objects.create( source=self.data_source,