)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"19920d467ccdaa0849b8eaa765e9e4bce699d93b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"39d6d023_75072da5","updated":"2026-08-04 12:58:20.000000000","message":"Changes lgtm. Should we add some coverage in unit tests for the new methods in the strategies?","commit_id":"0258f6a267631dcf5578169f6b178c6c32136d48"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"e375d9ecf1e61853457948c316bc91f0b601686b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"beaaddf3_ad91b77f","in_reply_to":"39d6d023_75072da5","updated":"2026-08-11 13:02:06.000000000","message":"there are some of them that are simple get that returns a property, i think those don\u0027t require. While others have a check based on input_parameters. So I guess that I can update all that is not alreadyu covered yes, since other are also being used within the strategy code.\nI will provide a new PS, thanks!","commit_id":"0258f6a267631dcf5578169f6b178c6c32136d48"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"552d8cae1373c7887479efccb5deb1879e3b5fb4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"52382b6c_7480d3f8","in_reply_to":"beaaddf3_ad91b77f","updated":"2026-08-12 11:17:41.000000000","message":"done in latest ps","commit_id":"0258f6a267631dcf5578169f6b178c6c32136d48"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"8f0c03c165969862ff7a20759d0919cf9ecddc52","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"9613cd74_3895b4fc","updated":"2026-08-17 15:36:15.000000000","message":"Marking as +1 as it requires rebase, but LGTM.","commit_id":"13d2e9e0e3fdcdafc6febfb383d0f7a9f59de04e"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"734162885f15bdcbe8a618b651fff0800b24a2c9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"ba964ad3_7eff415d","in_reply_to":"9613cd74_3895b4fc","updated":"2026-08-18 12:43:09.000000000","message":"rebased, thanks","commit_id":"13d2e9e0e3fdcdafc6febfb383d0f7a9f59de04e"}],"watcher/decision_engine/strategy/strategies/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":"14ed3dbf762a094ecea0fb5374d3710a1836cb50","unresolved":false,"context_lines":[{"line_number":411,"context_line":"    def input_parameters(self, p):"},{"line_number":412,"context_line":"        self._input_parameters \u003d p"},{"line_number":413,"context_line":""},{"line_number":414,"context_line":"    def get_period(self, resource):"},{"line_number":415,"context_line":"        return None"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"    def get_granularity(self, resource):"}],"source_content_type":"text/x-python","patch_set":4,"id":"65452d5c_241296df","line":414,"updated":"2026-08-12 12:30:48.000000000","message":"The get_period, get_granularity, and get_aggregate methods added to BaseStrategy have no docstrings, unlike get_datasource_metrics which documents its purpose and override contract. These new base methods define the API that subclasses override, so missing docstrings make the contract unclear for...\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Strategy developers overriding these methods have no documentation on the expected behavior of the resource parameter or the meaning of returning None. This is a minor maintainability concern since these are API methods meant to be overridden.\n\n**Recommendation**:\nAdd brief docstrings to get_period, get_granularity, and get_aggregate in BaseStrategy, similar to get_datasource_metrics, explaining they return None by default and should be overridden by strategies that configure these values.","commit_id":"ed42646e2092377dc8358b86131e8f8071fbbcec"}],"watcher/tests/unit/decision_engine/strategy/strategies/test_noisy_neighbor.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":"fd11a1ae792a429c7bc9d990ef85364947604606","unresolved":false,"context_lines":[{"line_number":143,"context_line":"    def test_get_period(self):"},{"line_number":144,"context_line":"        self.assertEqual(100.0, self.strategy.get_period(\u0027instance\u0027))"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    def test_get_period_default(self):"},{"line_number":147,"context_line":"        self.strategy.input_parameters \u003d utils.Struct()"},{"line_number":148,"context_line":"        self.strategy.input_parameters.update({\u0027period\u0027: 200})"},{"line_number":149,"context_line":"        self.assertEqual(200, self.strategy.get_period(\u0027instance\u0027))"}],"source_content_type":"text/x-python","patch_set":8,"id":"3200a1c8_46906bb7","line":146,"updated":"2026-08-19 20:22:39.000000000","message":"The new test named test_get_period_default replaces input_parameters with utils.Struct() and updates it with {\u0027period\u0027: 200}, then asserts get_period(\u0027instance\u0027) \u003d\u003d 200. That exercises the explicit-override path, not the \u0027period\u0027 key absent fallback. Meanwhile test_get_period asserts 100.0 using setUp\u0027s input_parameters, which already contain period\u003d100, so the genuine default branch (missing \u0027period\u0027 key returning 100.0) is never exercised anywhere for NoisyNeighbor, unlike test_uniform_airflow.py which contains a real default-path test.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: A future edit that breaks the 100.0 fallback (for example changing the hardcoded default away from the schema value of 100.0) would not be caught, and maintainers reading the suite may wrongly believe the default path is covered.\n\n**Recommendation**:\nRename the test to reflect that it checks an updated period value, and add a real default-path test that clears the \u0027period\u0027 key (or builds an empty Struct) and asserts get_period(\u0027instance\u0027) \u003d\u003d 100.0, mirroring test_uniform_airflow.py\u0027s test_get_period_default.","commit_id":"401d0d9b92ecd6112908aae25a95b3a03e8a7b67"}],"watcher/tests/unit/decision_engine/strategy/strategies/test_workload_balance.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":"22c916a1717a71f065afe465f2c990268316ac63","unresolved":false,"context_lines":[{"line_number":69,"context_line":"            }"},{"line_number":70,"context_line":"        )"},{"line_number":71,"context_line":"        self.strategy.threshold \u003d 25.0"},{"line_number":72,"context_line":"        self.strategy._period \u003d 300"},{"line_number":73,"context_line":"        self.strategy._meter \u003d \u0027instance_cpu_usage\u0027"},{"line_number":74,"context_line":"        self.strategy._granularity \u003d 300"},{"line_number":75,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"a555cbf1_3e97523d","line":72,"updated":"2026-08-12 22:38:07.000000000","message":"The setUp method still assigns self.strategy._period and self.strategy._granularity, but these instance attributes are no longer read by the strategy code after this refactoring replaced them with getter methods.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Dead assignments in test setUp do not cause test failures but create confusion about which attributes the strategy actually uses. Future maintainers may believe these values influence test outcomes when they do not.\n\n**Recommendation**:\nRemove the two stale lines (self.strategy._period \u003d 300 and self.strategy._granularity \u003d 300) from test_workload_balance.py setUp since the getter methods now read directly from input_parameters.","commit_id":"13d2e9e0e3fdcdafc6febfb383d0f7a9f59de04e"},{"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":"da6ce014038daccef9f44f1b0ee8b5920beeda4d","unresolved":false,"context_lines":[{"line_number":256,"context_line":"        self.strategy.input_parameters.update({\u0027period\u0027: 200})"},{"line_number":257,"context_line":"        self.assertEqual(200, self.strategy.get_period(\u0027instance\u0027))"},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"    def test_get_period_default(self):"},{"line_number":260,"context_line":"        self.assertEqual(300, self.strategy.get_period(\u0027instance\u0027))"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"    def test_get_granularity(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"ec7e7b8e_b18c3ad0","line":259,"updated":"2026-08-19 22:36:29.000000000","message":"test_noisy_neighbor.test_get_period_default replaces input_parameters with a Struct containing {\u0027period\u0027: 200} and asserts 200 - it only re-reads the value just set and never exercises the hardcoded 100.0 fallback. test_workload_balance.test_get_period_default asserts 300, but that 300 comes from setUp\u0027s explicit input_parameters.update({\u0027period\u0027: 300}), not from the getter\u0027s fallback. Additionally test_workload_balance setUp still assigns self.strategy._period \u003d 300 and self.strategy._granularity \u003d 300, attributes the strategy no longer reads after this commit removed them from pre_execute/do_execute.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: A regression in the fallback default (for example changing the schema default without the getter, or breaking the .get() fallback) would not be caught by any test, and the \u0027default\u0027 names give false confidence that those paths are covered; the dead _period/_granularity assignments imply an old contract that no longer exists.\n\n**Recommendation**:\nIn the *_default tests, reset input_parameters to an empty Struct (or delete the key) and assert the hardcoded fallbacks (100.0 for noisy_neighbor, 300 for workload_balance). Remove the unused self.strategy._period and self.strategy._granularity lines from test_workload_balance.setUp.","commit_id":"9c5199ff9b4e7dc35f9a4747cd956bf32eeb0cda"}]}