)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"ad39057fb0b568480b787058c960d7138cee4414","unresolved":true,"context_lines":[{"line_number":20,"context_line":"This lays the groundwork for the Audit Pipeline feature (cascade execution"},{"line_number":21,"context_line":"mode), where the pipeline handler will inject projected metric values"},{"line_number":22,"context_line":"between stages so that each strategy sees the expected post-execution"},{"line_number":23,"context_line":"cluster state without re-querying the datasource."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Strategies and wrapper methods (get_host_*, get_instance_*) are unchanged;"},{"line_number":26,"context_line":"they continue to call the public statistic_aggregation interface."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"b0e69bb6_ee30e6fc","line":23,"updated":"2026-06-29 12:03:00.000000000","message":"Althoug that will be follow-up patch, what\u0027s your plan for this? will create a single datasource object that will be used in all the strategies?","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"412ed238f0433b3d71c745d11787b9b55f430162","unresolved":true,"context_lines":[{"line_number":20,"context_line":"This lays the groundwork for the Audit Pipeline feature (cascade execution"},{"line_number":21,"context_line":"mode), where the pipeline handler will inject projected metric values"},{"line_number":22,"context_line":"between stages so that each strategy sees the expected post-execution"},{"line_number":23,"context_line":"cluster state without re-querying the datasource."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Strategies and wrapper methods (get_host_*, get_instance_*) are unchanged;"},{"line_number":26,"context_line":"they continue to call the public statistic_aggregation interface."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"09bc540c_c81fb13e","line":23,"in_reply_to":"2a3f0daa_63f231d9","updated":"2026-08-03 22:16:22.000000000","message":"New patch set added MetricDataCache, but in the end I moved the migration logic out of it, since is a operation that should live in other class, but it will be part of next Patch in the chain (that adds Audit Pipeline handler)","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"1f50f66a824639ecaeb1976315d97f686b844e32","unresolved":true,"context_lines":[{"line_number":20,"context_line":"This lays the groundwork for the Audit Pipeline feature (cascade execution"},{"line_number":21,"context_line":"mode), where the pipeline handler will inject projected metric values"},{"line_number":22,"context_line":"between stages so that each strategy sees the expected post-execution"},{"line_number":23,"context_line":"cluster state without re-querying the datasource."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Strategies and wrapper methods (get_host_*, get_instance_*) are unchanged;"},{"line_number":26,"context_line":"they continue to call the public statistic_aggregation interface."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"2a3f0daa_63f231d9","line":23,"in_reply_to":"47669ad0_f873c6f5","updated":"2026-07-29 11:57:07.000000000","message":"I have a MetricDataCache class that I will upload to this patch, it has allow us to out/get metric values, as also simulate migration (updating host metrics). It was implemented during with AuditPipeline Handler (not proposed yet) but I think that I can bring to this change.","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"cd8af3dc6b3c33016d5e9af223a8231856ab0678","unresolved":true,"context_lines":[{"line_number":20,"context_line":"This lays the groundwork for the Audit Pipeline feature (cascade execution"},{"line_number":21,"context_line":"mode), where the pipeline handler will inject projected metric values"},{"line_number":22,"context_line":"between stages so that each strategy sees the expected post-execution"},{"line_number":23,"context_line":"cluster state without re-querying the datasource."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Strategies and wrapper methods (get_host_*, get_instance_*) are unchanged;"},{"line_number":26,"context_line":"they continue to call the public statistic_aggregation interface."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"b6bdb9cd_86ba6203","line":23,"in_reply_to":"b0e69bb6_ee30e6fc","updated":"2026-06-30 13:40:53.000000000","message":"Not the datasource but the metric cache. Since datasource can change based on the metric requested, the strategy would need to select the datasource that matches the metrics required. The cache can them be updated per pipeline stage and set to every new datasource object. The cache hit will still depend on matching all keys, including the metric name.","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"cd8af3dc6b3c33016d5e9af223a8231856ab0678","unresolved":true,"context_lines":[{"line_number":20,"context_line":"This lays the groundwork for the Audit Pipeline feature (cascade execution"},{"line_number":21,"context_line":"mode), where the pipeline handler will inject projected metric values"},{"line_number":22,"context_line":"between stages so that each strategy sees the expected post-execution"},{"line_number":23,"context_line":"cluster state without re-querying the datasource."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Strategies and wrapper methods (get_host_*, get_instance_*) are unchanged;"},{"line_number":26,"context_line":"they continue to call the public statistic_aggregation interface."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"4f079765_5761f001","line":23,"in_reply_to":"b0e69bb6_ee30e6fc","updated":"2026-06-30 13:40:53.000000000","message":"So the idea in the follow up patches is that we can set both CDM (mutable) and the cache (also mutable) for a specific strategy object.","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"333235dfb7357045bced125430f5daee6531e0cf","unresolved":true,"context_lines":[{"line_number":20,"context_line":"This lays the groundwork for the Audit Pipeline feature (cascade execution"},{"line_number":21,"context_line":"mode), where the pipeline handler will inject projected metric values"},{"line_number":22,"context_line":"between stages so that each strategy sees the expected post-execution"},{"line_number":23,"context_line":"cluster state without re-querying the datasource."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Strategies and wrapper methods (get_host_*, get_instance_*) are unchanged;"},{"line_number":26,"context_line":"they continue to call the public statistic_aggregation interface."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"47669ad0_f873c6f5","line":23,"in_reply_to":"b6bdb9cd_86ba6203","updated":"2026-07-28 10:02:12.000000000","message":"You plan to include that in this patch?","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":"7d50e1a3a56b2b87f75712787970389b141054ff","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Parent:     fb688301 (Merge \"Add allocation-based capacity checks to vm_workload_consolidation\")"},{"line_number":2,"context_line":"Author:     Douglas Viroel \u003cviroel@gmail.com\u003e"},{"line_number":3,"context_line":"AuthorDate: 2026-06-24 15:52:26 -0300"},{"line_number":4,"context_line":"Commit:     Douglas Viroel \u003cviroel@gmail.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"0cbfc697_875302be","line":1,"updated":"2026-08-18 19:21:45.000000000","message":"The commit message body ends the design-rationale paragraph with \u0027... without re-querying the datasource.tox  -e docs\u0027, appending a shell command that was evidently pasted into the message editor. The fragment breaks the sentence and leaves an unexplained command in the permanent change history.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Impact**: Minor: the history record is slightly garbled and the fragment could confuse readers about whether a tox run is part of the change\u0027s instructions.\n\n**Recommendation**:\nAmend the commit message to remove the stray \u0027tox  -e docs\u0027 fragment before merge (the sentence should end at \u0027without re-querying the datasource.\u0027).","commit_id":"e8c70fc23210522c93f6fbc8e54771a66333875d"},{"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":"b301f9b4bcb6701bb6ba4481cc4521decbee18c0","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Parent:     fb688301 (Merge \"Add allocation-based capacity checks to vm_workload_consolidation\")"},{"line_number":2,"context_line":"Author:     Douglas Viroel \u003cviroel@gmail.com\u003e"},{"line_number":3,"context_line":"AuthorDate: 2026-06-24 15:52:26 -0300"},{"line_number":4,"context_line":"Commit:     Douglas Viroel \u003cviroel@gmail.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"c45e6139_c3f384c4","line":1,"updated":"2026-08-18 12:52:53.000000000","message":"The final paragraph of the commit message reads \u0027... so that each strategy sees the expected post-execution cluster state without re-querying the datasource.tox  -e docs\u0027. The \u0027tox  -e docs\u0027 text is an accidental terminal command paste appended directly to the sentence, leaving a malformed run-on sentence in the permanent change history.\n\n**Severity**: SUGGESTION | **Confidence**: 0.95\n\n**Impact**: The permanent git history contains an accidental command fragment that makes the final paragraph read as a typo and slightly obscures the stated purpose of the cache; no runtime impact.\n\n**Recommendation**:\nAmend the commit message to remove the stray \u0027tox  -e docs\u0027 fragment so the sentence ends at \u0027without re-querying the datasource.\u0027.","commit_id":"e8c70fc23210522c93f6fbc8e54771a66333875d"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"ad39057fb0b568480b787058c960d7138cee4414","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"21cb48e3_6e192c90","updated":"2026-06-29 12:03:00.000000000","message":"I left some comments but overall I like this solution, it\u0027s elegant and no disruptive with existing strategies.\n\nI only see an issue, currently the workload_stabilization implement it\u0027s own cache mechanism using oslo.cache https://github.com/openstack/watcher/blob/master/watcher/decision_engine/strategy/strategies/workload_stabilization.py . I think that will break this idea, as it will not go to the datasource after it gets the data on the first time. The only solution i see is totally remove the oslo.cache usage from it.\n\n\nI\u0027ll try to run a scale test with simulators to have an estimation about impact on memory in the decision-engine, which i expect to be acceptable.","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"cd8af3dc6b3c33016d5e9af223a8231856ab0678","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"add82f4b_27028cb1","updated":"2026-06-30 13:40:53.000000000","message":"Thanks for the review, I will get the feedback and propose updates in the following days!","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"0475c425d55befe2ffb1150a394ace1546f7025e","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"52278bbb_5dbb759f","in_reply_to":"0a6db1fb_aa472635","updated":"2026-08-04 12:17:39.000000000","message":"Done,thanks for the heads up!","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"cd8af3dc6b3c33016d5e9af223a8231856ab0678","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"65b238f3_cc33f11c","in_reply_to":"21cb48e3_6e192c90","updated":"2026-06-30 13:40:53.000000000","message":"Good point, since current implementation doesn\u0027t allow to exclude the caching, we are kind of duplicating the caching mechanism. So yeah, I think that we should remove the existing one since it would not allow us to do the simulated metric injection.","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"00b9ec3d02c959880b68143d01e686bfeee3747d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b3dfbb51_91259832","in_reply_to":"65b238f3_cc33f11c","updated":"2026-08-04 11:44:28.000000000","message":"You plan to include the removal of existing cache in workload_stabilization or make it a separate patch? ideally, that should be before or together this one.","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"bcd33cfb976aa6d8094cdc1c4ba51b387eb10fac","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"0a6db1fb_aa472635","in_reply_to":"b3dfbb51_91259832","updated":"2026-08-04 11:50:36.000000000","message":"I forgot about that yeah, I will push a new patch. Thanks Alfredo!","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"1f50f66a824639ecaeb1976315d97f686b844e32","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"d39d8cd7_c6020df7","updated":"2026-07-29 11:57:07.000000000","message":"Still want to include the MetricDataCache class that handles metrics.","commit_id":"cb00210504a32efa198a073621c3e20c8c2660cf"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"c2ade2f61918c16163f7853c3674cd92f5d834ec","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"570d7f76_af6b6a68","updated":"2026-08-04 12:22:27.000000000","message":"I think this is mostly fine. Just some comments, suggestion. For the one about the cache format is just an idea to consider, feel free to implement it or not, i don\u0027t have a clear opinion.","commit_id":"1b1f04adf7dc05f5155cd9095c106117d00022b1"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0b297b88229f59f586c49e49a3ac1db4f0ccbe5a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"fbf2b4b9_d2b4de94","updated":"2026-08-18 16:40:01.000000000","message":"i dont think any of the nits i have are worth respining this so ok lets proceed","commit_id":"e8c70fc23210522c93f6fbc8e54771a66333875d"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"576f3b8a722b09c6b6e5edee96ddb07e077f6530","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"1340e57f_2394361c","updated":"2026-08-18 19:10:09.000000000","message":"recheck\n\nthere is no additional log in dec-eng that points the reason for the audit to fail right after moving to ongoing (pre_execute)\nhttps://53c5a1ead62f56eeb235-d7d8b8c019a94a048af47540a9f762bb.ssl.cf2.rackcdn.com/openstack/ca5a7abd03644efe9ef67a25210f04dc/controller/logs/screen-watcher-decision-engine.txt","commit_id":"e8c70fc23210522c93f6fbc8e54771a66333875d"}],"doc/source/datasources/index.rst":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"ad39057fb0b568480b787058c960d7138cee4414","unresolved":true,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":".. note::"},{"line_number":40,"context_line":"   Because caching is keyed on ``(resource.uuid, meter_name, aggregate,"},{"line_number":41,"context_line":"   period)``, the ``granularity`` parameter does not affect cache lookup."},{"line_number":42,"context_line":"   A result fetched with one granularity value is returned for any later call"},{"line_number":43,"context_line":"   that differs only in granularity."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"Cache injection"},{"line_number":46,"context_line":"~~~~~~~~~~~~~~~"}],"source_content_type":"text/x-rst","patch_set":1,"id":"5fda753f_58bd8953","line":43,"range":{"start_line":41,"start_character":0,"end_line":43,"end_character":36},"updated":"2026-06-29 12:03:00.000000000","message":"any specific reason to ignore granularity in cache? iirc some of the backends ignore it, but it wouldn\u0027t hurt using it to cache too given that those will use default value, while other backends may benefit from having it.","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0b297b88229f59f586c49e49a3ac1db4f0ccbe5a","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":".. note::"},{"line_number":40,"context_line":"   Because caching is keyed on ``(resource.uuid, meter_name, aggregate,"},{"line_number":41,"context_line":"   period)``, the ``granularity`` parameter does not affect cache lookup."},{"line_number":42,"context_line":"   A result fetched with one granularity value is returned for any later call"},{"line_number":43,"context_line":"   that differs only in granularity."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"Cache injection"},{"line_number":46,"context_line":"~~~~~~~~~~~~~~~"}],"source_content_type":"text/x-rst","patch_set":1,"id":"2f2f443d_7105c8ea","line":43,"range":{"start_line":41,"start_character":0,"end_line":43,"end_character":36},"in_reply_to":"04b6a2f4_7127a158","updated":"2026-08-18 16:40:01.000000000","message":"Acknowledged","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"cd8af3dc6b3c33016d5e9af223a8231856ab0678","unresolved":true,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":".. note::"},{"line_number":40,"context_line":"   Because caching is keyed on ``(resource.uuid, meter_name, aggregate,"},{"line_number":41,"context_line":"   period)``, the ``granularity`` parameter does not affect cache lookup."},{"line_number":42,"context_line":"   A result fetched with one granularity value is returned for any later call"},{"line_number":43,"context_line":"   that differs only in granularity."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"Cache injection"},{"line_number":46,"context_line":"~~~~~~~~~~~~~~~"}],"source_content_type":"text/x-rst","patch_set":1,"id":"a60eb945_e3183bef","line":43,"range":{"start_line":41,"start_character":0,"end_line":43,"end_character":36},"in_reply_to":"5fda753f_58bd8953","updated":"2026-06-30 13:40:53.000000000","message":"In the end, for the same backend we will likely get the same value always, so it was just simplifying the key to match, but it can also be added. The idea was that would be acceptable to get a value from cache if exists, even if granularity is different...\nBut that can be changed yes, and include the granularity..","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"412ed238f0433b3d71c745d11787b9b55f430162","unresolved":true,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":".. note::"},{"line_number":40,"context_line":"   Because caching is keyed on ``(resource.uuid, meter_name, aggregate,"},{"line_number":41,"context_line":"   period)``, the ``granularity`` parameter does not affect cache lookup."},{"line_number":42,"context_line":"   A result fetched with one granularity value is returned for any later call"},{"line_number":43,"context_line":"   that differs only in granularity."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"Cache injection"},{"line_number":46,"context_line":"~~~~~~~~~~~~~~~"}],"source_content_type":"text/x-rst","patch_set":1,"id":"04b6a2f4_7127a158","line":43,"range":{"start_line":41,"start_character":0,"end_line":43,"end_character":36},"in_reply_to":"82b9cc48_b77c8ed1","updated":"2026-08-03 22:16:22.000000000","message":"New patch set now has a new MetricDataCache class and also includes granularity.","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"eb3926cf67d1b8e2407902a1241ebb369ec2af26","unresolved":true,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":".. note::"},{"line_number":40,"context_line":"   Because caching is keyed on ``(resource.uuid, meter_name, aggregate,"},{"line_number":41,"context_line":"   period)``, the ``granularity`` parameter does not affect cache lookup."},{"line_number":42,"context_line":"   A result fetched with one granularity value is returned for any later call"},{"line_number":43,"context_line":"   that differs only in granularity."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"Cache injection"},{"line_number":46,"context_line":"~~~~~~~~~~~~~~~"}],"source_content_type":"text/x-rst","patch_set":1,"id":"f3709946_d4d730fd","line":43,"range":{"start_line":41,"start_character":0,"end_line":43,"end_character":36},"in_reply_to":"a60eb945_e3183bef","updated":"2026-07-31 12:41:17.000000000","message":"I\u0027m thinking about this, It looks like different strategies have granularity as an input parameter (e.g. https://github.com/openstack/watcher/blob/master/watcher/decision_engine/strategy/strategies/workload_balance.py#L121, https://github.com/openstack/watcher/blob/master/watcher/decision_engine/strategy/strategies/vm_workload_consolidation.py#L129) will the cache be shared between strategies? I\u0027m thinking about oneshot or continuous audits, in the pipeline case iiuc the metric cache will be handled differently. If the metric cache would be shared, then I think we should include the granularity in the cache key","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"e9422e212da40c40e20ed4959df044d96c8229ea","unresolved":true,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":".. note::"},{"line_number":40,"context_line":"   Because caching is keyed on ``(resource.uuid, meter_name, aggregate,"},{"line_number":41,"context_line":"   period)``, the ``granularity`` parameter does not affect cache lookup."},{"line_number":42,"context_line":"   A result fetched with one granularity value is returned for any later call"},{"line_number":43,"context_line":"   that differs only in granularity."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"Cache injection"},{"line_number":46,"context_line":"~~~~~~~~~~~~~~~"}],"source_content_type":"text/x-rst","patch_set":1,"id":"82b9cc48_b77c8ed1","line":43,"range":{"start_line":41,"start_character":0,"end_line":43,"end_character":36},"in_reply_to":"f3709946_d4d730fd","updated":"2026-07-31 12:57:40.000000000","message":"yes, in the next PS i will update that. I have already added granularity. In case different strategies have different granularities, it will fetch the data again from the datasource. Metrics simulations will still work even with different metric parameters because at each stage, the simulation will happen and a pre-fetch of the metric will be needed in case it doesn\u0027t exists in the cache yet.","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"}],"requirements.txt":[{"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":"ed8844d313127149d4c30793bc170a1b853630b4","unresolved":false,"context_lines":[{"line_number":11,"context_line":"lxml\u003e\u003d4.5.1 # BSD"},{"line_number":12,"context_line":"croniter\u003e\u003d0.3.20 # MIT License"},{"line_number":13,"context_line":"os-resource-classes\u003e\u003d0.4.0"},{"line_number":14,"context_line":"oslo.concurrency\u003e\u003d3.26.0 # Apache-2.0"},{"line_number":15,"context_line":"oslo.config\u003e\u003d6.8.0 # Apache-2.0"},{"line_number":16,"context_line":"oslo.context\u003e\u003d2.21.0 # Apache-2.0"},{"line_number":17,"context_line":"oslo.db\u003e\u003d4.44.0 # Apache-2.0"}],"source_content_type":"text/plain","patch_set":5,"id":"ae24fcf5_35b16ab3","line":14,"updated":"2026-08-04 12:26:35.000000000","message":"The patch removes oslo.cache from requirements.txt but leaves the oslo.cache namespace entry in the oslo-config-generator configuration file, which will cause genconfig failures in environments where oslo.cache is not installed.\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Risk**: Config generation (\u0027tox -e genconfig\u0027 or similar CI jobs) will fail because oslo-config-generator cannot import the oslo.cache namespace that is no longer a dependency. This may also break downstream packaging or documentation builds that invoke config generation.\n\n**Priority**: Before merge\n**Why This Matters**: Config generation (\u0027tox -e genconfig\u0027 or similar CI jobs) will fail because oslo-config-generator cannot import the oslo.cache namespace that is no longer a dependency. This may also break downstream packaging or documentation builds that invoke config generation.\n\n**Recommendation**:\nRemove the \u0027namespace \u003d oslo.cache\u0027 line from etc/watcher/oslo-config-generator/watcher.conf to keep the config generator consistent with the dependency removal.","commit_id":"120c5e647307b3ccfb122d9f4cda6452c0ad319a"}],"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":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"cd8af3dc6b3c33016d5e9af223a8231856ab0678","unresolved":true,"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":"e45662c6_2e453826","line":192,"in_reply_to":"bc8acc4d_823d0813","updated":"2026-06-30 13:40:53.000000000","message":"Under discussion if should be included or ignored when a value already exists in cache","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":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"ad39057fb0b568480b787058c960d7138cee4414","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":"d22ffdf4_c9855cbb","line":203,"in_reply_to":"10690162_fabcb0b4","updated":"2026-06-29 12:03:00.000000000","message":"Good point to consider, imo. It\u0027d be good to not cache None values.","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"cd8af3dc6b3c33016d5e9af223a8231856ab0678","unresolved":true,"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":"0a1c9644_3c968a11","line":203,"in_reply_to":"d22ffdf4_c9855cbb","updated":"2026-06-30 13:40:53.000000000","message":"Yeah, good catch, we should not cache a None value return... Going to fix in a follow up","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"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"cd8af3dc6b3c33016d5e9af223a8231856ab0678","unresolved":true,"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":"b5e51bfe_bf858656","line":206,"in_reply_to":"4da67873_3ec67475","updated":"2026-06-30 13:40:53.000000000","message":"Going to think about that an update if needed","commit_id":"f14ab13ab3b884ad8771873ac8a2c93924a9ba24"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"eb3926cf67d1b8e2407902a1241ebb369ec2af26","unresolved":true,"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":"ad4a083a_a5b36ece","line":206,"in_reply_to":"b5e51bfe_bf858656","updated":"2026-07-31 12:41:17.000000000","message":"not a big deal, but I would change it if we need a respin, it will make it slightly easier to understand","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":"52ecb22044f80043b1839dc899a6d598edd49314","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        instance_root_disk_size\u003dNone,"},{"line_number":58,"context_line":"    )"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    def __init__(self):"},{"line_number":61,"context_line":"        self._metric_cache \u003d {}"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def _get_meter(self, meter_name):"}],"source_content_type":"text/x-python","patch_set":2,"id":"4ffa983d_7047f4fc","line":60,"updated":"2026-07-01 14:01:31.000000000","message":"The in-memory _metric_cache dict has no size limit or eviction policy and grows for the lifetime of the datasource object. In large deployments the cache accumulates an entry per (uuid, meter, aggregate, period) combination without bound.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: For single-audit usage the cache is bounded by resources times metrics. But if a datasource object is long-lived (reused across audits or held by a long-running process), memory grows monotonically, especially as inject_metric is writable by the pipeline.\n\n**Suggestion**:\nAdd a maximum size or TTL-based eviction, or document that the cache is intentionally per-audit-scope and that datasource objects must not be reused across audits. A simple safeguard is functools.lru_cache or an explicit size check that clears or evicts oldest entries when a threshold is exceeded.","commit_id":"fdbd3c158c9a731dc6348f137d7e6be8d7c46769"},{"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":"52ecb22044f80043b1839dc899a6d598edd49314","unresolved":false,"context_lines":[{"line_number":174,"context_line":"        try:"},{"line_number":175,"context_line":"            resource_uuid \u003d resource.uuid"},{"line_number":176,"context_line":"        except AttributeError:"},{"line_number":177,"context_line":"            LOG.warning("},{"line_number":178,"context_line":"                \"Failed to retrieve uuid from resource %s, \""},{"line_number":179,"context_line":"                \"skipping cache lookup and result caching.\","},{"line_number":180,"context_line":"                resource,"}],"source_content_type":"text/x-python","patch_set":2,"id":"01a33804_aa8a86f8","line":177,"updated":"2026-07-01 14:01:31.000000000","message":"The warning log on missing resource.uuid interpolates the resource object directly with %s, which calls repr on a potentially complex model object, producing a long or unhelpful string in the logs.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: Logging resource_type (which is available as a parameter) alongside a more targeted identifier would make the warning more actionable for operators diagnosing why caching is being skipped for certain resources.\n\n**Recommendation**:\nInclude resource_type in the log message: LOG.warning(\u0027Failed to retrieve uuid from resource (type\u003d%s): %s ...\u0027, resource_type, resource). Consider logging the object\u0027s type name rather than its full repr.","commit_id":"fdbd3c158c9a731dc6348f137d7e6be8d7c46769"},{"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":"52ecb22044f80043b1839dc899a6d598edd49314","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":2,"id":"007e6b8d_384c40df","line":192,"updated":"2026-07-01 14:01:31.000000000","message":"The cache key tuple (resource_uuid, meter_name, aggregate, period) is constructed inline in two places (statistic_aggregation at line 192 and inject_metric at line 219). Duplicating the key construction risks divergence if one site is updated but not the other.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Extracting a _cache_key(resource_uuid, meter_name, aggregate, period) static method ensures the key structure is defined once and makes future key changes (e.g. adding granularity) a single-point edit, eliminating the risk of the two construction sites drifting apart.\n\n**Recommendation**:\nAdd a @staticmethod def _cache_key(resource_uuid, meter_name, aggregate, period): return (resource_uuid, meter_name, aggregate, period) and call it from both statistic_aggregation and inject_metric. This also makes the key contract explicit and testable.","commit_id":"fdbd3c158c9a731dc6348f137d7e6be8d7c46769"},{"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":"52ecb22044f80043b1839dc899a6d598edd49314","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":2,"id":"f67f0c62_a96fcfe8","line":192,"updated":"2026-07-01 14:01:31.000000000","message":"granularity is excluded from the cache key but gnocchi._statistic_aggregation passes it into the backend query (gnocchi.metric.get_measures(granularity\u003dgranularity)). Two calls differing only in granularity return the first cached result, computed at a different granularity.\n\n**Severity**: HIGH | **Confidence**: 0.8\n\n**Risk**: A strategy or pipeline stage querying the same resource/metric/aggregate/period with a different granularity silently receives a value computed at the wrong sampling interval, producing incorrect metric values for gnocchi.\n\n**Priority**: Before merge\n**Why This Matters**: The index.rst note documents this as a limitation, but the gnocchi backend demonstrably varies its result by granularity (test_gnocchi_statistic_aggregation passes granularity\u003d360 and verifies it reaches the backend), making this a data-correctness bug, not a design trade-off.\n\n**Recommendation**:\nInclude granularity in the cache key: change to (resource_uuid, meter_name, aggregate, period, granularity). For prometheus and grafana where granularity is a no-op this adds no cost (callers pass the same default). Add a unit test verifying that two calls differing only in granularity produce two backend queries.","commit_id":"fdbd3c158c9a731dc6348f137d7e6be8d7c46769"},{"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":"52ecb22044f80043b1839dc899a6d598edd49314","unresolved":false,"context_lines":[{"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("},{"line_number":196,"context_line":"            resource\u003dresource,"}],"source_content_type":"text/x-python","patch_set":2,"id":"993d8f8b_a853c3fc","line":193,"updated":"2026-07-01 14:01:31.000000000","message":"_metric_cache is a plain dict accessed without synchronization. If strategies execute concurrently using threads, concurrent reads/writes during a cache miss can cause races or dict corruption under CPython if a resize occurs mid-operation.\n\n**Severity**: WARNING | **Confidence**: 0.7\n\n**Impact**: Under eventlet (monkey-patched cooperative scheduling) the dict operations are atomic between yields, so this is likely safe today. But the code has no guard if the execution model changes to native threads, and the lack of a threading.Lock makes the thread-safety assumption implicit and fragile.\n\n**Suggestion**:\nIf concurrent access is not expected, add a comment stating the cache is single-threaded only. If it might be accessed from threads, wrap the check-then-populate block in a threading.Lock (or use collections.ChainMap with a lock). At minimum, document the threading model assumption.","commit_id":"fdbd3c158c9a731dc6348f137d7e6be8d7c46769"},{"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":"52ecb22044f80043b1839dc899a6d598edd49314","unresolved":false,"context_lines":[{"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("},{"line_number":196,"context_line":"            resource\u003dresource,"},{"line_number":197,"context_line":"            resource_type\u003dresource_type,"},{"line_number":198,"context_line":"            meter_name\u003dmeter_name,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ed5abac7_af32a6ba","line":195,"updated":"2026-07-01 14:01:31.000000000","message":"The cache stores None returned by _statistic_aggregation on transient failures. All backends return None when a query fails or yields no data. Once None is cached it is returned for every subsequent call for the lifetime of the object, preventing retries.\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Risk**: A transient backend failure (network hiccup, empty result, rate limit) permanently poisons the cache entry. Subsequent strategies querying the same metric receive None and treat it as unavailable, degrading optimization decisions for the rest of the audit.\n\n**Priority**: Before merge\n**Why This Matters**: query_retry already implements retry logic with backoff, but a cache hit on a prior None short-circuits that entirely. In a cascade audit where multiple stages query the same metric, an early-stage transient failure would silently starve all later stages of data.\n\n**Recommendation**:\nSkip caching when the backend returns None: after \u0027value \u003d self._statistic_aggregation(...)\u0027 add \u0027if value is not None: self._metric_cache[cache_key] \u003d value\u0027 before storing. This lets subsequent calls re-query the backend after a failure while still caching successful results. Add a unit test (test_statistic_aggregation_does_not_cache_none) asserting _statistic_aggregation is called twice when it returns None twice.","commit_id":"fdbd3c158c9a731dc6348f137d7e6be8d7c46769"},{"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":"52ecb22044f80043b1839dc899a6d598edd49314","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":2,"id":"61e38fe3_10b371ac","line":206,"updated":"2026-07-01 14:01:31.000000000","message":"inject_metric accepts arbitrary values without validation: no check that metric is a valid METRIC_MAP key, aggregation is in AGGREGATES, or the UUID is known. A typo silently creates an unreachable cache entry or shadows legitimate data.\n\n**Severity**: WARNING | **Confidence**: 0.7\n\n**Impact**: Since inject_metric is the primary API for the upcoming audit pipeline, a mismatch between injected key components and what statistic_aggregation computes (e.g. metric\u003d\u0027cpu\u0027 vs \u0027host_cpu_usage\u0027) results in a silent no-op: the backend is queried and the injected value is never served.\n\n**Suggestion**:\nValidate the metric against METRIC_MAP (or document clearly that callers must use the watcher logical metric name, not the backend name). Consider logging at debug level when an injection occurs and at warning level when a cache key mismatch is detectable. At minimum, add a docstring example showing the exact key semantics.","commit_id":"fdbd3c158c9a731dc6348f137d7e6be8d7c46769"},{"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":"05df39f035332012aa5557962c7985a08596378b","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":3,"id":"4e6a3bad_5c7f1eaf","line":192,"updated":"2026-07-10 20:24:52.000000000","message":"The cache key (resource_uuid, meter_name, aggregate, period) in statistic_aggregation excludes the granularity parameter. In the Gnocchi backend, granularity affects both the API query (gnocchi.py:142) and the CPU usage percentage formula (gnocchi.py:174). Two calls to statistic_aggregation that...\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: If any strategy or caller queries the same resource with the same meter_name, aggregate, and period but different granularity values (e.g., 300 vs 600 seconds), the second query silently returns a value computed with the first granularity. For Gnocchi this produces wrong metric values, potentiall...\n\n**Suggestion**:\nInclude granularity in the cache key: cache_key \u003d (resource_uuid, meter_name, aggregate, period, granularity). Alternatively, document that granularity must be consistent across calls and add an assertion or warning if a mismatch is detected for a cached entry.","commit_id":"cb00210504a32efa198a073621c3e20c8c2660cf"},{"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":"d63fb4e61d46ef01badfa1ba661285717f134ab2","unresolved":false,"context_lines":[{"line_number":199,"context_line":"                aggregate\u003daggregate,"},{"line_number":200,"context_line":"                granularity\u003dgranularity,"},{"line_number":201,"context_line":"            )"},{"line_number":202,"context_line":"        cached \u003d self._metric_cache.get("},{"line_number":203,"context_line":"            resource_uuid, meter_name, aggregate, period, granularity"},{"line_number":204,"context_line":"        )"},{"line_number":205,"context_line":"        if cached is not None:"}],"source_content_type":"text/x-python","patch_set":4,"id":"723ab2ee_5265d329","line":202,"updated":"2026-08-03 22:21:28.000000000","message":"statistic_aggregation stores every result via _metric_cache.put(), including None. But the cache-hit check uses `if cached is not None`, so a stored None is indistinguishable from a cache miss. This means metrics that return None (not found, unavailable) are re-fetched from the datasource on ever...\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: Metrics that consistently return None (e.g. temporarily unavailable datasource, missing resource) will trigger a full datasource API call on every invocation instead of being served from cache. This wastes network round-trips and can slow down audit execution. Additionally, __len__ and __contains...\n\n**Suggestion**:\nEither skip caching when the datasource returns None (`if value is not None: self._metric_cache.put(...)`) or use a sentinel object in MetricDataCache.get to distinguish \u0027absent\u0027 from \u0027stored None\u0027. The former is simpler and likely matches intent since a None typically means the metric was not available.","commit_id":"1b1f04adf7dc05f5155cd9095c106117d00022b1"},{"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":"d63fb4e61d46ef01badfa1ba661285717f134ab2","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        self,"},{"line_number":227,"context_line":"        resource_uuid,"},{"line_number":228,"context_line":"        metric,"},{"line_number":229,"context_line":"        aggregation,"},{"line_number":230,"context_line":"        period,"},{"line_number":231,"context_line":"        value,"},{"line_number":232,"context_line":"        granularity\u003d300,"}],"source_content_type":"text/x-python","patch_set":4,"id":"06e4de63_4d8fdf29","line":229,"updated":"2026-08-03 22:21:28.000000000","message":"The new inject_metric method names its parameter \u0027aggregation\u0027, while all other methods in DataSourceBase, MetricDataCache, and the concrete datasource classes use \u0027aggregate\u0027. This naming mismatch creates confusion and makes it easy to pass the wrong keyword argument.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: A caller accustomed to the \u0027aggregate\u0027 keyword used throughout the datasource API may accidentally call inject_metric(..., aggregate\u003d\u0027mean\u0027) and get a TypeError. The internal `aggregate\u003daggregation` translation at line 252 is a code smell indicating the author recognized the mismatch.\n\n**Recommendation**:\nRename the parameter from \u0027aggregation\u0027 to \u0027aggregate\u0027 in inject_metric for consistency with the rest of the codebase. Update the docstring and any callers accordingly.","commit_id":"1b1f04adf7dc05f5155cd9095c106117d00022b1"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"c2ade2f61918c16163f7853c3674cd92f5d834ec","unresolved":true,"context_lines":[{"line_number":229,"context_line":"        aggregation,"},{"line_number":230,"context_line":"        period,"},{"line_number":231,"context_line":"        value,"},{"line_number":232,"context_line":"        granularity\u003d300,"},{"line_number":233,"context_line":"    ):"},{"line_number":234,"context_line":"        \"\"\"Store a metric value in the cache under the given key."},{"line_number":235,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"d4a9b1a4_b7c7a970","line":232,"updated":"2026-08-04 12:22:27.000000000","message":"Should this have the option to set simulated to True?","commit_id":"1b1f04adf7dc05f5155cd9095c106117d00022b1"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"f6858f45f1775efafd9816ffd028cafe83ccf045","unresolved":true,"context_lines":[{"line_number":229,"context_line":"        aggregation,"},{"line_number":230,"context_line":"        period,"},{"line_number":231,"context_line":"        value,"},{"line_number":232,"context_line":"        granularity\u003d300,"},{"line_number":233,"context_line":"    ):"},{"line_number":234,"context_line":"        \"\"\"Store a metric value in the cache under the given key."},{"line_number":235,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"893e476d_1b172e09","line":232,"in_reply_to":"d4a9b1a4_b7c7a970","updated":"2026-08-07 19:24:28.000000000","message":"Yeah, it is missing, in the end Audit Pipeline is dealing direct with the cache object, so this inject_metric is not really being used, but I think that makes sense to keep it here to have a simetric api. I will include the simulated here too. Thanks","commit_id":"1b1f04adf7dc05f5155cd9095c106117d00022b1"},{"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":"4a4ee2eac0e461999de034056e11df4747bba6fd","unresolved":false,"context_lines":[{"line_number":199,"context_line":"                aggregate\u003daggregate,"},{"line_number":200,"context_line":"                granularity\u003dgranularity,"},{"line_number":201,"context_line":"            )"},{"line_number":202,"context_line":"        cached \u003d self._metric_cache.get("},{"line_number":203,"context_line":"            resource_uuid, meter_name, aggregate, period, granularity"},{"line_number":204,"context_line":"        )"},{"line_number":205,"context_line":"        if cached is not None:"}],"source_content_type":"text/x-python","patch_set":6,"id":"fef5e8ff_74a84102","line":202,"updated":"2026-08-05 18:39:05.000000000","message":"When _statistic_aggregation returns None (common when a resource or metric data is not found), the wrapper stores None in the cache via put(), but the next call to statistic_aggregation cannot distinguish the stored None from a genuine cache miss. The check `if cached is not None` (base.py:205) c...\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Metrics that return None (resource not found, no data available, datasource temporarily unavailable) will trigger repeated datasource API calls on every invocation instead of being served from cache. This wastes external API calls and can slow audit execution, especially for clusters where some h...\n\n**Recommendation**:\nDistinguish cache-miss from cached-None by using a sentinel object for misses. For example, define `_MISSING \u003d object()` in MetricDataCache and have get() return `_MISSING` when the key is absent. Then in statistic_aggregation, check `if cached is not _MISSING: return cached` so that stored None values are properly returned from cache. Alternatively, check `key in self._cache` before retrieving the value.","commit_id":"bd20c76416410c70f5b5f60a84598398df6d9375"},{"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":"65760a5e0747db414f2b92bd193a6a6ce351a023","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        cached \u003d self._metric_cache.get("},{"line_number":203,"context_line":"            resource_uuid, meter_name, aggregate, period, granularity"},{"line_number":204,"context_line":"        )"},{"line_number":205,"context_line":"        if cached is not None:"},{"line_number":206,"context_line":"            return cached"},{"line_number":207,"context_line":"        value \u003d self._statistic_aggregation("},{"line_number":208,"context_line":"            resource\u003dresource,"}],"source_content_type":"text/x-python","patch_set":7,"id":"4e3b4f1d_ce251d1f","line":205,"updated":"2026-08-07 19:36:25.000000000","message":"When _statistic_aggregation returns None (common when resources are not found, metrics are unavailable, or no statistics exist), statistic_aggregation stores None in the cache via _metric_cache.put(). However, the retrieval uses \u0027if cached is not None: return cached\u0027, which means a cached None is...\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: For metrics that have no data (common in production — offline hosts, unsupported metric combinations), every call to statistic_aggregation hits the datasource API instead of the cache. This defeats the caching purpose for exactly the case where caching would save the most external calls. Addition...\n\n**Suggestion**:\nEither skip caching when the value is None (add \u0027if value is not None:\u0027 before the put call) or use a sentinel object to distinguish a cached None from a miss. Also add a test case where _statistic_aggregation returns None to verify the chosen behavior.","commit_id":"3e1cd24e4c2de7f642a1b34868afed936fdd3acf"},{"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":"46905268468010d7bec66880ab313c65894bc176","unresolved":false,"context_lines":[{"line_number":199,"context_line":"                aggregate\u003daggregate,"},{"line_number":200,"context_line":"                granularity\u003dgranularity,"},{"line_number":201,"context_line":"            )"},{"line_number":202,"context_line":"        cached \u003d self._metric_cache.get("},{"line_number":203,"context_line":"            resource_uuid, meter_name, aggregate, period, granularity"},{"line_number":204,"context_line":"        )"},{"line_number":205,"context_line":"        if cached is not None:"}],"source_content_type":"text/x-python","patch_set":8,"id":"4637e27b_d4ac753d","line":202,"updated":"2026-08-12 22:27:04.000000000","message":"When _statistic_aggregation returns None (metric unavailable), the code stores None in the cache via _metric_cache.put(). However, MetricDataCache.get() returns None for both \u0027key not found\u0027 and \u0027cached value is None\u0027, so the check \u0027if cached is not None\u0027 always falls through to call _statistic_a...\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: Strategies that query metrics which are unavailable for some resources (e.g., a host missing a temperature sensor) will repeatedly hit the datasource API for every call to statistic_aggregation with the same parameters, negating the performance benefit of caching for those entries. In workload_st...\n\n**Suggestion**:\nEither (a) skip caching when the datasource returns None (add \u0027if value is not None:\u0027 before the put call), or (b) use a sentinel or dict membership check (e.g., \u0027key in self._cache\u0027) in MetricDataCache.get to distinguish \u0027cached None\u0027 from \u0027not found\u0027, so cached None values short-circuit future calls.","commit_id":"84c56b8c45ef1782907a498a7515b1eae78b744f"}],"watcher/decision_engine/datasources/cache.py":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"c2ade2f61918c16163f7853c3674cd92f5d834ec","unresolved":true,"context_lines":[{"line_number":91,"context_line":"        )"},{"line_number":92,"context_line":"        value \u003d self._cache.get(key)"},{"line_number":93,"context_line":"        # NOTE(dviroel): Useful for debugging but may be removed"},{"line_number":94,"context_line":"        #  if generates too much logging."},{"line_number":95,"context_line":"        if value is not None:"},{"line_number":96,"context_line":"            LOG.debug("},{"line_number":97,"context_line":"                \"MetricDataCache.get: cache hit resource\u003d%s meter\u003d%s \""}],"source_content_type":"text/x-python","patch_set":4,"id":"0f05b459_b78ad190","line":94,"updated":"2026-08-04 12:22:27.000000000","message":"I have the same doubt, it may be too noise, but let\u0027s keep it so far. We actually have debug messages for each metric get out of the cache, so it\u0027s probably consistent.","commit_id":"1b1f04adf7dc05f5155cd9095c106117d00022b1"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"f6858f45f1775efafd9816ffd028cafe83ccf045","unresolved":true,"context_lines":[{"line_number":91,"context_line":"        )"},{"line_number":92,"context_line":"        value \u003d self._cache.get(key)"},{"line_number":93,"context_line":"        # NOTE(dviroel): Useful for debugging but may be removed"},{"line_number":94,"context_line":"        #  if generates too much logging."},{"line_number":95,"context_line":"        if value is not None:"},{"line_number":96,"context_line":"            LOG.debug("},{"line_number":97,"context_line":"                \"MetricDataCache.get: cache hit resource\u003d%s meter\u003d%s \""}],"source_content_type":"text/x-python","patch_set":4,"id":"70e6003f_43ea5f82","line":94,"in_reply_to":"0f05b459_b78ad190","updated":"2026-08-07 19:24:28.000000000","message":"Yeah, has been useful for initial debug of audit pipeline logic, but we may remove that in future or add an additional config option for it maybe","commit_id":"1b1f04adf7dc05f5155cd9095c106117d00022b1"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0b297b88229f59f586c49e49a3ac1db4f0ccbe5a","unresolved":true,"context_lines":[{"line_number":91,"context_line":"        )"},{"line_number":92,"context_line":"        value \u003d self._cache.get(key)"},{"line_number":93,"context_line":"        # NOTE(dviroel): Useful for debugging but may be removed"},{"line_number":94,"context_line":"        #  if generates too much logging."},{"line_number":95,"context_line":"        if value is not None:"},{"line_number":96,"context_line":"            LOG.debug("},{"line_number":97,"context_line":"                \"MetricDataCache.get: cache hit resource\u003d%s meter\u003d%s \""}],"source_content_type":"text/x-python","patch_set":4,"id":"0b64401a_51f426f8","line":94,"in_reply_to":"70e6003f_43ea5f82","updated":"2026-08-18 16:40:01.000000000","message":"debug logs shoudl be useful for debuging but we shoudl also be abel to run in production with full debug logging enabeld and not be overly verbose.\n\nso i guess we can see what this looks like in ci and decied if we want to keep this or drop it in a followup.","commit_id":"1b1f04adf7dc05f5155cd9095c106117d00022b1"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"c2ade2f61918c16163f7853c3674cd92f5d834ec","unresolved":true,"context_lines":[{"line_number":173,"context_line":"        )"},{"line_number":174,"context_line":"        return self._simulated.get(key, False)"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"    def remove(self, resource_id, meter_name\u003dNone):"},{"line_number":177,"context_line":"        \"\"\"Remove cached values for a resource."},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"        Can remove all metrics for a resource, or only a specific metric."},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"        :param resource_id: ID of the resource"},{"line_number":182,"context_line":"        :param meter_name: Name of the metric (optional filter)"},{"line_number":183,"context_line":"        \"\"\""},{"line_number":184,"context_line":"        keys_to_remove \u003d []"},{"line_number":185,"context_line":"        for key in self._cache:"},{"line_number":186,"context_line":"            parts \u003d key.split(\":\")"},{"line_number":187,"context_line":"            if parts[0] \u003d\u003d str(resource_id):"},{"line_number":188,"context_line":"                if meter_name is None or parts[1] \u003d\u003d str(meter_name):"},{"line_number":189,"context_line":"                    keys_to_remove.append(key)"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        for key in keys_to_remove:"},{"line_number":192,"context_line":"            del self._cache[key]"},{"line_number":193,"context_line":"            self._simulated.pop(key, None)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def clear(self):"},{"line_number":196,"context_line":"        \"\"\"Clear all cached values.\"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"872a1da1_335c30f9","line":193,"range":{"start_line":176,"start_character":0,"end_line":193,"end_character":42},"updated":"2026-08-04 12:22:27.000000000","message":"If we are using this method often, it may be worthy to use the resource_id as key in _cache as, `{ resource_id: {\"meter_name:aggregate:period:granularity\": value, ...}` so that we don\u0027t need to transverse the entire _cache to clean metrics for a resouce_id. I\u0027m not sure if the usage of this method justifies, but it may be a cleaner way to organize data, althogh, as con, for getting data we\u0027d need to make a double get. I\u0027m not sure if it\u0027s worthy, just an idea to consider.","commit_id":"1b1f04adf7dc05f5155cd9095c106117d00022b1"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"f6858f45f1775efafd9816ffd028cafe83ccf045","unresolved":true,"context_lines":[{"line_number":173,"context_line":"        )"},{"line_number":174,"context_line":"        return self._simulated.get(key, False)"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"    def remove(self, resource_id, meter_name\u003dNone):"},{"line_number":177,"context_line":"        \"\"\"Remove cached values for a resource."},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"        Can remove all metrics for a resource, or only a specific metric."},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"        :param resource_id: ID of the resource"},{"line_number":182,"context_line":"        :param meter_name: Name of the metric (optional filter)"},{"line_number":183,"context_line":"        \"\"\""},{"line_number":184,"context_line":"        keys_to_remove \u003d []"},{"line_number":185,"context_line":"        for key in self._cache:"},{"line_number":186,"context_line":"            parts \u003d key.split(\":\")"},{"line_number":187,"context_line":"            if parts[0] \u003d\u003d str(resource_id):"},{"line_number":188,"context_line":"                if meter_name is None or parts[1] \u003d\u003d str(meter_name):"},{"line_number":189,"context_line":"                    keys_to_remove.append(key)"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        for key in keys_to_remove:"},{"line_number":192,"context_line":"            del self._cache[key]"},{"line_number":193,"context_line":"            self._simulated.pop(key, None)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def clear(self):"},{"line_number":196,"context_line":"        \"\"\"Clear all cached values.\"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"c001d4a9_9f720f1f","line":193,"range":{"start_line":176,"start_character":0,"end_line":193,"end_character":42},"in_reply_to":"872a1da1_335c30f9","updated":"2026-08-07 19:24:28.000000000","message":"something to think about it yeah Not sure if this method will be more useful in future since for audit pipeline has been more useful just to cleanup all simulated or everything.","commit_id":"1b1f04adf7dc05f5155cd9095c106117d00022b1"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0b297b88229f59f586c49e49a3ac1db4f0ccbe5a","unresolved":false,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def __init__(self):"},{"line_number":68,"context_line":"        \"\"\"Initialize the metric cache.\"\"\""},{"line_number":69,"context_line":"        self._cache \u003d {}"},{"line_number":70,"context_line":"        self._simulated \u003d {}"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    def get("}],"source_content_type":"text/x-python","patch_set":9,"id":"a9041774_e023399f","line":69,"range":{"start_line":69,"start_character":7,"end_line":69,"end_character":24},"updated":"2026-08-18 16:40:01.000000000","message":"we could have made this cofnigurabel via oslo.cache potially but that is out of scope for now","commit_id":"e8c70fc23210522c93f6fbc8e54771a66333875d"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0b297b88229f59f586c49e49a3ac1db4f0ccbe5a","unresolved":true,"context_lines":[{"line_number":189,"context_line":"                    keys_to_remove.append(key)"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        for key in keys_to_remove:"},{"line_number":192,"context_line":"            del self._cache[key]"},{"line_number":193,"context_line":"            self._simulated.pop(key, None)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def clear(self):"},{"line_number":196,"context_line":"        \"\"\"Clear all cached values.\"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"f5d2e330_00ca28e6","line":193,"range":{"start_line":192,"start_character":10,"end_line":193,"end_character":42},"updated":"2026-08-18 16:40:01.000000000","message":"i would prefer if we used pop in both case by the way.\n\ni generally prefer to avoid the hard key error that del will create due to the subscript access.","commit_id":"e8c70fc23210522c93f6fbc8e54771a66333875d"}],"watcher/decision_engine/strategy/strategies/workload_stabilization.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0b297b88229f59f586c49e49a3ac1db4f0ccbe5a","unresolved":true,"context_lines":[{"line_number":21,"context_line":"import math"},{"line_number":22,"context_line":"import random"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"import oslo_cache"},{"line_number":25,"context_line":"import oslo_utils"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"from oslo_config import cfg"}],"source_content_type":"text/x-python","patch_set":9,"id":"40a777e3_6d32b770","side":"PARENT","line":24,"updated":"2026-08-18 16:40:01.000000000","message":"i geuss we can revisit this but we may have wanted to use oslo.cache to provide the cache backend in a conrigurable way.","commit_id":"fb6883011df46314b906a862947e43b93f1dd1ab"}],"watcher/tests/unit/decision_engine/datasources/test_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":"52ecb22044f80043b1839dc899a6d598edd49314","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        )"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"class TestDataSourceBaseCache(base.BaseTestCase):"},{"line_number":68,"context_line":"    def setUp(self):"},{"line_number":69,"context_line":"        super().setUp()"},{"line_number":70,"context_line":"        self.helper \u003d datasource.DataSourceBase()"}],"source_content_type":"text/x-python","patch_set":2,"id":"51cdbe04_b1304d7f","line":67,"updated":"2026-07-01 14:01:31.000000000","message":"The new test class TestDataSourceBaseCache does not test the None-caching scenario, the granularity-collision scenario, or the inject-then-overwrite scenario. The four tests cover basic miss, hit, inject, and no-uuid paths, but miss the edge cases most likely to harbor bugs.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Without a test asserting None is not cached (or documenting that it is intentionally cached), a future refactor could silently change this behavior. The granularity collision is similarly unprotected by tests.\n\n**Suggestion**:\nAdd: (1) test asserting _statistic_aggregation is called on every invocation when it returns None; (2) test asserting two calls with different granularity produce two backend calls (once granularity is added to the key); (3) test asserting inject_metric followed by a real fetch overwrites the injected value.","commit_id":"fdbd3c158c9a731dc6348f137d7e6be8d7c46769"},{"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":"d63fb4e61d46ef01badfa1ba661285717f134ab2","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        )"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"class TestDataSourceBaseCache(base.BaseTestCase):"},{"line_number":68,"context_line":"    def setUp(self):"},{"line_number":69,"context_line":"        super().setUp()"},{"line_number":70,"context_line":"        self.helper \u003d datasource.DataSourceBase()"}],"source_content_type":"text/x-python","patch_set":4,"id":"34e672f3_935ecc8a","line":67,"updated":"2026-08-03 22:21:28.000000000","message":"The test suite for the new caching wrapper (TestDataSourceBaseCache in test_base.py) covers cache hit, cache miss, inject, and no-uuid scenarios, but never exercises the case where _statistic_aggregation returns None. This is a common real-world scenario (metric unavailable, resource not found) a...\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: The None-value caching defect (CF-001) is not caught by the existing test suite. Without a test for this edge case, regressions in None handling could be introduced silently in future changes.\n\n**Recommendation**:\nAdd a test that sets _statistic_aggregation return_value to None and verifies that statistic_aggregation returns None and that the behavior is consistent (either cached or explicitly not cached). This also documents the intended behavior for missing metrics.","commit_id":"1b1f04adf7dc05f5155cd9095c106117d00022b1"}],"watcher/tests/unit/decision_engine/datasources/test_cache.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0b297b88229f59f586c49e49a3ac1db4f0ccbe5a","unresolved":true,"context_lines":[{"line_number":51,"context_line":"    def setUp(self):"},{"line_number":52,"context_line":"        super().setUp()"},{"line_number":53,"context_line":"        self.cache \u003d MetricDataCache()"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    # ------------------------------------------------------------------"},{"line_number":56,"context_line":"    # get / put"},{"line_number":57,"context_line":"    # ------------------------------------------------------------------"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def test_get_returns_none_on_miss(self):"},{"line_number":60,"context_line":"        self.assertIsNone(self.cache.get(\u0027no-such-resource\u0027, \u0027host_cpu_usage\u0027))"},{"line_number":61,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9b8c2730_aa59db28","line":58,"range":{"start_line":54,"start_character":1,"end_line":58,"end_character":1},"updated":"2026-08-18 16:40:01.000000000","message":"i know ai likes to doi this but these get out of date fast so we shoudl remove them","commit_id":"e8c70fc23210522c93f6fbc8e54771a66333875d"}]}