)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"ec40ecc480cc5ab4b4bf6f8f4e0d2edd85644cda","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"664189fc_7786693b","updated":"2025-11-28 14:50:59.000000000","message":"since the issue affect users, I think that we could provide a release notes for this fix","commit_id":"3156218fd8aeaa79f60a5f1d9e8b8de3206f2ec2"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"0859f59f5f7039988e4b64080019e62dfb4f1e4d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"19128eef_ebc76131","in_reply_to":"664189fc_7786693b","updated":"2025-11-28 15:08:19.000000000","message":"good call, I\u0027ve added the relase note","commit_id":"3156218fd8aeaa79f60a5f1d9e8b8de3206f2ec2"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"6f163d5514134506053622b8359f9d8a9547e73a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"3ddfe12d_400aec92","updated":"2025-12-05 08:40:17.000000000","message":"LTGM, just a minor non-blocking suggestion about tests coverage.","commit_id":"8e0dc5a804bcd2e228a8e404fa093d089de2ed59"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"f315b40a8589655e12317ddaa34c1a6dff4b6872","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"9e61ea96_3e992f90","updated":"2025-11-28 17:09:58.000000000","message":"thanks lgtm","commit_id":"8e0dc5a804bcd2e228a8e404fa093d089de2ed59"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"83d138e461e2ec85cd4a4fbeabef81227ddf1ea7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"3004d5f0_fbeb9c52","updated":"2025-12-11 17:17:50.000000000","message":"thanks for the updates","commit_id":"8f2ca05185912bebb0c0cc04d3f1357c8e04e124"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"4045b875e8993034dfafeae5fecb0f1341661a0d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"ba6c8d43_85e9a6fb","updated":"2026-03-05 12:35:53.000000000","message":"check-rdo","commit_id":"1d561e2fa1522b976dba51ad91a10551a69551b5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4d3632024279019a131412fcbd13791e56faa9b4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"606e80f4_38b31e7d","updated":"2026-03-05 16:10:23.000000000","message":"im ok with fast approving this if you want to respin it for the test nit\n\notherwise feel free to +w it and we can adress the nits in a followup","commit_id":"1d561e2fa1522b976dba51ad91a10551a69551b5"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"fc7362ddb263ab2aff0f25e511250f83b592ea08","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"10b3c654_50a366c6","updated":"2026-03-05 15:46:49.000000000","message":"lgtm, just a nit in test that can be removed later.","commit_id":"1d561e2fa1522b976dba51ad91a10551a69551b5"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"f7c7ccb7c3d67e77d26530a14af87b2789adb32b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"1429578c_94eb3537","updated":"2026-03-05 16:57:05.000000000","message":"looks good to go, thanks","commit_id":"16446385dd840b5cc072e3453e271e726aa38a72"}],"watcher/decision_engine/strategy/strategies/zone_migration.py":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"e872cfceeeeaa12b9d32ad8c357d80bd6903ffbf","unresolved":true,"context_lines":[{"line_number":398,"context_line":"                        pool.get(\"dst_type\"))"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        # Second pass: fall back to matching just src_pool"},{"line_number":401,"context_line":"        for pool in self.migrate_storage_pools:"},{"line_number":402,"context_line":"            if pool.get(\"src_pool\") \u003d\u003d src_pool:"},{"line_number":403,"context_line":"                return (pool.get(\"dst_pool\"),"},{"line_number":404,"context_line":"                        pool.get(\"dst_type\"))"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"    def volumes_migration(self, volumes, action_counter, instance_targets\u003d[]):"},{"line_number":407,"context_line":"        instance_target_ids \u003d {instance.id for instance in instance_targets}"}],"source_content_type":"text/x-python","patch_set":6,"id":"738ab11d_f5160986","line":404,"range":{"start_line":401,"start_character":0,"end_line":404,"end_character":45},"updated":"2025-12-11 09:15:09.000000000","message":"So with this patch, the behavior of the strategy is:\n\n- An audit config with `[{\"src_pool\": pool1}]` (whatever the dst values are) would match for retype/migrate all volumes in the pool and provides dst_pool or dst_type.\n- An audit config with a `[{\"src_pool\": pool1, \"src_type\": type1}]` would match only volumes in the src_pool and src_type.\n- An audit config with `[{\"src_pool\": pool1, \"src_type\": type1}, {\"src_pool\": pool1, \"src_type\": type2}]` would match only volumes in pool1 and type1 or type2. A volume in pool1 and type3 would not be selected for migration/retype because it would not be added to the target list of volumes in:\n\nhttps://github.com/openstack/watcher/blob/6704838265998619793bb884171ef1cc3277f4de/watcher/decision_engine/strategy/strategies/zone_migration.py#L595\n\ngiven the logic in https://github.com/openstack/watcher/blob/6704838265998619793bb884171ef1cc3277f4de/watcher/decision_engine/strategy/strategies/zone_migration.py#L564-L577\n\n- An audit config with `[{\"src_pool\": pool1, \"src_type\": type1}, {\"src_pool\": pool1}]` would match all volumes in pool1. The destination for volumes in pool1 and type1 would not be given by the first, most restrictive config, while the rest of volumes would be given by the one without type.\n\nFor me the behavior of this method it\u0027s slightly missleading because in the case of an audit with config `[{\"src_pool\": pool1, \"src_type\": type1}, {\"src_pool\": pool1, \"src_type\": type2}]` or `[{\"src_pool\": pool1, \"src_type\": type1}]`, a request to the method for a volume in pool1 and type3 will return the pool/type for type1. However, this case would never happen in the strategy as the volumes with pool1/type3 will be filtered out in the strategy since the creation of the target list. Similar issue exist for case, am I correct? \n\nFor me the behavior of the overall strategy is correct with this patch although reading only this method can be missleading.","commit_id":"310125ed40038716d410c7da57c9f472f53f8914"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4d3632024279019a131412fcbd13791e56faa9b4","unresolved":false,"context_lines":[{"line_number":398,"context_line":"                        pool.get(\"dst_type\"))"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        # Second pass: fall back to matching just src_pool"},{"line_number":401,"context_line":"        for pool in self.migrate_storage_pools:"},{"line_number":402,"context_line":"            if pool.get(\"src_pool\") \u003d\u003d src_pool:"},{"line_number":403,"context_line":"                return (pool.get(\"dst_pool\"),"},{"line_number":404,"context_line":"                        pool.get(\"dst_type\"))"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"    def volumes_migration(self, volumes, action_counter, instance_targets\u003d[]):"},{"line_number":407,"context_line":"        instance_target_ids \u003d {instance.id for instance in instance_targets}"}],"source_content_type":"text/x-python","patch_set":6,"id":"e0b9221f_06e60ecd","line":404,"range":{"start_line":401,"start_character":0,"end_line":404,"end_character":45},"in_reply_to":"63936abd_bc58079b","updated":"2026-03-05 16:10:23.000000000","message":"Acknowledged","commit_id":"310125ed40038716d410c7da57c9f472f53f8914"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"c02829275d99ed70b42771d2bc63eda02a538f84","unresolved":false,"context_lines":[{"line_number":398,"context_line":"                        pool.get(\"dst_type\"))"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        # Second pass: fall back to matching just src_pool"},{"line_number":401,"context_line":"        for pool in self.migrate_storage_pools:"},{"line_number":402,"context_line":"            if pool.get(\"src_pool\") \u003d\u003d src_pool:"},{"line_number":403,"context_line":"                return (pool.get(\"dst_pool\"),"},{"line_number":404,"context_line":"                        pool.get(\"dst_type\"))"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"    def volumes_migration(self, volumes, action_counter, instance_targets\u003d[]):"},{"line_number":407,"context_line":"        instance_target_ids \u003d {instance.id for instance in instance_targets}"}],"source_content_type":"text/x-python","patch_set":6,"id":"e0611c11_62f485b5","line":404,"range":{"start_line":401,"start_character":0,"end_line":404,"end_character":45},"in_reply_to":"63936abd_bc58079b","updated":"2026-03-05 16:28:24.000000000","message":"Done","commit_id":"310125ed40038716d410c7da57c9f472f53f8914"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"dae63a39192c22685e024ca8fa2b8bc4244967a4","unresolved":true,"context_lines":[{"line_number":398,"context_line":"                        pool.get(\"dst_type\"))"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        # Second pass: fall back to matching just src_pool"},{"line_number":401,"context_line":"        for pool in self.migrate_storage_pools:"},{"line_number":402,"context_line":"            if pool.get(\"src_pool\") \u003d\u003d src_pool:"},{"line_number":403,"context_line":"                return (pool.get(\"dst_pool\"),"},{"line_number":404,"context_line":"                        pool.get(\"dst_type\"))"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"    def volumes_migration(self, volumes, action_counter, instance_targets\u003d[]):"},{"line_number":407,"context_line":"        instance_target_ids \u003d {instance.id for instance in instance_targets}"}],"source_content_type":"text/x-python","patch_set":6,"id":"dddc03aa_20b24bb6","line":404,"range":{"start_line":401,"start_character":0,"end_line":404,"end_character":45},"in_reply_to":"738ab11d_f5160986","updated":"2025-12-11 09:56:40.000000000","message":"I think everything you wrote is correct. As you pointed out, the method implicitely relies on the fact that the strategy will have already filtered out the volumes so it should never receive a (src_pool, src_type) that is not present in the input parameters either as a explicit match of pool and type or just pool. The `get_dst_pool_and_type` method could be written slightly different to instead separate the pools listed in the input into two lists, one with the inputs that have both src_type and src_pool and one with the inputs that only have src_pool\n\n```\nsrc_pool_type_inputs \u003d []\nsrc_pool_inputs \u003d []\nfor pool in self.migrate_storage_pools:\n    if pool.get(\"src_type\") is None:\n        src_pool_inputs.append(pool)\n    else:\n        src_pool_type_inputs.append(pool)\n\npool_to_check \u003d (src_pool, src_type)\n# First pass: prefer exact match (both src_pool and src_type)\nfor pool in src_pool_type_inputs:\n    user_pool \u003d (pool.get(\"src_pool\"), pool.get(\"src_type\"))\n    if pool_to_check \u003d\u003d user_pool:\n        return (pool.get(\"dst_pool\"), pool.get(\"dst_type\"))\n\n# Second pass: fall back to matching just src_pool\nfor pool in src_pool_inputs:\n    if pool.get(\"src_pool\") \u003d\u003d src_pool:\n        return (pool.get(\"dst_pool\"), pool.get(\"dst_type\"))\n\n\n```","commit_id":"310125ed40038716d410c7da57c9f472f53f8914"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"67cfd36e6614cb1a617152999bc9e3709a327c43","unresolved":true,"context_lines":[{"line_number":398,"context_line":"                        pool.get(\"dst_type\"))"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        # Second pass: fall back to matching just src_pool"},{"line_number":401,"context_line":"        for pool in self.migrate_storage_pools:"},{"line_number":402,"context_line":"            if pool.get(\"src_pool\") \u003d\u003d src_pool:"},{"line_number":403,"context_line":"                return (pool.get(\"dst_pool\"),"},{"line_number":404,"context_line":"                        pool.get(\"dst_type\"))"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"    def volumes_migration(self, volumes, action_counter, instance_targets\u003d[]):"},{"line_number":407,"context_line":"        instance_target_ids \u003d {instance.id for instance in instance_targets}"}],"source_content_type":"text/x-python","patch_set":6,"id":"63936abd_bc58079b","line":404,"range":{"start_line":401,"start_character":0,"end_line":404,"end_character":45},"in_reply_to":"b92c6cd3_a227f648","updated":"2025-12-11 11:16:44.000000000","message":"ack, I agree it\u0027s more clear, I\u0027ve changed it in patchset 7","commit_id":"310125ed40038716d410c7da57c9f472f53f8914"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"edec068b7c4ae9d7a92441b67d2cb594e847edd6","unresolved":true,"context_lines":[{"line_number":398,"context_line":"                        pool.get(\"dst_type\"))"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        # Second pass: fall back to matching just src_pool"},{"line_number":401,"context_line":"        for pool in self.migrate_storage_pools:"},{"line_number":402,"context_line":"            if pool.get(\"src_pool\") \u003d\u003d src_pool:"},{"line_number":403,"context_line":"                return (pool.get(\"dst_pool\"),"},{"line_number":404,"context_line":"                        pool.get(\"dst_type\"))"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"    def volumes_migration(self, volumes, action_counter, instance_targets\u003d[]):"},{"line_number":407,"context_line":"        instance_target_ids \u003d {instance.id for instance in instance_targets}"}],"source_content_type":"text/x-python","patch_set":6,"id":"b92c6cd3_a227f648","line":404,"range":{"start_line":401,"start_character":0,"end_line":404,"end_character":45},"in_reply_to":"dddc03aa_20b24bb6","updated":"2025-12-11 10:59:36.000000000","message":"Personally, I like more this proposal as it would return None in case that no src_pool, src_type combination matches the volume and there is no untyped pool config. IMO it is more accurate.","commit_id":"310125ed40038716d410c7da57c9f472f53f8914"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4d3632024279019a131412fcbd13791e56faa9b4","unresolved":true,"context_lines":[{"line_number":386,"context_line":""},{"line_number":387,"context_line":"        :param src_pool: storage pool name"},{"line_number":388,"context_line":"        :param src_type: storage volume type"},{"line_number":389,"context_line":"        :returns: set of storage des pool name and dst volume type name, any"},{"line_number":390,"context_line":"        of those values can be None if no match for it is found in the"},{"line_number":391,"context_line":"        input_parameters"},{"line_number":392,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"0c3a71ad_78aa2283","line":389,"range":{"start_line":389,"start_character":33,"end_line":389,"end_character":36},"updated":"2026-03-05 16:10:23.000000000","message":"nit: dest or dst for consitency","commit_id":"1d561e2fa1522b976dba51ad91a10551a69551b5"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"c02829275d99ed70b42771d2bc63eda02a538f84","unresolved":false,"context_lines":[{"line_number":386,"context_line":""},{"line_number":387,"context_line":"        :param src_pool: storage pool name"},{"line_number":388,"context_line":"        :param src_type: storage volume type"},{"line_number":389,"context_line":"        :returns: set of storage des pool name and dst volume type name, any"},{"line_number":390,"context_line":"        of those values can be None if no match for it is found in the"},{"line_number":391,"context_line":"        input_parameters"},{"line_number":392,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"34c7d54f_aa932ed7","line":389,"range":{"start_line":389,"start_character":33,"end_line":389,"end_character":36},"in_reply_to":"0c3a71ad_78aa2283","updated":"2026-03-05 16:28:24.000000000","message":"Done","commit_id":"1d561e2fa1522b976dba51ad91a10551a69551b5"}],"watcher/tests/decision_engine/strategy/strategies/test_zone_migration.py":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"6f163d5514134506053622b8359f9d8a9547e73a","unresolved":true,"context_lines":[{"line_number":1597,"context_line":"            }"},{"line_number":1598,"context_line":"        ]"},{"line_number":1599,"context_line":"        pool \u003d \"src1@back1#pool1\""},{"line_number":1600,"context_line":"        src_type \u003d \"type2\""},{"line_number":1601,"context_line":"        dst_pool, dst_type \u003d self.strategy.get_dst_pool_and_type("},{"line_number":1602,"context_line":"            pool, src_type"},{"line_number":1603,"context_line":"        )"},{"line_number":1604,"context_line":"        self.assertEqual(dst_pool, \"dst3@back1#pool1\")"},{"line_number":1605,"context_line":"        self.assertEqual(dst_type, \"type3\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"c52dade7_20494fb9","line":1605,"range":{"start_line":1600,"start_character":0,"end_line":1605,"end_character":43},"updated":"2025-12-05 08:40:17.000000000","message":"Not blocker, but I would validate the expected result also for pool\u003dsrc1@back1#pool1 and src_type\u003dtypeX in this case. You test the fallback case (no src_type) in previous test but this case is slighly different because of the duplicate src_pool.\n\nAlso I\u0027d add a separate case with:\n\n```\n self.input_parameters[\"storage_pools\"] \u003d [\n            {\n                \"src_pool\": \"src1@back1#pool1\",\n                \"dst_pool\": \"dst1@back1#pool1\",\n                \"src_type\": \"type1\", \"dst_type\": \"type4\", \n            },\n            {\n                \"src_pool\": \"src1@back1#pool1\",\n                \"dst_pool\": \"dst3@back1#pool1\",\n                \"src_type\": \"type2\", \"dst_type\": \"type3\"\n            }\n        ]\n```\n\n\nTo validate the behavior for duplicated src_pools with explicit different src_types.","commit_id":"8e0dc5a804bcd2e228a8e404fa093d089de2ed59"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"97f6d3023621d488d0bed9ccaa74527dc97c482b","unresolved":true,"context_lines":[{"line_number":1597,"context_line":"            }"},{"line_number":1598,"context_line":"        ]"},{"line_number":1599,"context_line":"        pool \u003d \"src1@back1#pool1\""},{"line_number":1600,"context_line":"        src_type \u003d \"type2\""},{"line_number":1601,"context_line":"        dst_pool, dst_type \u003d self.strategy.get_dst_pool_and_type("},{"line_number":1602,"context_line":"            pool, src_type"},{"line_number":1603,"context_line":"        )"},{"line_number":1604,"context_line":"        self.assertEqual(dst_pool, \"dst3@back1#pool1\")"},{"line_number":1605,"context_line":"        self.assertEqual(dst_type, \"type3\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"90c63203_3fa856bd","line":1605,"range":{"start_line":1600,"start_character":0,"end_line":1605,"end_character":43},"in_reply_to":"7bf0a4bc_75d33d49","updated":"2025-12-08 22:14:05.000000000","message":"this is fine as it is not ambaiguous however we cant resize and migrate in one api action so im not conved we shoudl allow this.\n\nim inlcined to say this should also be a 400","commit_id":"8e0dc5a804bcd2e228a8e404fa093d089de2ed59"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"1955a7eecb3b46a9fb9f2f527dd2e5edf3be92da","unresolved":true,"context_lines":[{"line_number":1597,"context_line":"            }"},{"line_number":1598,"context_line":"        ]"},{"line_number":1599,"context_line":"        pool \u003d \"src1@back1#pool1\""},{"line_number":1600,"context_line":"        src_type \u003d \"type2\""},{"line_number":1601,"context_line":"        dst_pool, dst_type \u003d self.strategy.get_dst_pool_and_type("},{"line_number":1602,"context_line":"            pool, src_type"},{"line_number":1603,"context_line":"        )"},{"line_number":1604,"context_line":"        self.assertEqual(dst_pool, \"dst3@back1#pool1\")"},{"line_number":1605,"context_line":"        self.assertEqual(dst_type, \"type3\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"7bf0a4bc_75d33d49","line":1605,"range":{"start_line":1600,"start_character":0,"end_line":1605,"end_character":43},"in_reply_to":"c52dade7_20494fb9","updated":"2025-12-05 09:02:24.000000000","message":"those are good points, I\u0027ve added the extra checks, thanks!","commit_id":"8e0dc5a804bcd2e228a8e404fa093d089de2ed59"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"97f6d3023621d488d0bed9ccaa74527dc97c482b","unresolved":true,"context_lines":[{"line_number":1595,"context_line":"                \"dst_pool\": \"dst3@back1#pool1\","},{"line_number":1596,"context_line":"                \"src_type\": \"type2\", \"dst_type\": \"type3\""},{"line_number":1597,"context_line":"            }"},{"line_number":1598,"context_line":"        ]"},{"line_number":1599,"context_line":"        pool \u003d \"src1@back1#pool1\""},{"line_number":1600,"context_line":"        src_type \u003d \"type2\""},{"line_number":1601,"context_line":"        dst_pool, dst_type \u003d self.strategy.get_dst_pool_and_type("}],"source_content_type":"text/x-python","patch_set":4,"id":"8faf940a_8206107d","line":1598,"updated":"2025-12-08 22:14:05.000000000","message":"as noted in teh repoducer this is ambiguase and shoudl result in a 400 error\n\nbecause we cannot determin where voluem of source type \"type2\"\n\nshoudl move too.\n\nif we allwo the same souce pool to exist twice or more in the same request it\nmust be unambiguous so this shoudl be a 400 bad request","commit_id":"eec22e000cd8dc4a27e052697e472f872c03a81c"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"e872cfceeeeaa12b9d32ad8c357d80bd6903ffbf","unresolved":true,"context_lines":[{"line_number":1607,"context_line":"            pool, src_type"},{"line_number":1608,"context_line":"        )"},{"line_number":1609,"context_line":"        self.assertIsNone(dst_pool)"},{"line_number":1610,"context_line":"        self.assertEqual(dst_type, \"type3\")"}],"source_content_type":"text/x-python","patch_set":6,"id":"52023b33_45d8fbcc","line":1610,"range":{"start_line":1610,"start_character":0,"end_line":1610,"end_character":2},"updated":"2025-12-11 09:15:09.000000000","message":"I\u0027d validate the request for type1 too.\n\nAlso, IIUC, we also want to cover the case `[{\"src_pool\": pool1, \"src_type\": type1}, {\"src_pool\": pool1}]` , where second config acts as fallback for volumes in pool1 and type !\u003d type1 so I would add another test for it.","commit_id":"310125ed40038716d410c7da57c9f472f53f8914"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"dae63a39192c22685e024ca8fa2b8bc4244967a4","unresolved":true,"context_lines":[{"line_number":1607,"context_line":"            pool, src_type"},{"line_number":1608,"context_line":"        )"},{"line_number":1609,"context_line":"        self.assertIsNone(dst_pool)"},{"line_number":1610,"context_line":"        self.assertEqual(dst_type, \"type3\")"}],"source_content_type":"text/x-python","patch_set":6,"id":"f20c2b81_64ce3e73","line":1610,"range":{"start_line":1610,"start_character":0,"end_line":1610,"end_character":2},"in_reply_to":"52023b33_45d8fbcc","updated":"2025-12-11 09:56:40.000000000","message":"I\u0027ve added the check for type1. about the second case, there is some ambiguity in an input like that we had some discussion with @smooney@redhat.com in the parent patch https://review.opendev.org/c/openstack/watcher/+/964717/comment/c04cb37c_90ba6e1e/","commit_id":"310125ed40038716d410c7da57c9f472f53f8914"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"67cfd36e6614cb1a617152999bc9e3709a327c43","unresolved":true,"context_lines":[{"line_number":1607,"context_line":"            pool, src_type"},{"line_number":1608,"context_line":"        )"},{"line_number":1609,"context_line":"        self.assertIsNone(dst_pool)"},{"line_number":1610,"context_line":"        self.assertEqual(dst_type, \"type3\")"}],"source_content_type":"text/x-python","patch_set":6,"id":"8acd3dbe_c8d3991b","line":1610,"range":{"start_line":1610,"start_character":0,"end_line":1610,"end_character":2},"in_reply_to":"ba73aee2_8026ac33","updated":"2025-12-11 11:16:44.000000000","message":"hmm I think you might be right and I misread that comment. I\u0027ve updated the tests and included your suggested test case, thanks Alfredo!","commit_id":"310125ed40038716d410c7da57c9f472f53f8914"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"edec068b7c4ae9d7a92441b67d2cb594e847edd6","unresolved":true,"context_lines":[{"line_number":1607,"context_line":"            pool, src_type"},{"line_number":1608,"context_line":"        )"},{"line_number":1609,"context_line":"        self.assertIsNone(dst_pool)"},{"line_number":1610,"context_line":"        self.assertEqual(dst_type, \"type3\")"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba73aee2_8026ac33","line":1610,"range":{"start_line":1610,"start_character":0,"end_line":1610,"end_character":2},"in_reply_to":"f20c2b81_64ce3e73","updated":"2025-12-11 10:59:36.000000000","message":"IIUC that comment was about ambiguity in the destination, not in source. The case I\u0027m missing in the tests would be:\n\n```\nself.input_parameters[\"storage_pools\"] \u003d [\n            {\n                \"src_pool\": \"src1@back1#pool1\",\n                \"src_type\": \"type1\",\n                \"dst_pool\": \"dst1@back1#pool1\",\n            },\n            {\n                \"src_pool\": \"src1@back1#pool1\",\n                \"dst_pool\": \"dst2@back2#pool2\"\n            }\n```\n\nI think that\u0027s not covered in the test and would be a valid case. All volumes in type1 in pool src1@back1#pool1 should be migrated to dst1@back1#pool1 while the rest of volumes in the same pool would be migrated to dst2@back2#pool2","commit_id":"310125ed40038716d410c7da57c9f472f53f8914"}],"watcher/tests/unit/decision_engine/strategy/strategies/test_zone_migration.py":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"fc7362ddb263ab2aff0f25e511250f83b592ea08","unresolved":true,"context_lines":[{"line_number":1570,"context_line":"            pool, src_type"},{"line_number":1571,"context_line":"        )"},{"line_number":1572,"context_line":"        self.assertEqual(dst_pool, \"dst1@back1#pool1\")"},{"line_number":1573,"context_line":"        self.assertIsNone(dst_type, \"type1\")"},{"line_number":1574,"context_line":""},{"line_number":1575,"context_line":"    def test_get_dst_pool_and_type_matching_src_pool(self):"},{"line_number":1576,"context_line":"        self.input_parameters[\"storage_pools\"] \u003d ["}],"source_content_type":"text/x-python","patch_set":9,"id":"e28d8914_7532d995","line":1573,"range":{"start_line":1573,"start_character":37,"end_line":1573,"end_character":42},"updated":"2026-03-05 15:46:49.000000000","message":"This additional parameter would be the error message, but I think that you added by mistake here.","commit_id":"1d561e2fa1522b976dba51ad91a10551a69551b5"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"c02829275d99ed70b42771d2bc63eda02a538f84","unresolved":true,"context_lines":[{"line_number":1570,"context_line":"            pool, src_type"},{"line_number":1571,"context_line":"        )"},{"line_number":1572,"context_line":"        self.assertEqual(dst_pool, \"dst1@back1#pool1\")"},{"line_number":1573,"context_line":"        self.assertIsNone(dst_type, \"type1\")"},{"line_number":1574,"context_line":""},{"line_number":1575,"context_line":"    def test_get_dst_pool_and_type_matching_src_pool(self):"},{"line_number":1576,"context_line":"        self.input_parameters[\"storage_pools\"] \u003d ["}],"source_content_type":"text/x-python","patch_set":9,"id":"5e200194_bc5f3be4","line":1573,"range":{"start_line":1573,"start_character":37,"end_line":1573,"end_character":42},"in_reply_to":"0dd532cd_54d3e0f7","updated":"2026-03-05 16:28:24.000000000","message":"the correct call here is `self.assertIsNone(dst_type)`. I\u0027ve removed the extra argument","commit_id":"1d561e2fa1522b976dba51ad91a10551a69551b5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4d3632024279019a131412fcbd13791e56faa9b4","unresolved":true,"context_lines":[{"line_number":1570,"context_line":"            pool, src_type"},{"line_number":1571,"context_line":"        )"},{"line_number":1572,"context_line":"        self.assertEqual(dst_pool, \"dst1@back1#pool1\")"},{"line_number":1573,"context_line":"        self.assertIsNone(dst_type, \"type1\")"},{"line_number":1574,"context_line":""},{"line_number":1575,"context_line":"    def test_get_dst_pool_and_type_matching_src_pool(self):"},{"line_number":1576,"context_line":"        self.input_parameters[\"storage_pools\"] \u003d ["}],"source_content_type":"text/x-python","patch_set":9,"id":"0dd532cd_54d3e0f7","line":1573,"range":{"start_line":1573,"start_character":37,"end_line":1573,"end_character":42},"in_reply_to":"e28d8914_7532d995","updated":"2026-03-05 16:10:23.000000000","message":"yep we shoudl  fix this\ni think the inte was to use assertEqual here not assertIsNone","commit_id":"1d561e2fa1522b976dba51ad91a10551a69551b5"}]}