)]}'
{"watcher/decision_engine/datasources/base.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":"f5f5275fe425d99eb4a4b9a02b70cce8295b8d6d","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        pass"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    @abc.abstractmethod"},{"line_number":130,"context_line":"    def _statistic_aggregation("},{"line_number":131,"context_line":"        self,"},{"line_number":132,"context_line":"        resource\u003dNone,"},{"line_number":133,"context_line":"        resource_type\u003dNone,"}],"source_content_type":"text/x-python","patch_set":1,"id":"968b5e98_b433076c","line":130,"updated":"2026-06-26 18:36:15.000000000","message":"Missing reno release note for a breaking public API change: the abstract statistic_aggregation was renamed to _statistic_aggregation. Any out-of-tree subclass overriding statistic_aggregation now has its override silently bypassed, as the concrete wrapper delegates only to _statistic_aggregation.\n\n**Severity**: HIGH | **Confidence**: 0.8\n\n**Risk**: Out-of-tree datasource plugins (and any downstream subclass) break silently: their statistic_aggregation override is no longer called, the cache miss path invokes the undefined _statistic_aggregation, and they get no signal at import time because DataSourceBase lacks an ABCMeta metaclass.\n\n**Priority**: Before merge\n**Why This Matters**: Watcher datasources are a documented plugin extension point. A silent override-bypass is one of the worst failure modes for downstream integrators because it produces wrong/empty metrics without errors. An upgrade note is the minimum courtesy; ideally add a temporary compatibility shim.\n\n**Recommendation**:\nAdd a reno note (releasenotes/notes/...) describing the rename, that subclasses must now implement _statistic_aggregation instead of statistic_aggregation, and the new caching/inject_metric behavior. If feasible, keep a transitional check: if a subclass defines statistic_aggregation but not _statistic_aggregation, log a deprecation warning and delegate to the old method for one release.","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"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":"f5f5275fe425d99eb4a4b9a02b70cce8295b8d6d","unresolved":false,"context_lines":[{"line_number":189,"context_line":"                aggregate\u003daggregate,"},{"line_number":190,"context_line":"                granularity\u003dgranularity,"},{"line_number":191,"context_line":"            )"},{"line_number":192,"context_line":"        cache_key \u003d (resource_uuid, meter_name, aggregate, period)"},{"line_number":193,"context_line":"        if cache_key in self._metric_cache:"},{"line_number":194,"context_line":"            return self._metric_cache[cache_key]"},{"line_number":195,"context_line":"        value \u003d self._statistic_aggregation("}],"source_content_type":"text/x-python","patch_set":1,"id":"bc8acc4d_823d0813","line":192,"updated":"2026-06-26 18:36:15.000000000","message":"granularity is a parameter to statistic_aggregation (and differs from period) but is deliberately excluded from cache_key, so two calls that differ only in granularity collide and return the first-computed value.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: A caller querying the same resource/meter/aggregate/period at two granularities gets a stale, granularity-mismatched value on the second call. No in-tree caller varies granularity today, but the signature advertises it as a dimension and inject_metric cannot distinguish it either.\n\n**Suggestion**:\nEither include granularity in cache_key (full fidelity), or document explicitly in the statistic_aggregation and inject_metric docstrings that granularity is intentionally not part of the cache identity and that all callers for a given (resource, meter, aggregate, period) must use a single granularity. Add a test asserting the current collision behavior so the decision is locked in.","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"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":"f5f5275fe425d99eb4a4b9a02b70cce8295b8d6d","unresolved":false,"context_lines":[{"line_number":200,"context_line":"            aggregate\u003daggregate,"},{"line_number":201,"context_line":"            granularity\u003dgranularity,"},{"line_number":202,"context_line":"        )"},{"line_number":203,"context_line":"        self._metric_cache[cache_key] \u003d value"},{"line_number":204,"context_line":"        return value"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    def inject_metric(self, resource_uuid, metric, aggregation, period, value):"}],"source_content_type":"text/x-python","patch_set":1,"id":"10690162_fabcb0b4","line":203,"updated":"2026-06-26 18:36:15.000000000","message":"The cache stores whatever _statistic_aggregation returns, including None (datasource unavailable / metric not found). A None result is cached permanently for the object lifetime, so subsequent calls never re-query even after the datasource recovers.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Transient datasource failures or missing-metric responses are memoized: once None is stored, every later call for that key returns None without retry, masking recoverable errors for the whole life of the datasource object.\n\n**Suggestion**:\nDo not cache falsy/error-sentinel results: only store when value is not None (e.g. `if value is not None: self._metric_cache[cache_key] \u003d value`). Add a unit test covering the None-return case to assert it is not cached.","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"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":"f5f5275fe425d99eb4a4b9a02b70cce8295b8d6d","unresolved":false,"context_lines":[{"line_number":203,"context_line":"        self._metric_cache[cache_key] \u003d value"},{"line_number":204,"context_line":"        return value"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    def inject_metric(self, resource_uuid, metric, aggregation, period, value):"},{"line_number":207,"context_line":"        \"\"\"Store a metric value in the cache under the given key."},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        Subsequent statistic_aggregation calls with a resource whose uuid"}],"source_content_type":"text/x-python","patch_set":1,"id":"4da67873_3ec67475","line":206,"updated":"2026-06-26 18:36:15.000000000","message":"inject_metric\u0027s parameter is named \u0027metric\u0027 while statistic_aggregation uses \u0027meter_name\u0027 for the same cache-key dimension. The two must receive the same value for an injection to be hit, but the naming gives no signal of that requirement.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Consistent naming removes a real footgun: a caller reading inject_metric(resource_uuid, metric\u003d...) has no signal that the value must equal the meter_name passed to statistic_aggregation. A mismatch produces a silent cache miss with no injection applied.\n\n**Recommendation**:\nRename inject_metric\u0027s parameter from \u0027metric\u0027 to \u0027meter_name\u0027 (matching statistic_aggregation), or document the equivalence in the docstring. Update the one test caller accordingly.","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"}]}
