)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"4d0371f647215664cacf74e611f022673f48bb18","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"570b917a_6154ea21","updated":"2025-10-13 16:46:00.000000000","message":"check-rdo","commit_id":"413358d64a65054e614c7316d1f7e1a2f28b2414"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"18bb7bc2bc9fae3bb6450430f98cd99cd894dde2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"54d9b638_18cf219d","updated":"2025-10-20 19:43:00.000000000","message":"-1: not sure if I understood the extra_specs and capabilities checking in the code. Waiting for your inputs there. Thanks!","commit_id":"6b9bde87e983e84d79a363657baaa791df92a5a1"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"5c4aec6f7e4032dce6c172f1559cb45d7e9d75d7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"8c60ebbc_edd46b77","updated":"2026-02-12 16:40:42.000000000","message":"recheck unrelated test failure","commit_id":"22fd676d793b7dba260459f84dc5c48bbbe9ecf4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c82fb6ad90cca803b201de585ad9565980dde245","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"e6674e23_f9325dbf","updated":"2026-03-05 15:34:33.000000000","message":"some minior nits but we can clean it up in a followup\noverall this looks ok","commit_id":"22fd676d793b7dba260459f84dc5c48bbbe9ecf4"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"5c3119da2eef806e9959311f838d0bff12235bb9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"95631677_2c6cdf17","updated":"2026-03-05 17:52:37.000000000","message":"I think tha we still need to do some more testing wrt to pool and extra-specs capacilities, but for now we can proceed as is, which fix the related bug.\nThanks Joan","commit_id":"b3595d77a755645d68332a9f153c5596b295e1d3"}],"watcher/common/cinder_helper.py":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"18bb7bc2bc9fae3bb6450430f98cd99cd894dde2","unresolved":true,"context_lines":[{"line_number":131,"context_line":"                # it can be used in any pool"},{"line_number":132,"context_line":"                pool_volume_types.append(volume_type.name)"},{"line_number":133,"context_line":"                continue"},{"line_number":134,"context_line":"            for field_name, field_value in volume_type.extra_specs.items():"},{"line_number":135,"context_line":"                if field_value \u003d\u003d pool[\"capabilities\"].get(field_name):"},{"line_number":136,"context_line":"                    LOG.debug("},{"line_number":137,"context_line":"                        f\"property {field_name} with value {field_value} \""},{"line_number":138,"context_line":"                        f\"is found in pool {pool[\u0027name\u0027]}\""},{"line_number":139,"context_line":"                    )"},{"line_number":140,"context_line":"                    # the property associated with the volume type is"},{"line_number":141,"context_line":"                    # also defined in the pool, so the type can be used in the"},{"line_number":142,"context_line":"                    # pool"},{"line_number":143,"context_line":"                    pool_volume_types.append(volume_type.name)"},{"line_number":144,"context_line":"                    continue"},{"line_number":145,"context_line":"        return pool_volume_types"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    def check_volume_deleted(self, volume, retry\u003d120, retry_interval\u003d10):"}],"source_content_type":"text/x-python","patch_set":2,"id":"b620caae_2097ead3","line":144,"range":{"start_line":134,"start_character":0,"end_line":144,"end_character":27},"updated":"2025-10-20 19:43:00.000000000","message":"So here you break the loop if the first capability is equal, but what about the other ones?","commit_id":"6b9bde87e983e84d79a363657baaa791df92a5a1"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"c9df894cbae5d562a91e063864245b9f48eb90c5","unresolved":true,"context_lines":[{"line_number":131,"context_line":"                # it can be used in any pool"},{"line_number":132,"context_line":"                pool_volume_types.append(volume_type.name)"},{"line_number":133,"context_line":"                continue"},{"line_number":134,"context_line":"            for field_name, field_value in volume_type.extra_specs.items():"},{"line_number":135,"context_line":"                if field_value \u003d\u003d pool[\"capabilities\"].get(field_name):"},{"line_number":136,"context_line":"                    LOG.debug("},{"line_number":137,"context_line":"                        f\"property {field_name} with value {field_value} \""},{"line_number":138,"context_line":"                        f\"is found in pool {pool[\u0027name\u0027]}\""},{"line_number":139,"context_line":"                    )"},{"line_number":140,"context_line":"                    # the property associated with the volume type is"},{"line_number":141,"context_line":"                    # also defined in the pool, so the type can be used in the"},{"line_number":142,"context_line":"                    # pool"},{"line_number":143,"context_line":"                    pool_volume_types.append(volume_type.name)"},{"line_number":144,"context_line":"                    continue"},{"line_number":145,"context_line":"        return pool_volume_types"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    def check_volume_deleted(self, volume, retry\u003d120, retry_interval\u003d10):"}],"source_content_type":"text/x-python","patch_set":2,"id":"6a455723_40beaf1e","line":144,"range":{"start_line":134,"start_character":0,"end_line":144,"end_character":27},"in_reply_to":"39d975a2_3b04966f","updated":"2025-10-21 11:46:00.000000000","message":"Still doesn\u0027t look that you are doing the same, in the code that you linked, there are 2 for loops, with the inner one that breaks when finds the capability.\nhttps://github.com/openstack/cinder/blob/d02171164bdd702b12b59888b744d172f30d712d/cinder/scheduler/filters/capabilities_filter.py#L56-L83","commit_id":"6b9bde87e983e84d79a363657baaa791df92a5a1"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"ebd35bab171d48a8ada02417a9a0050598e48efa","unresolved":true,"context_lines":[{"line_number":131,"context_line":"                # it can be used in any pool"},{"line_number":132,"context_line":"                pool_volume_types.append(volume_type.name)"},{"line_number":133,"context_line":"                continue"},{"line_number":134,"context_line":"            for field_name, field_value in volume_type.extra_specs.items():"},{"line_number":135,"context_line":"                if field_value \u003d\u003d pool[\"capabilities\"].get(field_name):"},{"line_number":136,"context_line":"                    LOG.debug("},{"line_number":137,"context_line":"                        f\"property {field_name} with value {field_value} \""},{"line_number":138,"context_line":"                        f\"is found in pool {pool[\u0027name\u0027]}\""},{"line_number":139,"context_line":"                    )"},{"line_number":140,"context_line":"                    # the property associated with the volume type is"},{"line_number":141,"context_line":"                    # also defined in the pool, so the type can be used in the"},{"line_number":142,"context_line":"                    # pool"},{"line_number":143,"context_line":"                    pool_volume_types.append(volume_type.name)"},{"line_number":144,"context_line":"                    continue"},{"line_number":145,"context_line":"        return pool_volume_types"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    def check_volume_deleted(self, volume, retry\u003d120, retry_interval\u003d10):"}],"source_content_type":"text/x-python","patch_set":2,"id":"71de0c42_fae6cf87","line":144,"range":{"start_line":134,"start_character":0,"end_line":144,"end_character":27},"in_reply_to":"6a455723_40beaf1e","updated":"2025-10-21 13:13:52.000000000","message":"ok, you\u0027re right, I see it now, that code checks that all extra_specs are matched in the backend, and we should do the same, let me fix the patch","commit_id":"6b9bde87e983e84d79a363657baaa791df92a5a1"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"acb4566760a6b4fa5c2df076b616081ed563d348","unresolved":false,"context_lines":[{"line_number":131,"context_line":"                # it can be used in any pool"},{"line_number":132,"context_line":"                pool_volume_types.append(volume_type.name)"},{"line_number":133,"context_line":"                continue"},{"line_number":134,"context_line":"            for field_name, field_value in volume_type.extra_specs.items():"},{"line_number":135,"context_line":"                if field_value \u003d\u003d pool[\"capabilities\"].get(field_name):"},{"line_number":136,"context_line":"                    LOG.debug("},{"line_number":137,"context_line":"                        f\"property {field_name} with value {field_value} \""},{"line_number":138,"context_line":"                        f\"is found in pool {pool[\u0027name\u0027]}\""},{"line_number":139,"context_line":"                    )"},{"line_number":140,"context_line":"                    # the property associated with the volume type is"},{"line_number":141,"context_line":"                    # also defined in the pool, so the type can be used in the"},{"line_number":142,"context_line":"                    # pool"},{"line_number":143,"context_line":"                    pool_volume_types.append(volume_type.name)"},{"line_number":144,"context_line":"                    continue"},{"line_number":145,"context_line":"        return pool_volume_types"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    def check_volume_deleted(self, volume, retry\u003d120, retry_interval\u003d10):"}],"source_content_type":"text/x-python","patch_set":2,"id":"8cedd011_657d4fd9","line":144,"range":{"start_line":134,"start_character":0,"end_line":144,"end_character":27},"in_reply_to":"71de0c42_fae6cf87","updated":"2026-02-12 14:31:07.000000000","message":"Done","commit_id":"6b9bde87e983e84d79a363657baaa791df92a5a1"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"b1e6b2df4c5ef442c505a6a135aa0364633d3ed2","unresolved":true,"context_lines":[{"line_number":131,"context_line":"                # it can be used in any pool"},{"line_number":132,"context_line":"                pool_volume_types.append(volume_type.name)"},{"line_number":133,"context_line":"                continue"},{"line_number":134,"context_line":"            for field_name, field_value in volume_type.extra_specs.items():"},{"line_number":135,"context_line":"                if field_value \u003d\u003d pool[\"capabilities\"].get(field_name):"},{"line_number":136,"context_line":"                    LOG.debug("},{"line_number":137,"context_line":"                        f\"property {field_name} with value {field_value} \""},{"line_number":138,"context_line":"                        f\"is found in pool {pool[\u0027name\u0027]}\""},{"line_number":139,"context_line":"                    )"},{"line_number":140,"context_line":"                    # the property associated with the volume type is"},{"line_number":141,"context_line":"                    # also defined in the pool, so the type can be used in the"},{"line_number":142,"context_line":"                    # pool"},{"line_number":143,"context_line":"                    pool_volume_types.append(volume_type.name)"},{"line_number":144,"context_line":"                    continue"},{"line_number":145,"context_line":"        return pool_volume_types"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    def check_volume_deleted(self, volume, retry\u003d120, retry_interval\u003d10):"}],"source_content_type":"text/x-python","patch_set":2,"id":"39d975a2_3b04966f","line":144,"range":{"start_line":134,"start_character":0,"end_line":144,"end_character":27},"in_reply_to":"b620caae_2097ead3","updated":"2025-10-21 07:59:25.000000000","message":"I that as long as one extra_spec from the extra_spec matched the backend capabilities, it was enough for the volume to be scheduled in the backend. I\u0027m not sure if this is correct, so I looked at the `CapabilitiesFilter` filter from the cinder scheduler https://github.com/openstack/cinder/blob/d02171164bdd702b12b59888b744d172f30d712d/cinder/scheduler/filters/capabilities_filter.py#L81 and it seems to do the same, loop over the capabilities and return a match as soon as one capability matches the type extra_specs","commit_id":"6b9bde87e983e84d79a363657baaa791df92a5a1"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"54525cf2d0e5aa0973ea3fe76c5dfe6a2d75a771","unresolved":true,"context_lines":[{"line_number":133,"context_line":"        \"\"\""},{"line_number":134,"context_line":"        for field_name, field_value in volume_type.extra_specs.items():"},{"line_number":135,"context_line":"            pool_value \u003d pool[\"capabilities\"].get(field_name)"},{"line_number":136,"context_line":"            if field_value !\u003d pool_value:"},{"line_number":137,"context_line":"                # the property associated with the volume type is"},{"line_number":138,"context_line":"                # not defined in the pool, so the type can\u0027t be used in the"},{"line_number":139,"context_line":"                # pool"}],"source_content_type":"text/x-python","patch_set":4,"id":"5859fad7_d88c7174","line":136,"range":{"start_line":136,"start_character":0,"end_line":136,"end_character":41},"updated":"2025-11-12 17:35:03.000000000","message":"the way that capabilites are validated can be more complex I think[1]. Not sure if we should adapt or relax the checking here.\n\n[1] https://github.com/openstack/cinder/blob/763fec10d0a84e7e9014fcc2fce292b792e3f9ce/cinder/scheduler/filters/extra_specs_ops.py","commit_id":"55f193192e458c75d0255526b80a3ca4487bd455"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"8a3b89f59b879f2b813284e3fff0954e0c4d132c","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        \"\"\""},{"line_number":134,"context_line":"        for field_name, field_value in volume_type.extra_specs.items():"},{"line_number":135,"context_line":"            pool_value \u003d pool[\"capabilities\"].get(field_name)"},{"line_number":136,"context_line":"            if field_value !\u003d pool_value:"},{"line_number":137,"context_line":"                # the property associated with the volume type is"},{"line_number":138,"context_line":"                # not defined in the pool, so the type can\u0027t be used in the"},{"line_number":139,"context_line":"                # pool"}],"source_content_type":"text/x-python","patch_set":4,"id":"30942e10_b633453f","line":136,"range":{"start_line":136,"start_character":0,"end_line":136,"end_character":41},"in_reply_to":"1deb8431_09d3e99e","updated":"2026-03-05 16:16:40.000000000","message":"Done","commit_id":"55f193192e458c75d0255526b80a3ca4487bd455"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"542e7a1adc6aba4e2ba08af26adac141b8901ffb","unresolved":true,"context_lines":[{"line_number":133,"context_line":"        \"\"\""},{"line_number":134,"context_line":"        for field_name, field_value in volume_type.extra_specs.items():"},{"line_number":135,"context_line":"            pool_value \u003d pool[\"capabilities\"].get(field_name)"},{"line_number":136,"context_line":"            if field_value !\u003d pool_value:"},{"line_number":137,"context_line":"                # the property associated with the volume type is"},{"line_number":138,"context_line":"                # not defined in the pool, so the type can\u0027t be used in the"},{"line_number":139,"context_line":"                # pool"}],"source_content_type":"text/x-python","patch_set":4,"id":"8a7c3e47_f921c968","line":136,"range":{"start_line":136,"start_character":0,"end_line":136,"end_character":41},"in_reply_to":"5859fad7_d88c7174","updated":"2025-11-18 15:24:30.000000000","message":"I think implementing a \"relaxed\" check from watcher side is probably what we can do from watcher side. In the usual OpenStack deployment, I\u0027d say that the list of pools and volume_types and extra_specs is pretty static and something that the admin should know when explicitely defining the source and destination pools and types (which IIUC is the only case where we may hit issues with compatibility). In the worst case, cinder will simply fail the migration.","commit_id":"55f193192e458c75d0255526b80a3ca4487bd455"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6f44ab7d8353bf9928e07217ca12a700c724af35","unresolved":true,"context_lines":[{"line_number":133,"context_line":"        \"\"\""},{"line_number":134,"context_line":"        for field_name, field_value in volume_type.extra_specs.items():"},{"line_number":135,"context_line":"            pool_value \u003d pool[\"capabilities\"].get(field_name)"},{"line_number":136,"context_line":"            if field_value !\u003d pool_value:"},{"line_number":137,"context_line":"                # the property associated with the volume type is"},{"line_number":138,"context_line":"                # not defined in the pool, so the type can\u0027t be used in the"},{"line_number":139,"context_line":"                # pool"}],"source_content_type":"text/x-python","patch_set":4,"id":"1deb8431_09d3e99e","line":136,"range":{"start_line":136,"start_character":0,"end_line":136,"end_character":41},"in_reply_to":"8a7c3e47_f921c968","updated":"2026-02-12 13:30:14.000000000","message":"im inclidned to say that we shoudl not try to second guess cidner here and let it do the main check and ya have a relaxed/besteffort check on our side.\n\nif its not compatibale that is fine cinder will reject it when we try to do the migrate.\n\nill also note that there are other wasy that do not use extra specs  to mapp pools and voluem types such as AZs and backend configs\n\n\nso we cant actully check all the cases in watcher.","commit_id":"55f193192e458c75d0255526b80a3ca4487bd455"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"54525cf2d0e5aa0973ea3fe76c5dfe6a2d75a771","unresolved":true,"context_lines":[{"line_number":131,"context_line":"        :returns: True if pool matches all volume type requirements,"},{"line_number":132,"context_line":"                  False otherwise"},{"line_number":133,"context_line":"        \"\"\""},{"line_number":134,"context_line":"        for field_name, field_value in volume_type.extra_specs.items():"},{"line_number":135,"context_line":"            pool_value \u003d pool[\"capabilities\"].get(field_name)"},{"line_number":136,"context_line":"            if field_value !\u003d pool_value:"},{"line_number":137,"context_line":"                # the property associated with the volume type is"},{"line_number":138,"context_line":"                # not defined in the pool, so the type can\u0027t be used in the"},{"line_number":139,"context_line":"                # pool"},{"line_number":140,"context_line":"                LOG.debug("},{"line_number":141,"context_line":"                    f\"property {field_name} with value {field_value} \""},{"line_number":142,"context_line":"                    f\"does not match value {pool_value} from pool \""},{"line_number":143,"context_line":"                    f\"{pool[\u0027name\u0027]}\""},{"line_number":144,"context_line":"                )"},{"line_number":145,"context_line":"                return False"},{"line_number":146,"context_line":"        return True"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    def get_volume_types_for_pool(self, pool):"},{"line_number":149,"context_line":"        \"\"\"Return a list of volume types that can be associated with a pool."}],"source_content_type":"text/x-python","patch_set":4,"id":"f0561da5_2822a10a","line":146,"range":{"start_line":134,"start_character":0,"end_line":146,"end_character":19},"updated":"2025-11-12 17:35:03.000000000","message":"in cinder scheduler, this check seems more complex[1], like dropping some vendor specific capabilities, so we may need to check with some cinder reviewer if this validation requires more work, or maybe just relax it to avoid too many cinder logic here? wdyt?\n\n[1] https://github.com/openstack/cinder/blob/763fec10d0a84e7e9014fcc2fce292b792e3f9ce/cinder/scheduler/filters/capabilities_filter.py#L28-L91","commit_id":"55f193192e458c75d0255526b80a3ca4487bd455"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"542e7a1adc6aba4e2ba08af26adac141b8901ffb","unresolved":true,"context_lines":[{"line_number":131,"context_line":"        :returns: True if pool matches all volume type requirements,"},{"line_number":132,"context_line":"                  False otherwise"},{"line_number":133,"context_line":"        \"\"\""},{"line_number":134,"context_line":"        for field_name, field_value in volume_type.extra_specs.items():"},{"line_number":135,"context_line":"            pool_value \u003d pool[\"capabilities\"].get(field_name)"},{"line_number":136,"context_line":"            if field_value !\u003d pool_value:"},{"line_number":137,"context_line":"                # the property associated with the volume type is"},{"line_number":138,"context_line":"                # not defined in the pool, so the type can\u0027t be used in the"},{"line_number":139,"context_line":"                # pool"},{"line_number":140,"context_line":"                LOG.debug("},{"line_number":141,"context_line":"                    f\"property {field_name} with value {field_value} \""},{"line_number":142,"context_line":"                    f\"does not match value {pool_value} from pool \""},{"line_number":143,"context_line":"                    f\"{pool[\u0027name\u0027]}\""},{"line_number":144,"context_line":"                )"},{"line_number":145,"context_line":"                return False"},{"line_number":146,"context_line":"        return True"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    def get_volume_types_for_pool(self, pool):"},{"line_number":149,"context_line":"        \"\"\"Return a list of volume types that can be associated with a pool."}],"source_content_type":"text/x-python","patch_set":4,"id":"de90dc9a_b18f11e2","line":146,"range":{"start_line":134,"start_character":0,"end_line":146,"end_character":19},"in_reply_to":"01cc391c_4acb244c","updated":"2025-11-18 15:24:30.000000000","message":"Given that Cinder has their own logic to check compatibility between type and pools, I wonder if there is really a good reason to check it by ourselves at the migrate execution time. In this case, we are not adding much value, the result is the same, the action will simply fail at execution whether in our check or by cinder and cinder knows it better than us.\n\nI would see more value in checking compatibility between type and pool in the strategies to validate that parameters dst_pool and dst_type are compatible and fail on the audit execution otherwise, not in the action execution itself. Actually.","commit_id":"55f193192e458c75d0255526b80a3ca4487bd455"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6f44ab7d8353bf9928e07217ca12a700c724af35","unresolved":true,"context_lines":[{"line_number":131,"context_line":"        :returns: True if pool matches all volume type requirements,"},{"line_number":132,"context_line":"                  False otherwise"},{"line_number":133,"context_line":"        \"\"\""},{"line_number":134,"context_line":"        for field_name, field_value in volume_type.extra_specs.items():"},{"line_number":135,"context_line":"            pool_value \u003d pool[\"capabilities\"].get(field_name)"},{"line_number":136,"context_line":"            if field_value !\u003d pool_value:"},{"line_number":137,"context_line":"                # the property associated with the volume type is"},{"line_number":138,"context_line":"                # not defined in the pool, so the type can\u0027t be used in the"},{"line_number":139,"context_line":"                # pool"},{"line_number":140,"context_line":"                LOG.debug("},{"line_number":141,"context_line":"                    f\"property {field_name} with value {field_value} \""},{"line_number":142,"context_line":"                    f\"does not match value {pool_value} from pool \""},{"line_number":143,"context_line":"                    f\"{pool[\u0027name\u0027]}\""},{"line_number":144,"context_line":"                )"},{"line_number":145,"context_line":"                return False"},{"line_number":146,"context_line":"        return True"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    def get_volume_types_for_pool(self, pool):"},{"line_number":149,"context_line":"        \"\"\"Return a list of volume types that can be associated with a pool."}],"source_content_type":"text/x-python","patch_set":4,"id":"f2876a0b_b0fd3c5d","line":146,"range":{"start_line":134,"start_character":0,"end_line":146,"end_character":19},"in_reply_to":"404dfc5b_086a7b48","updated":"2026-02-12 13:30:14.000000000","message":"its looks better over all but we need to fix the assumation that all extra specs will existi on the pool","commit_id":"55f193192e458c75d0255526b80a3ca4487bd455"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"4bdbe3c4df2adc26d915e486893eda26a385329a","unresolved":true,"context_lines":[{"line_number":131,"context_line":"        :returns: True if pool matches all volume type requirements,"},{"line_number":132,"context_line":"                  False otherwise"},{"line_number":133,"context_line":"        \"\"\""},{"line_number":134,"context_line":"        for field_name, field_value in volume_type.extra_specs.items():"},{"line_number":135,"context_line":"            pool_value \u003d pool[\"capabilities\"].get(field_name)"},{"line_number":136,"context_line":"            if field_value !\u003d pool_value:"},{"line_number":137,"context_line":"                # the property associated with the volume type is"},{"line_number":138,"context_line":"                # not defined in the pool, so the type can\u0027t be used in the"},{"line_number":139,"context_line":"                # pool"},{"line_number":140,"context_line":"                LOG.debug("},{"line_number":141,"context_line":"                    f\"property {field_name} with value {field_value} \""},{"line_number":142,"context_line":"                    f\"does not match value {pool_value} from pool \""},{"line_number":143,"context_line":"                    f\"{pool[\u0027name\u0027]}\""},{"line_number":144,"context_line":"                )"},{"line_number":145,"context_line":"                return False"},{"line_number":146,"context_line":"        return True"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    def get_volume_types_for_pool(self, pool):"},{"line_number":149,"context_line":"        \"\"\"Return a list of volume types that can be associated with a pool."}],"source_content_type":"text/x-python","patch_set":4,"id":"404dfc5b_086a7b48","line":146,"range":{"start_line":134,"start_character":0,"end_line":146,"end_character":19},"in_reply_to":"de90dc9a_b18f11e2","updated":"2025-11-19 08:47:00.000000000","message":"Said that, I think the implementation in this patch is better and it\u0027s a valid fix for the existing behavior.","commit_id":"55f193192e458c75d0255526b80a3ca4487bd455"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"4986285aabdc3a77561acdfb302eae8a4f3a3e59","unresolved":true,"context_lines":[{"line_number":131,"context_line":"        :returns: True if pool matches all volume type requirements,"},{"line_number":132,"context_line":"                  False otherwise"},{"line_number":133,"context_line":"        \"\"\""},{"line_number":134,"context_line":"        for field_name, field_value in volume_type.extra_specs.items():"},{"line_number":135,"context_line":"            pool_value \u003d pool[\"capabilities\"].get(field_name)"},{"line_number":136,"context_line":"            if field_value !\u003d pool_value:"},{"line_number":137,"context_line":"                # the property associated with the volume type is"},{"line_number":138,"context_line":"                # not defined in the pool, so the type can\u0027t be used in the"},{"line_number":139,"context_line":"                # pool"},{"line_number":140,"context_line":"                LOG.debug("},{"line_number":141,"context_line":"                    f\"property {field_name} with value {field_value} \""},{"line_number":142,"context_line":"                    f\"does not match value {pool_value} from pool \""},{"line_number":143,"context_line":"                    f\"{pool[\u0027name\u0027]}\""},{"line_number":144,"context_line":"                )"},{"line_number":145,"context_line":"                return False"},{"line_number":146,"context_line":"        return True"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    def get_volume_types_for_pool(self, pool):"},{"line_number":149,"context_line":"        \"\"\"Return a list of volume types that can be associated with a pool."}],"source_content_type":"text/x-python","patch_set":4,"id":"01cc391c_4acb244c","line":146,"range":{"start_line":134,"start_character":0,"end_line":146,"end_character":19},"in_reply_to":"f0561da5_2822a10a","updated":"2025-11-13 15:32:18.000000000","message":"yes, that is quite more complex. TBH I don\u0027t think it would be a good idea to try to have a similarly complex check in watcher, it would introduce an excessive maintenance burden for watcher imo. The current version (patchset 4) has a \"best effort\" check that tries to detect if the destination host is valid. We know that this approach should work for `volume_backend_name` but we don\u0027t coverage that I know of for other capabilities. I think it\u0027s reasonable to keep a simple check here, from the user point of view, the consequence of catching a wrong destination host here of after the cinder call will just be a few seconds, since both scenarios will end up in a failed action","commit_id":"55f193192e458c75d0255526b80a3ca4487bd455"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c82fb6ad90cca803b201de585ad9565980dde245","unresolved":false,"context_lines":[{"line_number":131,"context_line":"        :returns: True if pool matches all volume type requirements,"},{"line_number":132,"context_line":"                  False otherwise"},{"line_number":133,"context_line":"        \"\"\""},{"line_number":134,"context_line":"        for field_name, field_value in volume_type.extra_specs.items():"},{"line_number":135,"context_line":"            pool_value \u003d pool[\"capabilities\"].get(field_name)"},{"line_number":136,"context_line":"            if field_value !\u003d pool_value:"},{"line_number":137,"context_line":"                # the property associated with the volume type is"},{"line_number":138,"context_line":"                # not defined in the pool, so the type can\u0027t be used in the"},{"line_number":139,"context_line":"                # pool"},{"line_number":140,"context_line":"                LOG.debug("},{"line_number":141,"context_line":"                    f\"property {field_name} with value {field_value} \""},{"line_number":142,"context_line":"                    f\"does not match value {pool_value} from pool \""},{"line_number":143,"context_line":"                    f\"{pool[\u0027name\u0027]}\""},{"line_number":144,"context_line":"                )"},{"line_number":145,"context_line":"                return False"},{"line_number":146,"context_line":"        return True"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    def get_volume_types_for_pool(self, pool):"},{"line_number":149,"context_line":"        \"\"\"Return a list of volume types that can be associated with a pool."}],"source_content_type":"text/x-python","patch_set":4,"id":"624a99d2_f994a77c","line":146,"range":{"start_line":134,"start_character":0,"end_line":146,"end_character":19},"in_reply_to":"f2876a0b_b0fd3c5d","updated":"2026-03-05 15:34:33.000000000","message":"Done","commit_id":"55f193192e458c75d0255526b80a3ca4487bd455"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"54525cf2d0e5aa0973ea3fe76c5dfe6a2d75a771","unresolved":true,"context_lines":[{"line_number":151,"context_line":"        :param pool: Storage pool dictionary with capabilities"},{"line_number":152,"context_line":"        :returns: List of volume types than can be scheduled in the input pool"},{"line_number":153,"context_line":"        \"\"\""},{"line_number":154,"context_line":"        volume_type_list \u003d self.get_volume_type_list()"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        pool_volume_types \u003d []"},{"line_number":157,"context_line":"        for volume_type in volume_type_list:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3d6c596f_ee4cf48e","line":154,"range":{"start_line":154,"start_character":32,"end_line":154,"end_character":52},"updated":"2025-11-12 17:35:03.000000000","message":"I was thinking here, that we may also have issues with private volume types. Not all volume types are public, so I was wondering if this list will also include the volume types that are private to specific projects. I think that we need to handle this scenario too.","commit_id":"55f193192e458c75d0255526b80a3ca4487bd455"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"4986285aabdc3a77561acdfb302eae8a4f3a3e59","unresolved":true,"context_lines":[{"line_number":151,"context_line":"        :param pool: Storage pool dictionary with capabilities"},{"line_number":152,"context_line":"        :returns: List of volume types than can be scheduled in the input pool"},{"line_number":153,"context_line":"        \"\"\""},{"line_number":154,"context_line":"        volume_type_list \u003d self.get_volume_type_list()"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        pool_volume_types \u003d []"},{"line_number":157,"context_line":"        for volume_type in volume_type_list:"}],"source_content_type":"text/x-python","patch_set":4,"id":"71ce05f3_d2e2044c","line":154,"range":{"start_line":154,"start_character":32,"end_line":154,"end_character":52},"in_reply_to":"3d6c596f_ee4cf48e","updated":"2025-11-13 15:32:18.000000000","message":"that\u0027s a good point, I had to check it. But this seems to work fine. The `get_volume_type_list` method https://github.com/openstack/watcher/blob/f25853733515f1e3e56407263c1da2e0c4a356cb/watcher/common/cinder_helper.py#L71 makes the following call to cinder:\n`GET call to volumev3 for http://192.168.70.37/volume/v3/types?is_public\u003dNone used request id req-96c775cf-af2b-4068-b1c9-a1d13cf1f653 {{(pid\u003d627002) request /opt/stack/data/venv/lib/python3.12/site-packages/keystoneauth1/session.py:1073}}`\n\nI created a private volume type in a devstack environment and the call above returned:\n`[\u003cVolumeType: test_private\u003e, \u003cVolumeType: lvmdriver-1\u003e, \u003cVolumeType: __DEFAULT__\u003e]`\n\nwhile `openstack volume type list` returns:\n\n```\n(venv) ubuntu@jgilaber-watcher-1:~$ openstack volume type list\n+--------------------------------------+--------------+-----------+\n| ID                                   | Name         | Is Public |\n+--------------------------------------+--------------+-----------+\n| 60c25437-fd72-44a1-960b-938384c1001e | test_private | False     |\n| c81d2053-9c7a-4626-bcb3-01dc1a73b3e3 | lvmdriver-1  | True      |\n| 4d6ef0b4-6dc3-4484-a1df-1c818925c92d | __DEFAULT__  | True      |\n+--------------------------------------+--------------+-----------+\n```\n\nthat api call has an `is_public` parameter to filter public or private types, which by default is set to `None` so it returns all of them. We could change `get_volume_type_list` to set the parameter explicitely in case the default ever changes, but other than that the current call seems to work well","commit_id":"55f193192e458c75d0255526b80a3ca4487bd455"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6f44ab7d8353bf9928e07217ca12a700c724af35","unresolved":true,"context_lines":[{"line_number":151,"context_line":"        :param pool: Storage pool dictionary with capabilities"},{"line_number":152,"context_line":"        :returns: List of volume types than can be scheduled in the input pool"},{"line_number":153,"context_line":"        \"\"\""},{"line_number":154,"context_line":"        volume_type_list \u003d self.get_volume_type_list()"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        pool_volume_types \u003d []"},{"line_number":157,"context_line":"        for volume_type in volume_type_list:"}],"source_content_type":"text/x-python","patch_set":4,"id":"a3ce1371_405f4b38","line":154,"range":{"start_line":154,"start_character":32,"end_line":154,"end_character":52},"in_reply_to":"71ce05f3_d2e2044c","updated":"2026-02-12 13:30:14.000000000","message":"well we(watcher) are admins so we should be able to see all voluem types as a result\n\nbut if we were usign a non admin/service token that could indeed fail.","commit_id":"55f193192e458c75d0255526b80a3ca4487bd455"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"5c3119da2eef806e9959311f838d0bff12235bb9","unresolved":true,"context_lines":[{"line_number":151,"context_line":"        :param pool: Storage pool dictionary with capabilities"},{"line_number":152,"context_line":"        :returns: List of volume types than can be scheduled in the input pool"},{"line_number":153,"context_line":"        \"\"\""},{"line_number":154,"context_line":"        volume_type_list \u003d self.get_volume_type_list()"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        pool_volume_types \u003d []"},{"line_number":157,"context_line":"        for volume_type in volume_type_list:"}],"source_content_type":"text/x-python","patch_set":4,"id":"569f3242_cc4db55b","line":154,"range":{"start_line":154,"start_character":32,"end_line":154,"end_character":52},"in_reply_to":"95f89592_d43c65f5","updated":"2026-03-05 17:52:37.000000000","message":"Just a feedback on this. My doubt was different, it was about retyping a volume and choose a private volume type that is not visible in current project\u0027s volume. So my understanding was that you would need to pick a Public or Private types within the same project-id.\nSo I did a quick test in my env, and it doesn\u0027t fail. I could retype to a private volume type that is not visible to the current volume\u0027s project. It sucessfully retypes (it was to the same host/backend name). The project now can see its volume in a different type now, but can\u0027t get details about the volume type.\nSo in my simple test, it turns to be valid get any other private volume type.","commit_id":"55f193192e458c75d0255526b80a3ca4487bd455"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c82fb6ad90cca803b201de585ad9565980dde245","unresolved":false,"context_lines":[{"line_number":151,"context_line":"        :param pool: Storage pool dictionary with capabilities"},{"line_number":152,"context_line":"        :returns: List of volume types than can be scheduled in the input pool"},{"line_number":153,"context_line":"        \"\"\""},{"line_number":154,"context_line":"        volume_type_list \u003d self.get_volume_type_list()"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        pool_volume_types \u003d []"},{"line_number":157,"context_line":"        for volume_type in volume_type_list:"}],"source_content_type":"text/x-python","patch_set":4,"id":"95f89592_d43c65f5","line":154,"range":{"start_line":154,"start_character":32,"end_line":154,"end_character":52},"in_reply_to":"a3ce1371_405f4b38","updated":"2026-03-05 15:34:33.000000000","message":"Acknowledged","commit_id":"55f193192e458c75d0255526b80a3ca4487bd455"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c82fb6ad90cca803b201de585ad9565980dde245","unresolved":false,"context_lines":[{"line_number":131,"context_line":"        :returns: True if pool matches all volume type requirements,"},{"line_number":132,"context_line":"                  False otherwise"},{"line_number":133,"context_line":"        \"\"\""},{"line_number":134,"context_line":"        for field_name, field_value in volume_type.extra_specs.items():"},{"line_number":135,"context_line":"            pool_value \u003d pool[\"capabilities\"].get(field_name)"},{"line_number":136,"context_line":"            if field_value !\u003d pool_value:"},{"line_number":137,"context_line":"                # the property associated with the volume type is"}],"source_content_type":"text/x-python","patch_set":5,"id":"0dda0a4c_ca943654","line":134,"in_reply_to":"1b793e53_dbbacd92","updated":"2026-03-05 15:34:33.000000000","message":"Done","commit_id":"f9de5807968bfea40a22868545e18a309b4fa8ca"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"acb4566760a6b4fa5c2df076b616081ed563d348","unresolved":true,"context_lines":[{"line_number":131,"context_line":"        :returns: True if pool matches all volume type requirements,"},{"line_number":132,"context_line":"                  False otherwise"},{"line_number":133,"context_line":"        \"\"\""},{"line_number":134,"context_line":"        for field_name, field_value in volume_type.extra_specs.items():"},{"line_number":135,"context_line":"            pool_value \u003d pool[\"capabilities\"].get(field_name)"},{"line_number":136,"context_line":"            if field_value !\u003d pool_value:"},{"line_number":137,"context_line":"                # the property associated with the volume type is"}],"source_content_type":"text/x-python","patch_set":5,"id":"1b793e53_dbbacd92","line":134,"in_reply_to":"7f440dd6_f100a8fb","updated":"2026-02-12 14:31:07.000000000","message":"that\u0027s true, I assumed that a pool would always have a capabilities defined, but if that might not always be true then we should add the get. I\u0027ve done so, and considered that pools that don\u0027t have capabilities should be accepted, so that we can try the migration and let cinder do a more accurate check","commit_id":"f9de5807968bfea40a22868545e18a309b4fa8ca"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6f44ab7d8353bf9928e07217ca12a700c724af35","unresolved":true,"context_lines":[{"line_number":131,"context_line":"        :returns: True if pool matches all volume type requirements,"},{"line_number":132,"context_line":"                  False otherwise"},{"line_number":133,"context_line":"        \"\"\""},{"line_number":134,"context_line":"        for field_name, field_value in volume_type.extra_specs.items():"},{"line_number":135,"context_line":"            pool_value \u003d pool[\"capabilities\"].get(field_name)"},{"line_number":136,"context_line":"            if field_value !\u003d pool_value:"},{"line_number":137,"context_line":"                # the property associated with the volume type is"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f440dd6_f100a8fb","line":134,"in_reply_to":"82ab080f_fb151491","updated":"2026-02-12 13:30:14.000000000","message":"the better fix is \n\u003e for field_name, field_value in volume_type.extra_specs.items():\n\u003e     pool_value \u003d pool.get(\"capabilities\",{}).get(field_name)\n\nof course you the need to skip cases weher pool_value is None below\n\ncurrenly we can get a key errror when lookign at the pool capablieis fi the extra sepc is not refelcted there so there is a bug in the current code.","commit_id":"f9de5807968bfea40a22868545e18a309b4fa8ca"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6f44ab7d8353bf9928e07217ca12a700c724af35","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                # the property associated with the volume type is"},{"line_number":138,"context_line":"                # not defined in the pool, so the type can\u0027t be used in the"},{"line_number":139,"context_line":"                # pool"},{"line_number":140,"context_line":"                LOG.debug("},{"line_number":141,"context_line":"                    f\"property {field_name} with value {field_value} \""},{"line_number":142,"context_line":"                    f\"does not match value {pool_value} from pool \""},{"line_number":143,"context_line":"                    f\"{pool[\u0027name\u0027]}\""}],"source_content_type":"text/x-python","patch_set":5,"id":"1282d079_a060afdc","line":140,"in_reply_to":"87be87a3_3398a537","updated":"2026-02-12 13:30:14.000000000","message":"so if this was an excption method that woudl be correct but we do not translate logs anymore delayed formatign was purly for the usecase","commit_id":"f9de5807968bfea40a22868545e18a309b4fa8ca"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6f44ab7d8353bf9928e07217ca12a700c724af35","unresolved":false,"context_lines":[{"line_number":190,"context_line":"            time.sleep(retry_interval)"},{"line_number":191,"context_line":"            if getattr(volume, \u0027migration_status\u0027) \u003d\u003d \u0027error\u0027:"},{"line_number":192,"context_line":"                host_name \u003d getattr(volume, \u0027os-vol-host-attr:host\u0027)"},{"line_number":193,"context_line":"                error_msg \u003d (\"Volume migration error : \""},{"line_number":194,"context_line":"                             f\"volume {volume.id} is now on host \""},{"line_number":195,"context_line":"                             f\"\u0027{host_name}\u0027.\")"},{"line_number":196,"context_line":"                LOG.error(error_msg)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ae1ba2a0_b237fa17","line":193,"in_reply_to":"dcfc5cf9_4cd7c809","updated":"2026-02-12 13:30:14.000000000","message":"again not relevent i will try an fix that in the next week or so","commit_id":"f9de5807968bfea40a22868545e18a309b4fa8ca"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6f44ab7d8353bf9928e07217ca12a700c724af35","unresolved":true,"context_lines":[{"line_number":259,"context_line":"        pool_dict \u003d self.get_storage_pool_by_name(dest_node).to_dict()"},{"line_number":260,"context_line":"        dest_types \u003d self.get_volume_types_for_pool(pool_dict)"},{"line_number":261,"context_line":"        if volume.volume_type not in dest_types:"},{"line_number":262,"context_line":"            raise exception.Invalid("},{"line_number":263,"context_line":"                message\u003d("},{"line_number":264,"context_line":"                    _("},{"line_number":265,"context_line":"                        \"Volume type of the volume being migrated must \""}],"source_content_type":"text/x-python","patch_set":5,"id":"9a254f2e_135f924a","line":262,"in_reply_to":"322ee918_5eb343a9","updated":"2026-02-12 13:30:14.000000000","message":"ya thats valid.","commit_id":"f9de5807968bfea40a22868545e18a309b4fa8ca"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"acb4566760a6b4fa5c2df076b616081ed563d348","unresolved":false,"context_lines":[{"line_number":259,"context_line":"        pool_dict \u003d self.get_storage_pool_by_name(dest_node).to_dict()"},{"line_number":260,"context_line":"        dest_types \u003d self.get_volume_types_for_pool(pool_dict)"},{"line_number":261,"context_line":"        if volume.volume_type not in dest_types:"},{"line_number":262,"context_line":"            raise exception.Invalid("},{"line_number":263,"context_line":"                message\u003d("},{"line_number":264,"context_line":"                    _("},{"line_number":265,"context_line":"                        \"Volume type of the volume being migrated must \""}],"source_content_type":"text/x-python","patch_set":5,"id":"7d7c757c_7da5ee3c","line":262,"in_reply_to":"9a254f2e_135f924a","updated":"2026-02-12 14:31:07.000000000","message":"that\u0027s a good suggestion, done","commit_id":"f9de5807968bfea40a22868545e18a309b4fa8ca"}],"watcher/tests/unit/common/test_cinder_helper.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6f44ab7d8353bf9928e07217ca12a700c724af35","unresolved":true,"context_lines":[{"line_number":160,"context_line":"        setattr("},{"line_number":161,"context_line":"            volume, \u0027os-vol-host-attr:host\u0027,"},{"line_number":162,"context_line":"            kwargs.get(\u0027os_vol_host_attr_host\u0027)"},{"line_number":163,"context_line":"        )"},{"line_number":164,"context_line":"        return volume"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    @mock.patch.object(time, \u0027sleep\u0027, mock.Mock())"}],"source_content_type":"text/x-python","patch_set":5,"id":"5b0e5465_7cdb685f","line":163,"updated":"2026-02-12 13:30:14.000000000","message":"you shoudl not need to update this altohg i gues syou rodign that becasue tis not actuly called that?\n\ni would prefer to avoid the setattr here but i guess the real fix is to use the sdk and have dataclasess instead.\n\nso we could live with this for now","commit_id":"f9de5807968bfea40a22868545e18a309b4fa8ca"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"acb4566760a6b4fa5c2df076b616081ed563d348","unresolved":true,"context_lines":[{"line_number":160,"context_line":"        setattr("},{"line_number":161,"context_line":"            volume, \u0027os-vol-host-attr:host\u0027,"},{"line_number":162,"context_line":"            kwargs.get(\u0027os_vol_host_attr_host\u0027)"},{"line_number":163,"context_line":"        )"},{"line_number":164,"context_line":"        return volume"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    @mock.patch.object(time, \u0027sleep\u0027, mock.Mock())"}],"source_content_type":"text/x-python","patch_set":5,"id":"8bdbf6dc_85adeac8","line":163,"in_reply_to":"5b0e5465_7cdb685f","updated":"2026-02-12 14:31:07.000000000","message":"you\u0027re right, this is not strictly necessary for the fix, I put it here to avoid having similar `setattr` calls like we have in line 182 of the base version. I thought this was slightly cleaner and will make it a bit easier in the future to replace this function with another that creates a real Volume class like we\u0027ve done with the nova related tests. I can undo this change if preferred, it should not affect the fix","commit_id":"f9de5807968bfea40a22868545e18a309b4fa8ca"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c82fb6ad90cca803b201de585ad9565980dde245","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        setattr("},{"line_number":161,"context_line":"            volume, \u0027os-vol-host-attr:host\u0027,"},{"line_number":162,"context_line":"            kwargs.get(\u0027os_vol_host_attr_host\u0027)"},{"line_number":163,"context_line":"        )"},{"line_number":164,"context_line":"        return volume"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    @mock.patch.object(time, \u0027sleep\u0027, mock.Mock())"}],"source_content_type":"text/x-python","patch_set":5,"id":"26cdcb6f_e9e6bd45","line":163,"in_reply_to":"8bdbf6dc_85adeac8","updated":"2026-03-05 15:34:33.000000000","message":"Acknowledged","commit_id":"f9de5807968bfea40a22868545e18a309b4fa8ca"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c82fb6ad90cca803b201de585ad9565980dde245","unresolved":true,"context_lines":[{"line_number":23,"context_line":"from cinderclient import exceptions as cinder_exception"},{"line_number":24,"context_line":"from cinderclient.v3.pools import Pool as CinderPool"},{"line_number":25,"context_line":"from cinderclient.v3.volume_types import VolumeType as CinderVolType"},{"line_number":26,"context_line":"from cinderclient.v3.volumes import Volume as CinderVolume"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"from watcher.common import cinder_helper"},{"line_number":29,"context_line":"from watcher.common import clients"}],"source_content_type":"text/x-python","patch_set":6,"id":"b6542008_ecedecb4","line":26,"updated":"2026-03-05 15:34:33.000000000","message":"this is not idal as we should really be minimising oru depency on things like the cinder client in our unit tests","commit_id":"22fd676d793b7dba260459f84dc5c48bbbe9ecf4"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"8a3b89f59b879f2b813284e3fff0954e0c4d132c","unresolved":true,"context_lines":[{"line_number":23,"context_line":"from cinderclient import exceptions as cinder_exception"},{"line_number":24,"context_line":"from cinderclient.v3.pools import Pool as CinderPool"},{"line_number":25,"context_line":"from cinderclient.v3.volume_types import VolumeType as CinderVolType"},{"line_number":26,"context_line":"from cinderclient.v3.volumes import Volume as CinderVolume"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"from watcher.common import cinder_helper"},{"line_number":29,"context_line":"from watcher.common import clients"}],"source_content_type":"text/x-python","patch_set":6,"id":"bb79ddbb_4a9111e3","line":26,"in_reply_to":"b6542008_ecedecb4","updated":"2026-03-05 16:16:40.000000000","message":"yes, in the next release I plan to do the same changes I\u0027ve done in nova_helper for the rest of the helper. Then this won\u0027t be needed","commit_id":"22fd676d793b7dba260459f84dc5c48bbbe9ecf4"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"8a3b89f59b879f2b813284e3fff0954e0c4d132c","unresolved":false,"context_lines":[{"line_number":644,"context_line":"            {\u0027volume\u0027: volume.id})"},{"line_number":645,"context_line":"        self.assertFalse(result)"},{"line_number":646,"context_line":""},{"line_number":647,"context_line":"    def test_get_volume_types_for_pool(self, mock_cinder):"},{"line_number":648,"context_line":"        \"\"\"Test the get_volume_types_for_pool method"},{"line_number":649,"context_line":""},{"line_number":650,"context_line":"        The test should verify the following scenarios:"}],"source_content_type":"text/x-python","patch_set":6,"id":"c59fd4e1_11c4eb4c","line":647,"in_reply_to":"0fc03d35_aecf94e7","updated":"2026-03-05 16:16:40.000000000","message":"fixed in a follow-up patch https://review.opendev.org/c/openstack/watcher/+/979029","commit_id":"22fd676d793b7dba260459f84dc5c48bbbe9ecf4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c82fb6ad90cca803b201de585ad9565980dde245","unresolved":true,"context_lines":[{"line_number":644,"context_line":"            {\u0027volume\u0027: volume.id})"},{"line_number":645,"context_line":"        self.assertFalse(result)"},{"line_number":646,"context_line":""},{"line_number":647,"context_line":"    def test_get_volume_types_for_pool(self, mock_cinder):"},{"line_number":648,"context_line":"        \"\"\"Test the get_volume_types_for_pool method"},{"line_number":649,"context_line":""},{"line_number":650,"context_line":"        The test should verify the following scenarios:"}],"source_content_type":"text/x-python","patch_set":6,"id":"84440cf6_f137b95b","line":647,"in_reply_to":"73e4bd61_24ebda05","updated":"2026-03-05 15:34:33.000000000","message":"this is sort of a duplicate of the above we could have better test coverage of the edge cases but i think we are mostly ok","commit_id":"22fd676d793b7dba260459f84dc5c48bbbe9ecf4"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"8a3b89f59b879f2b813284e3fff0954e0c4d132c","unresolved":false,"context_lines":[{"line_number":644,"context_line":"            {\u0027volume\u0027: volume.id})"},{"line_number":645,"context_line":"        self.assertFalse(result)"},{"line_number":646,"context_line":""},{"line_number":647,"context_line":"    def test_get_volume_types_for_pool(self, mock_cinder):"},{"line_number":648,"context_line":"        \"\"\"Test the get_volume_types_for_pool method"},{"line_number":649,"context_line":""},{"line_number":650,"context_line":"        The test should verify the following scenarios:"}],"source_content_type":"text/x-python","patch_set":6,"id":"56270889_d6902e7b","line":647,"in_reply_to":"84440cf6_f137b95b","updated":"2026-03-05 16:16:40.000000000","message":"fixed in a follow-up patch https://review.opendev.org/c/openstack/watcher/+/979029","commit_id":"22fd676d793b7dba260459f84dc5c48bbbe9ecf4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c82fb6ad90cca803b201de585ad9565980dde245","unresolved":true,"context_lines":[{"line_number":644,"context_line":"            {\u0027volume\u0027: volume.id})"},{"line_number":645,"context_line":"        self.assertFalse(result)"},{"line_number":646,"context_line":""},{"line_number":647,"context_line":"    def test_get_volume_types_for_pool(self, mock_cinder):"},{"line_number":648,"context_line":"        \"\"\"Test the get_volume_types_for_pool method"},{"line_number":649,"context_line":""},{"line_number":650,"context_line":"        The test should verify the following scenarios:"}],"source_content_type":"text/x-python","patch_set":6,"id":"0fc03d35_aecf94e7","line":647,"in_reply_to":"a3fdbb4c_ba1792bb","updated":"2026-03-05 15:34:33.000000000","message":"this is valid but it shoudl be its own test.\nwe could add this later","commit_id":"22fd676d793b7dba260459f84dc5c48bbbe9ecf4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c82fb6ad90cca803b201de585ad9565980dde245","unresolved":true,"context_lines":[{"line_number":683,"context_line":"        result \u003d cinder_util.get_volume_types_for_pool(pool1.to_dict())"},{"line_number":684,"context_line":"        self.assertEqual(sorted(result), [\u0027type_matching\u0027, \u0027type_no_specs\u0027])"},{"line_number":685,"context_line":""},{"line_number":686,"context_line":"        # Scenario 2: Pool with volume_backend_name, two volume types"},{"line_number":687,"context_line":"        # (one without extra_specs, one with different backend),"},{"line_number":688,"context_line":"        # only type without extra_specs selected"},{"line_number":689,"context_line":"        pool2 \u003d self.fake_pool("},{"line_number":690,"context_line":"            name\u003d\u0027host@backend#pool\u0027,"},{"line_number":691,"context_line":"            capabilities\u003d{\u0027volume_backend_name\u0027: \u0027backend\u0027}"}],"source_content_type":"text/x-python","patch_set":6,"id":"4b3f1a6f_dca11820","line":688,"range":{"start_line":686,"start_character":7,"end_line":688,"end_character":48},"updated":"2026-03-05 15:34:33.000000000","message":"this is a code smell you should nto have multiple sensieas in one test\n\neach of these sections shoudl be there own test.\n\neach test shoudl test one thing","commit_id":"22fd676d793b7dba260459f84dc5c48bbbe9ecf4"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"8a3b89f59b879f2b813284e3fff0954e0c4d132c","unresolved":false,"context_lines":[{"line_number":683,"context_line":"        result \u003d cinder_util.get_volume_types_for_pool(pool1.to_dict())"},{"line_number":684,"context_line":"        self.assertEqual(sorted(result), [\u0027type_matching\u0027, \u0027type_no_specs\u0027])"},{"line_number":685,"context_line":""},{"line_number":686,"context_line":"        # Scenario 2: Pool with volume_backend_name, two volume types"},{"line_number":687,"context_line":"        # (one without extra_specs, one with different backend),"},{"line_number":688,"context_line":"        # only type without extra_specs selected"},{"line_number":689,"context_line":"        pool2 \u003d self.fake_pool("},{"line_number":690,"context_line":"            name\u003d\u0027host@backend#pool\u0027,"},{"line_number":691,"context_line":"            capabilities\u003d{\u0027volume_backend_name\u0027: \u0027backend\u0027}"}],"source_content_type":"text/x-python","patch_set":6,"id":"7596762a_9e64c121","line":688,"range":{"start_line":686,"start_character":7,"end_line":688,"end_character":48},"in_reply_to":"4b3f1a6f_dca11820","updated":"2026-03-05 16:16:40.000000000","message":"fixed in a follow-up patch https://review.opendev.org/c/openstack/watcher/+/979029","commit_id":"22fd676d793b7dba260459f84dc5c48bbbe9ecf4"}]}