)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"eec3cfadfdbfa326a20a60966b0bb63829dccde7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"afed36b8_54ef6e44","updated":"2026-06-25 10:03:05.000000000","message":"lgtm, just spotted something that could be removed","commit_id":"30fdaca273ea38427955e5c291e6c993914856e7"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"4a65429431c2918a4b73313d0dc939c251a4bf3d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"a8fbb432_622856f0","updated":"2026-07-21 14:40:43.000000000","message":"lgtm","commit_id":"b9609c61342e76a4da705a82154f73fd660753d8"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"2c62650319a8ab0af2866683af723ba902b9510f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"607d759d_a1b32028","updated":"2026-07-21 12:19:30.000000000","message":"looks good, thanks Alfredo","commit_id":"b9609c61342e76a4da705a82154f73fd660753d8"}],"watcher/decision_engine/strategy/strategies/zone_migration.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":"10449994b22cbd3fe39a579b6405a37f30cdf99b","unresolved":false,"context_lines":[{"line_number":634,"context_line":"            return src_type is None or volume.volume_type \u003d\u003d src_type"},{"line_number":635,"context_line":""},{"line_number":636,"context_line":"        target_volumes \u003d []"},{"line_number":637,"context_line":"        for volume in self.storage_model.get_all_volumes().values():"},{"line_number":638,"context_line":"            pool_name \u003d volume.host"},{"line_number":639,"context_line":"            for migrate_input in self.migrate_storage_pools:"},{"line_number":640,"context_line":"                src_pool \u003d migrate_input[\"src_pool\"]"}],"source_content_type":"text/x-python","patch_set":1,"id":"2b9de2bf_585b16e8","line":637,"updated":"2026-06-24 17:01:21.000000000","message":"get_volumes() previously filtered volumes through storage_model.has_node(volume.id) to skip volumes not in the model. The new code removes this check entirely. While iterating get_all_volumes() makes the check unnecessary, the comment explaining scope constraints was also removed.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Future maintainers may not understand why volumes outside the storage model are excluded from migration targets, since the filtering is now implicit via get_all_volumes(). If the model is incomplete, those volumes will silently not be migrated.\n\n**Suggestion**:\nAdd a brief comment before the loop explaining that get_all_volumes() returns only volumes tracked in the storage cluster data model and that volumes not in the model are intentionally excluded. Optionally add a debug log noting the total volume count being evaluated.","commit_id":"2413dbe55c3a7d03d85afed8bf2621cf7320164d"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"de4b755229e869ae985a952d9d2f2155b9897ce1","unresolved":false,"context_lines":[{"line_number":634,"context_line":"            return src_type is None or volume.volume_type \u003d\u003d src_type"},{"line_number":635,"context_line":""},{"line_number":636,"context_line":"        target_volumes \u003d []"},{"line_number":637,"context_line":"        for volume in self.storage_model.get_all_volumes().values():"},{"line_number":638,"context_line":"            pool_name \u003d volume.host"},{"line_number":639,"context_line":"            for migrate_input in self.migrate_storage_pools:"},{"line_number":640,"context_line":"                src_pool \u003d migrate_input[\"src_pool\"]"}],"source_content_type":"text/x-python","patch_set":1,"id":"44c275b0_0a2ce325","line":637,"in_reply_to":"2b9de2bf_585b16e8","updated":"2026-06-25 07:32:42.000000000","message":"Seems pretty obvious to me for any watcher developer, no need to document imo. Strategies should consider the model as the source of truth for the cluster topology. Other than that is a bug (actually, that\u0027s what this patch is fixing).","commit_id":"2413dbe55c3a7d03d85afed8bf2621cf7320164d"},{"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":"4c5389c052ee2a14c5b8360522c9dd249e40f021","unresolved":false,"context_lines":[{"line_number":880,"context_line":"        LOG.debug(\"project_id: %s, sort_key: %s\", project_id, sort_key)"},{"line_number":881,"context_line":"        return project_id \u003d\u003d sort_key"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":"    def get_project_id(self, item):"},{"line_number":884,"context_line":"        \"\"\"get project id of item"},{"line_number":885,"context_line":""},{"line_number":886,"context_line":"        :param item: instance object or volume object"}],"source_content_type":"text/x-python","patch_set":2,"id":"4ca05e1d_d7cd023e","line":883,"updated":"2026-06-25 11:15:27.000000000","message":"get_project_id in ProjectSortFilter returns None implicitly when item is neither element.Volume nor element.Instance. This is unchanged behavior, but making the None return explicit would clarify the method contract.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Making the implicit None return explicit improves readability and makes the method contract clearer for future maintainers, especially since the method is called from compare_func which compares the result to sort_key.\n\n**Recommendation**:\nConsider adding an explicit \u0027return None\u0027 after the isinstance check, or restructuring to make the fallback clear. This is a minor readability improvement, not a correctness issue.","commit_id":"30fdaca273ea38427955e5c291e6c993914856e7"}],"watcher/tests/unit/decision_engine/strategy/strategies/test_zone_migration.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":"10449994b22cbd3fe39a579b6405a37f30cdf99b","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        except exception.PoolNotFound:"},{"line_number":120,"context_line":"            pool \u003d element.Pool("},{"line_number":121,"context_line":"                name\u003dhost,"},{"line_number":122,"context_line":"                total_volumes\u003d1,"},{"line_number":123,"context_line":"                total_capacity_gb\u003d500,"},{"line_number":124,"context_line":"                free_capacity_gb\u003d420,"},{"line_number":125,"context_line":"                provisioned_capacity_gb\u003d80,"}],"source_content_type":"text/x-python","patch_set":1,"id":"49862435_addd7987","line":122,"updated":"2026-06-24 17:01:21.000000000","message":"The add_volume_to_model test helper creates Pool objects with total_volumes\u003d1 hardcoded. When multiple volumes share the same pool (common in several tests), the pool total_volumes count will be incorrect.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: More accurate test fixtures that reflect real-world pool state, reducing the risk of tests passing against incorrect model data.\n\n**Recommendation**:\nAfter adding the volume to the pool, increment pool.total_volumes if the pool already exists. Alternatively, remove the hardcoded value and let the model manage the count if add_volume handles it.","commit_id":"2413dbe55c3a7d03d85afed8bf2621cf7320164d"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"de4b755229e869ae985a952d9d2f2155b9897ce1","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        except exception.PoolNotFound:"},{"line_number":120,"context_line":"            pool \u003d element.Pool("},{"line_number":121,"context_line":"                name\u003dhost,"},{"line_number":122,"context_line":"                total_volumes\u003d1,"},{"line_number":123,"context_line":"                total_capacity_gb\u003d500,"},{"line_number":124,"context_line":"                free_capacity_gb\u003d420,"},{"line_number":125,"context_line":"                provisioned_capacity_gb\u003d80,"}],"source_content_type":"text/x-python","patch_set":1,"id":"618c6d1e_02335f48","line":122,"in_reply_to":"49862435_addd7987","updated":"2026-06-25 07:32:42.000000000","message":"fixed.","commit_id":"2413dbe55c3a7d03d85afed8bf2621cf7320164d"},{"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":"10449994b22cbd3fe39a579b6405a37f30cdf99b","unresolved":false,"context_lines":[{"line_number":131,"context_line":"            try:"},{"line_number":132,"context_line":"                node \u003d model.get_node_by_name(node_name)"},{"line_number":133,"context_line":"            except exception.StorageNodeNotFound:"},{"line_number":134,"context_line":"                node \u003d element.StorageNode("},{"line_number":135,"context_line":"                    host\u003dnode_name,"},{"line_number":136,"context_line":"                    zone\u003d\u0027zone\u0027,"},{"line_number":137,"context_line":"                    status\u003d\u0027enabled\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"93801fcb_cbe27f95","line":134,"updated":"2026-06-24 17:01:21.000000000","message":"The add_volume_to_model test helper hardcodes StorageNode volume_type\u003d[\u0027type1\u0027] for all newly created nodes. When a test adds a volume with a different volume_type to a new pool, the StorageNode will not list that type, creating a model inconsistency.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Makes the test helper more robust by reflecting the actual volume_type being added, preventing false-positive test passes when node type filtering logic is added to the strategy later.\n\n**Recommendation**:\nDerive the StorageNode volume_type from the volume being added: volume_type\u003d[kwargs.get(\u0027volume_type\u0027, \u0027type1\u0027)], or set it to a broader list like [\u0027type1\u0027, \u0027type2\u0027, \u0027type3\u0027] to cover all test scenarios.","commit_id":"2413dbe55c3a7d03d85afed8bf2621cf7320164d"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"de4b755229e869ae985a952d9d2f2155b9897ce1","unresolved":false,"context_lines":[{"line_number":131,"context_line":"            try:"},{"line_number":132,"context_line":"                node \u003d model.get_node_by_name(node_name)"},{"line_number":133,"context_line":"            except exception.StorageNodeNotFound:"},{"line_number":134,"context_line":"                node \u003d element.StorageNode("},{"line_number":135,"context_line":"                    host\u003dnode_name,"},{"line_number":136,"context_line":"                    zone\u003d\u0027zone\u0027,"},{"line_number":137,"context_line":"                    status\u003d\u0027enabled\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"73a96e42_3d0e82b5","line":134,"in_reply_to":"93801fcb_cbe27f95","updated":"2026-06-25 07:32:42.000000000","message":"fixed","commit_id":"2413dbe55c3a7d03d85afed8bf2621cf7320164d"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"eec3cfadfdbfa326a20a60966b0bb63829dccde7","unresolved":true,"context_lines":[{"line_number":87,"context_line":"        self.strategy \u003d strategies.ZoneMigration(config\u003dmock.Mock())"},{"line_number":88,"context_line":"        self.strategy.input_parameters \u003d self.input_parameters"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"        self.m_osc \u003d self.useFixture("},{"line_number":91,"context_line":"            fixtures.MockPatch("},{"line_number":92,"context_line":"                \"watcher.common.clients.OpenStackClients\", autospec\u003dTrue"},{"line_number":93,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":2,"id":"5bd46ffb_f0ca40e0","line":90,"updated":"2026-06-25 10:03:05.000000000","message":"this is not used anymore, we could remove it as well","commit_id":"30fdaca273ea38427955e5c291e6c993914856e7"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"2c62650319a8ab0af2866683af723ba902b9510f","unresolved":false,"context_lines":[{"line_number":87,"context_line":"        self.strategy \u003d strategies.ZoneMigration(config\u003dmock.Mock())"},{"line_number":88,"context_line":"        self.strategy.input_parameters \u003d self.input_parameters"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"        self.m_osc \u003d self.useFixture("},{"line_number":91,"context_line":"            fixtures.MockPatch("},{"line_number":92,"context_line":"                \"watcher.common.clients.OpenStackClients\", autospec\u003dTrue"},{"line_number":93,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":2,"id":"d465e420_ba5c887d","line":90,"in_reply_to":"5bd46ffb_f0ca40e0","updated":"2026-07-21 12:19:30.000000000","message":"Done","commit_id":"30fdaca273ea38427955e5c291e6c993914856e7"},{"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":"4c5389c052ee2a14c5b8360522c9dd249e40f021","unresolved":false,"context_lines":[{"line_number":127,"context_line":"                virtual_free\u003d420,"},{"line_number":128,"context_line":"            )"},{"line_number":129,"context_line":"            model.add_pool(pool)"},{"line_number":130,"context_line":"        node_name \u003d host.split(\u0027#\u0027)[0]"},{"line_number":131,"context_line":"        try:"},{"line_number":132,"context_line":"            node \u003d model.get_node_by_name(node_name)"},{"line_number":133,"context_line":"            if volume.volume_type not in node.volume_type:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ea2bdde_799e6715","line":130,"updated":"2026-06-25 11:15:27.000000000","message":"The add_volume_to_model test helper creates StorageNode and Pool objects with hardcoded capacity values when they do not exist. While appropriate for test infrastructure, the node_name derivation (host.split(\u0027#\u0027)[0]) assumes all pool names follow the host@backend#pool format.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Adding a brief comment documenting the pool name format assumption makes the test helper easier to understand and maintain, especially since the production code relies on the same host@backend#pool convention.\n\n**Recommendation**:\nThis is test-only code and the assumption matches the production convention used throughout the strategy. No change required, but a brief inline comment documenting the format assumption would aid future test maintenance.","commit_id":"30fdaca273ea38427955e5c291e6c993914856e7"},{"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":"4c5389c052ee2a14c5b8360522c9dd249e40f021","unresolved":false,"context_lines":[{"line_number":219,"context_line":""},{"line_number":220,"context_line":"        self.assertEqual([], instances)"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"    def test_get_volumes_with_volume_not_found(self):"},{"line_number":223,"context_line":"        # Volume on a pool that is not in the migration parameters"},{"line_number":224,"context_line":"        self.add_volume_to_model(host\u003d\"src3@back3#pool1\", name\u003d\"volume_1\")"},{"line_number":225,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"f9d06f2f_b434de79","line":222,"updated":"2026-06-25 11:15:27.000000000","message":"test_get_volumes_with_volume_not_found no longer tests its stated scenario. It now adds a volume on a non-matching pool (src3@back3#pool1), testing pool mismatch filtering rather than volume-not-in-model. The name and comment are misleading.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: The original edge case (volume in Cinder but absent from the storage model) is no longer tested because volumes now come from the model itself. The test name suggests it still tests volume-not-found, but it actually tests pool mismatch. Future maintainers may be confused.\n\n**Suggestion**:\nRename the test to reflect what it now tests, e.g. test_get_volumes_pool_not_in_migration_params, and update the comment. The has_node scope-filtering scenario is no longer applicable since volumes come from the model directly.","commit_id":"30fdaca273ea38427955e5c291e6c993914856e7"},{"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":"192784eb91ad77c2e462587b10e39d10ba5c7bcd","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        self.strategy \u003d strategies.ZoneMigration(config\u003dmock.Mock())"},{"line_number":86,"context_line":"        self.strategy.input_parameters \u003d self.input_parameters"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"    def add_volume_to_model(self, **kwargs):"},{"line_number":89,"context_line":"        host \u003d kwargs.get(\u0027host\u0027, \u0027fake@back#pool\u0027)"},{"line_number":90,"context_line":"        volume \u003d element.Volume("},{"line_number":91,"context_line":"            uuid\u003dkwargs.get(\u0027uuid\u0027, utils.generate_uuid()),"}],"source_content_type":"text/x-python","patch_set":3,"id":"2814c05c_ccba52b9","line":88,"updated":"2026-07-02 15:37:21.000000000","message":"The test helper add_volume_to_model creates the pool-\u003enode graph edge (model.map_pool) only in the StorageNodeNotFound branch. When the node already exists, the pool is not remapped to it. Harmless for zone_migration tests, but could surprise a future test walking pool-\u003enode edges.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: Makes the test helper robust for future tests that exercise the full storage model graph (e.g. node-to-pool traversal) without silent gaps in the model topology.\n\n**Recommendation**:\nIn the existing-node branch of add_volume_to_model, also ensure the pool is mapped to the node when the volume type is appended, for example by guarding model.map_pool(pool, node) so it runs whenever a new pool is introduced or by calling it unconditionally if map_pool is idempotent. This keeps the helper\u0027s model topology consistent across both branches.","commit_id":"79032102012593f433c26697e41dc0b718674ac3"}]}