Clean up logic in _on_config_changed event (canonical#285)

zxhdaze · Aug 8, 2024 · cd1055d · cd1055d
1 parent 4f41afd
commit cd1055d
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 21 deletions.
diff --git a/src/charm.py b/src/charm.py
@@ -231,6 +231,7 @@ def _on_config_changed(self, event: EventBase) -> None:
                 event.handle,
             )
             event.defer()
+            return
 
         if self.cos_agent_related:
             success, message = self.validate_configs()

diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
@@ -552,29 +552,34 @@ def test_config_changed(
 
             if not resource_installed:
                 mock_logger.info.assert_called()
-            if not cos_agent_related:
                 self.harness.charm.validate_configs.assert_not_called()
+                self.harness.charm._on_update_status.assert_not_called()
+            else:
+                if not cos_agent_related:
+                    self.harness.charm.validate_configs.assert_not_called()
+                    self.harness.charm._on_update_status.assert_called()
+                    return
+                if not validate_configs_return[0]:
+                    self.assertEqual(self.harness.charm.unit.status, BlockedStatus("invalid msg"))
+                    self.harness.charm.exporters[0].render_config.assert_not_called()
+                    return
+                if not all(mock_exporters_render_config_returns):
+                    for mock_exporter, render_config_return in zip(
+                        mock_exporters,
+                        mock_exporters_render_config_returns,
+                    ):
+                        if render_config_return:
+                            mock_exporter.restart.assert_called()
+                        else:
+                            message = (
+                                f"Failed to configure {mock_exporter.exporter_name}, "
+                                f"please check if the server is healthy."
+                            )
+                            self.assertEqual(
+                                self.harness.charm.unit.status, BlockedStatus(message)
+                            )
+                    self.harness.charm._on_update_status.assert_called()
                 self.harness.charm._on_update_status.assert_called()
-                return
-            if not validate_configs_return[0]:
-                self.assertEqual(self.harness.charm.unit.status, BlockedStatus("invalid msg"))
-                self.harness.charm.exporters[0].render_config.assert_not_called()
-                return
-            if not all(mock_exporters_render_config_returns):
-                for mock_exporter, render_config_return in zip(
-                    mock_exporters,
-                    mock_exporters_render_config_returns,
-                ):
-                    if render_config_return:
-                        mock_exporter.restart.assert_called()
-                    else:
-                        message = (
-                            f"Failed to configure {mock_exporter.exporter_name}, "
-                            f"please check if the server is healthy."
-                        )
-                        self.assertEqual(self.harness.charm.unit.status, BlockedStatus(message))
-                self.harness.charm._on_update_status.assert_called()
-            self.harness.charm._on_update_status.assert_called()
 
     def test_config_changed_update_alert_rules(self):
         """Test config changed will update alert rule."""