fix(netapp): unify logging format strings

Replace positional %s format strings with dictionary-based %(key)s
format in all NetApp module logging calls to fix literal format string
display

Author: skrobul

python/understack-workflows/tests/test_netapp_config.py

@@ -369,11 +369,14 @@ def test_config_with_extra_options(self):
             assert config.password == "test-password"
 
         os.unlink(f.name)
-    def test_integration_netapp_config_with_from_nautobot_response(self, config_with_nic_prefix):
-        """Test integration between NetAppConfig and NetappIPInterfaceConfig.from_nautobot_response."""
+
+    def test_integration_netapp_config_with_from_nautobot_response(
+        self, config_with_nic_prefix
+    ):
+        """Test integration between NetAppConfig and NetappIPInterfaceConfig."""
         from unittest.mock import MagicMock
+
         from understack_workflows.netapp.value_objects import NetappIPInterfaceConfig
-        import ipaddress
 
         # Create config with custom NIC prefix
         config = NetAppConfig(config_with_nic_prefix)
@@ -401,9 +404,11 @@ def test_integration_netapp_config_with_from_nautobot_response(self, config_with
         assert configs[1].base_port_name == "e5b"
         assert configs[0].nic_slot_prefix == "e5"
         assert configs[1].nic_slot_prefix == "e5"
+
     def test_from_nautobot_response_default_prefix(self, valid_config_file):
-        """Test that from_nautobot_response uses default prefix when no config provided."""
+        """Test that from_nautobot_response uses default when no config provided."""
         from unittest.mock import MagicMock
+
         from understack_workflows.netapp.value_objects import NetappIPInterfaceConfig
 
         # Create a mock nautobot response
@@ -423,7 +428,9 @@ def test_from_nautobot_response_default_prefix(self, valid_config_file):
 
         # Test with config that has default prefix
         config = NetAppConfig(valid_config_file)
-        configs_with_config = NetappIPInterfaceConfig.from_nautobot_response(mock_response, config)
+        configs_with_config = NetappIPInterfaceConfig.from_nautobot_response(
+            mock_response, config
+        )
         assert len(configs_with_config) == 1
         assert configs_with_config[0].base_port_name == "e4a"
-        assert configs_with_config[0].nic_slot_prefix == "e4"
+        assert configs_with_config[0].nic_slot_prefix == "e4"

python/understack-workflows/tests/test_netapp_error_handler.py

@@ -215,7 +215,8 @@ def test_log_warning_with_context(self, error_handler, mock_logger):
         error_handler.log_warning(message, context)
 
         mock_logger.warning.assert_called_once_with(
-            "%s - Context: %s", message, context
+            "%(message)s - Context: %(context)s",
+            {"message": message, "context": context},
         )
 
     def test_log_warning_without_context(self, error_handler, mock_logger):
@@ -233,7 +234,10 @@ def test_log_info_with_context(self, error_handler, mock_logger):
 
         error_handler.log_info(message, context)
 
-        mock_logger.info.assert_called_once_with("%s - Context: %s", message, context)
+        mock_logger.info.assert_called_once_with(
+            "%(message)s - Context: %(context)s",
+            {"message": message, "context": context},
+        )
 
     def test_log_info_without_context(self, error_handler, mock_logger):
         """Test logging info without context."""
@@ -250,7 +254,10 @@ def test_log_debug_with_context(self, error_handler, mock_logger):
 
         error_handler.log_debug(message, context)
 
-        mock_logger.debug.assert_called_once_with("%s - Context: %s", message, context)
+        mock_logger.debug.assert_called_once_with(
+            "%(message)s - Context: %(context)s",
+            {"message": message, "context": context},
+        )
 
     def test_log_debug_without_context(self, error_handler, mock_logger):
         """Test logging debug without context."""

python/understack-workflows/tests/test_netapp_manager.py

@@ -74,7 +74,9 @@ def test_create_volume_delegates_to_service(
     ):
         """Test create_volume delegates to VolumeService with correct parameters."""
         manager = NetAppManager(mock_config_file)
-        manager._volume_service.create_volume = MagicMock(return_value="vol_test-project")
+        manager._volume_service.create_volume = MagicMock(
+            return_value="vol_test-project"
+        )
 
         result = manager.create_volume("test-project", "1TB", "test-aggregate")
 
@@ -126,7 +128,9 @@ def test_delete_volume_standard_name_delegates_to_service(
         result = manager.delete_volume("vol_test-project", force=True)
 
         # Verify delegation extracts project_id correctly
-        manager._volume_service.delete_volume.assert_called_once_with("test-project", True)
+        manager._volume_service.delete_volume.assert_called_once_with(
+            "test-project", True
+        )
         assert result is True
 
     @patch("understack_workflows.netapp.manager.config")
@@ -158,7 +162,9 @@ def test_mapped_namespaces_standard_names_delegates_to_service(
         result = manager.mapped_namespaces("os-test-project", "vol_test-project")
 
         # Verify delegation with extracted project_id
-        manager._volume_service.get_mapped_namespaces.assert_called_once_with("test-project")
+        manager._volume_service.get_mapped_namespaces.assert_called_once_with(
+            "test-project"
+        )
         assert result == expected_namespaces
 
     @patch("understack_workflows.netapp.manager.config")
@@ -179,7 +185,9 @@ def test_create_lif_delegates_to_service(
 
         manager.create_lif("test-project", config_obj)
 
-        manager._lif_service.create_lif.assert_called_once_with("test-project", config_obj)
+        manager._lif_service.create_lif.assert_called_once_with(
+            "test-project", config_obj
+        )
 
     @patch("understack_workflows.netapp.manager.config")
     @patch("understack_workflows.netapp.manager.HostConnection")
@@ -205,6 +213,7 @@ def test_error_propagation_from_services(
 
         # Test SVM service error propagation
         from understack_workflows.netapp.exceptions import SvmOperationError
+
         manager._svm_service.create_svm = MagicMock(
             side_effect=SvmOperationError("SVM creation failed")
         )
@@ -214,6 +223,7 @@ def test_error_propagation_from_services(
 
         # Test Volume service error propagation
         from understack_workflows.netapp.exceptions import VolumeOperationError
+
         manager._volume_service.create_volume = MagicMock(
             side_effect=VolumeOperationError("Volume creation failed")
         )
@@ -240,7 +250,9 @@ def test_cleanup_project_orchestration(
         # Verify orchestration sequence
         manager._volume_service.exists.assert_called_once_with("test-project")
         manager._svm_service.exists.assert_called_once_with("test-project")
-        manager._volume_service.delete_volume.assert_called_once_with("test-project", force=True)
+        manager._volume_service.delete_volume.assert_called_once_with(
+            "test-project", force=True
+        )
         manager._svm_service.delete_svm.assert_called_once_with("test-project")
 
         assert result == {"volume": True, "svm": True}
@@ -262,7 +274,9 @@ def test_cleanup_project_volume_failure_stops_svm_deletion(
         result = manager.cleanup_project("test-project")
 
         # Verify SVM deletion was not attempted
-        manager._volume_service.delete_volume.assert_called_once_with("test-project", force=True)
+        manager._volume_service.delete_volume.assert_called_once_with(
+            "test-project", force=True
+        )
         manager._svm_service.delete_svm.assert_not_called()
 
         assert result == {"volume": False, "svm": False}

python/understack-workflows/tests/test_netapp_manager_integration.py

@@ -112,7 +112,9 @@ def test_cleanup_project_full_success_coordination(
         # Verify service coordination sequence
         manager._volume_service.exists.assert_called_once_with(project_id)
         manager._svm_service.exists.assert_called_once_with(project_id)
-        manager._volume_service.delete_volume.assert_called_once_with(project_id, force=True)
+        manager._volume_service.delete_volume.assert_called_once_with(
+            project_id, force=True
+        )
         manager._svm_service.delete_svm.assert_called_once_with(project_id)
 
         assert result == {"volume": True, "svm": True}
@@ -135,7 +137,9 @@ def test_cleanup_project_volume_failure_coordination(
         result = manager.cleanup_project(project_id)
 
         # Verify volume service was called but SVM service was not
-        manager._volume_service.delete_volume.assert_called_once_with(project_id, force=True)
+        manager._volume_service.delete_volume.assert_called_once_with(
+            project_id, force=True
+        )
         manager._svm_service.delete_svm.assert_not_called()
 
         assert result == {"volume": False, "svm": False}
@@ -158,7 +162,9 @@ def test_cleanup_project_partial_failure_coordination(
         result = manager.cleanup_project(project_id)
 
         # Verify both services were called
-        manager._volume_service.delete_volume.assert_called_once_with(project_id, force=True)
+        manager._volume_service.delete_volume.assert_called_once_with(
+            project_id, force=True
+        )
         manager._svm_service.delete_svm.assert_called_once_with(project_id)
 
         assert result == {"volume": True, "svm": False}
@@ -202,7 +208,9 @@ def test_cleanup_project_mixed_existence_scenarios(
 
         result = manager.cleanup_project("test-project-1")
 
-        manager._volume_service.delete_volume.assert_called_once_with("test-project-1", force=True)
+        manager._volume_service.delete_volume.assert_called_once_with(
+            "test-project-1", force=True
+        )
         manager._svm_service.delete_svm.assert_not_called()
         assert result == {"volume": True, "svm": True}
 
@@ -242,7 +250,8 @@ def test_cleanup_project_exception_handling_coordination(
         manager._svm_service.delete_svm.assert_not_called()
         assert result == {"volume": False, "svm": False}
 
-        # Test SVM service exception after successful volume deletion (new manager instance)
+        # Test SVM service exception after successful volume deletion (new
+        # manager instance)
         manager2 = NetAppManager(mock_config_file)
         manager2._volume_service.exists = MagicMock(return_value=True)
         manager2._volume_service.delete_volume = MagicMock(return_value=True)
@@ -254,7 +263,9 @@ def test_cleanup_project_exception_handling_coordination(
         result = manager2.cleanup_project(project_id)
 
         # Verify both services were called despite SVM failure
-        manager2._volume_service.delete_volume.assert_called_once_with(project_id, force=True)
+        manager2._volume_service.delete_volume.assert_called_once_with(
+            project_id, force=True
+        )
         manager2._svm_service.delete_svm.assert_called_once_with(project_id)
         assert result == {"volume": True, "svm": False}
 
@@ -268,15 +279,21 @@ def test_cleanup_project_existence_check_failures(
         project_id = "test-project-123"
 
         # Mock existence check failures
-        manager._volume_service.exists = MagicMock(side_effect=Exception("Connection error"))
-        manager._svm_service.exists = MagicMock(side_effect=Exception("Connection error"))
+        manager._volume_service.exists = MagicMock(
+            side_effect=Exception("Connection error")
+        )
+        manager._svm_service.exists = MagicMock(
+            side_effect=Exception("Connection error")
+        )
         manager._volume_service.delete_volume = MagicMock(return_value=True)
         manager._svm_service.delete_svm = MagicMock(return_value=True)
 
         result = manager.cleanup_project(project_id)
 
         # Verify cleanup still proceeds (assumes both exist when check fails)
-        manager._volume_service.delete_volume.assert_called_once_with(project_id, force=True)
+        manager._volume_service.delete_volume.assert_called_once_with(
+            project_id, force=True
+        )
         manager._svm_service.delete_svm.assert_called_once_with(project_id)
         assert result == {"volume": True, "svm": True}
 
@@ -295,7 +312,9 @@ def test_end_to_end_project_lifecycle(
 
         # Mock successful creation workflow
         manager._svm_service.create_svm = MagicMock(return_value=f"os-{project_id}")
-        manager._volume_service.create_volume = MagicMock(return_value=f"vol_{project_id}")
+        manager._volume_service.create_volume = MagicMock(
+            return_value=f"vol_{project_id}"
+        )
         manager._svm_service.exists = MagicMock(return_value=True)
         manager._volume_service.exists = MagicMock(return_value=True)
 
@@ -316,11 +335,15 @@ def test_end_to_end_project_lifecycle(
         assert cleanup_result == {"volume": True, "svm": True}
 
         # Verify all service interactions
-        manager._svm_service.create_svm.assert_called_once_with(project_id, "test-aggregate")
+        manager._svm_service.create_svm.assert_called_once_with(
+            project_id, "test-aggregate"
+        )
         manager._volume_service.create_volume.assert_called_once_with(
             project_id, "1TB", "test-aggregate"
         )
-        manager._volume_service.delete_volume.assert_called_once_with(project_id, force=True)
+        manager._volume_service.delete_volume.assert_called_once_with(
+            project_id, force=True
+        )
         manager._svm_service.delete_svm.assert_called_once_with(project_id)
 
     @patch("understack_workflows.netapp.manager.config")
@@ -361,12 +384,17 @@ def test_logging_coordination_across_services(
             result = manager.cleanup_project("test-project-123")
 
             # Verify appropriate log messages were called at manager level
-            mock_logger.info.assert_any_call("Starting cleanup for project: %s", "test-project-123")
             mock_logger.info.assert_any_call(
-                "Successfully deleted volume for project: %s", "test-project-123"
+                "Starting cleanup for project: %(project_id)s",
+                {"project_id": "test-project-123"},
+            )
+            mock_logger.info.assert_any_call(
+                "Successfully deleted volume for project: %(project_id)s",
+                {"project_id": "test-project-123"},
             )
             mock_logger.warning.assert_any_call(
-                "Failed to delete SVM for project: %s", "test-project-123"
+                "Failed to delete SVM for project: %(project_id)s",
+                {"project_id": "test-project-123"},
             )
 
         assert result == {"volume": True, "svm": False}
@@ -409,7 +437,10 @@ def test_backward_compatibility_maintained(
 
             # Network operations
             import ipaddress
-            from understack_workflows.netapp.value_objects import NetappIPInterfaceConfig
+
+            from understack_workflows.netapp.value_objects import (
+                NetappIPInterfaceConfig,
+            )
 
             config_obj = NetappIPInterfaceConfig(
                 name="test",

python/understack-workflows/understack_workflows/main/netapp_configure_net.py

@@ -352,7 +352,9 @@ def netapp_create_interfaces(
         Exception: If SVM for the project is not found
         NetAppRestError: If LIF creation fails on the NetApp system
     """
-    configs = NetappIPInterfaceConfig.from_nautobot_response(nautobot_response, mgr.config)
+    configs = NetappIPInterfaceConfig.from_nautobot_response(
+        nautobot_response, mgr.config
+    )
     for interface_config in configs:
         logger.info("Creating LIF %s for project %s", interface_config.name, project_id)
         mgr.create_lif(project_id, interface_config)