)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"a207196b10372c9e91f35d403efe2ff085045d1d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"95358bdd_5b6d3c11","updated":"2026-09-23 11:14:04.000000000","message":"change lgtm, some testing coverage would be useful","commit_id":"0e8544972101bec2cbe01da22ef601af551e114d"}],"watcher/decision_engine/datasources/manager.py":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"ec9f2ea16fdb2280a94227364c4e50faf43654db","unresolved":false,"context_lines":[{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        # Dynamically update grafana metric map, only available at runtime"},{"line_number":57,"context_line":"        # The metric map can still be overridden by a yaml config file"},{"line_number":58,"context_line":"        if self.datasources and graf.GrafanaHelper.NAME in self.datasources:"},{"line_number":59,"context_line":"            self.metric_map[graf.GrafanaHelper.NAME] \u003d self.grafana.METRIC_MAP"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"        metric_map_path \u003d cfg.CONF.watcher_decision_engine.metric_map_path"}],"source_content_type":"text/x-python","patch_set":1,"id":"2c609ea7_177dfab0","line":58,"updated":"2026-09-22 16:52:00.000000000","message":"The conditional at watcher/decision_engine/datasources/manager.py:58 is the entire fix for bug 2152623, yet no test asserts the negative path: that constructing DataSourceManager without \u0027grafana\u0027 in the configured datasources does not instantiate GrafanaHelper. watcher/tests/unit/decision_engine/datasources/test_manager.py only covers the grafana-configured direction (test_metric_file_metric_override_grafana, test_get_backend_grafana, test_dynamic_metric_map_grafana); tests without grafana (test_get_backend, test_metric_file_invalid_ds, test_datasource_validation_*) never assert instantiation behavior, and test_grafana_helper.py tests the helper itself rather than the manager\u0027s conditional.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: A future refactor (e.g., reordering __init__, renaming self.datasources, or reverting the condition) could reinstate eager GrafanaHelper construction, restoring the spurious \u0027GrafanaHelper authentication token not configured\u0027 CRITICAL log on every audit for non-Grafana deployments, with no CI failure to catch it.\n\n**Recommendation**:\nAdd a unit test to TestDataSourceManager that patches GrafanaHelper (e.g., @mock.patch.object(ds_manager.graf, \u0027GrafanaHelper\u0027, autospec\u003dTrue)) and constructs DataSourceManager with config datasources [\u0027gnocchi\u0027], asserting the helper class was never instantiated; optionally mirror it with an assertion that a manager configured with [\u0027grafana\u0027] does construct it.","commit_id":"0e8544972101bec2cbe01da22ef601af551e114d"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"a207196b10372c9e91f35d403efe2ff085045d1d","unresolved":true,"context_lines":[{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        # Dynamically update grafana metric map, only available at runtime"},{"line_number":57,"context_line":"        # The metric map can still be overridden by a yaml config file"},{"line_number":58,"context_line":"        if self.datasources and graf.GrafanaHelper.NAME in self.datasources:"},{"line_number":59,"context_line":"            self.metric_map[graf.GrafanaHelper.NAME] \u003d self.grafana.METRIC_MAP"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"        metric_map_path \u003d cfg.CONF.watcher_decision_engine.metric_map_path"}],"source_content_type":"text/x-python","patch_set":1,"id":"bb915876_b76ce94a","line":58,"in_reply_to":"2c609ea7_177dfab0","updated":"2026-09-23 11:14:04.000000000","message":"this seems valid, even though it\u0027s a simple change adding some test assertion to prevent a regression would be good","commit_id":"0e8544972101bec2cbe01da22ef601af551e114d"}]}