)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0059b8e27c7facebad05ff44cdea1b87841f1d4f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3d2e2fe6_a57da996","updated":"2025-08-19 10:05:23.000000000","message":"This is a great start. Sorry for not reviewing it sooner. I have left a lot of comments. Please let me know if anything is unclear! 🙏","commit_id":"b038fb7d6b0660c65242e41e0c2e6642a9a5053c"},{"author":{"_account_id":37954,"name":"Lee Moon Young","display_name":"MoonYoung","email":"happy518596@gmail.com","username":"happy7656"},"change_message_id":"6615ce7fcb79889017211311a48b04abbd1def36","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7f96bcd1_8676a92c","in_reply_to":"3d2e2fe6_a57da996","updated":"2025-08-22 12:21:09.000000000","message":"Thank you for the thorough review and guidance, and apologies for the delayed reply. To address your comments properly, I took some time to deepen my understanding of a few Python patterns and the test fixtures in this codebase. My earlier patch lacked that context, which made this update take longer than expected.\n\nI updated all test classes to subclass volume_fakes.TestVolume, replaced the cinder-style fakes with openstack.test.fakes.generate_fake_resource, and switched all microversion checks to sdk_utils.supports_microversion.\n\nI’ve pushed a new patchset. I’m not fully certain I captured your intent everywhere, and there may still be gaps given my limited understanding of some OpenStack modules. Would you be willing to take another look? Thanks again for the detailed feedback.","commit_id":"b038fb7d6b0660c65242e41e0c2e6642a9a5053c"},{"author":{"_account_id":37954,"name":"Lee Moon Young","display_name":"MoonYoung","email":"happy518596@gmail.com","username":"happy7656"},"change_message_id":"6615ce7fcb79889017211311a48b04abbd1def36","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"95aba291_f7d36b08","updated":"2025-08-22 12:21:09.000000000","message":"Uploaded PS3. Changes:\n– tests now subclass volume_fakes.TestVolume\n– replaced cinder-style fakes with openstack.test.fakes.generate_fake_resource\n– unified microversion checks via sdk_utils.supports_microversion\n\nPlease re-review.","commit_id":"0c758917683baa5f5151a77155f5667e2df5aa45"}],"openstackclient/tests/unit/volume/v3/test_volume_group_type.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0059b8e27c7facebad05ff44cdea1b87841f1d4f","unresolved":true,"context_lines":[{"line_number":20,"context_line":"from openstackclient.volume.v3.volume_group_type import _format_group_type"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"class TestVolumeGroupType(volume_fakes.TestVolume):"},{"line_number":24,"context_line":"    def setUp(self):"},{"line_number":25,"context_line":"        super().setUp()"},{"line_number":26,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"862da6a4_cdb6d7a6","line":23,"updated":"2025-08-19 10:05:23.000000000","message":"You don\u0027t need this base class anymore since you\u0027re not using the mocks created in `setUp`. You can delete this and just subclass all tests from `volume_fakes.TestVolume` instead","commit_id":"b038fb7d6b0660c65242e41e0c2e6642a9a5053c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0059b8e27c7facebad05ff44cdea1b87841f1d4f","unresolved":true,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"class TestVolumeGroupTypeCreate(TestVolumeGroupType):"},{"line_number":32,"context_line":"    fake_volume_group_type \u003d volume_fakes.create_one_volume_group_type()"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    columns \u003d ("},{"line_number":35,"context_line":"        \u0027ID\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"1faa3f82_cd03ffa7","line":32,"updated":"2025-08-19 10:05:23.000000000","message":"This returns a cinderclient-like resource rather an SDK `VolumeType` instance. Instead of doing this, can you use the `generate_fake_resource` function from the `openstack.test.fakes` module? There will be lots of examples in the compute unit tests. You may be able to delete the `create_one_volume_group_type` helper too assuming there are no other users.\n\nSame comment for all other tests here","commit_id":"b038fb7d6b0660c65242e41e0c2e6642a9a5053c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0059b8e27c7facebad05ff44cdea1b87841f1d4f","unresolved":true,"context_lines":[{"line_number":49,"context_line":"    def setUp(self):"},{"line_number":50,"context_line":"        super().setUp()"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        self.sdk_client \u003d mock.Mock()"},{"line_number":53,"context_line":"        self.app.client_manager.sdk_connection \u003d self.sdk_client"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        self.sdk_client.block_storage.create_group_type.return_value \u003d ("}],"source_content_type":"text/x-python","patch_set":2,"id":"9a59020a_09382e62","line":52,"updated":"2025-08-19 10:05:23.000000000","message":"You don\u0027t need to do this. There\u0027s a `volume_sdk_client` attribute on the class that you can use instead.\n\nSame comment for all other tests here.","commit_id":"b038fb7d6b0660c65242e41e0c2e6642a9a5053c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0059b8e27c7facebad05ff44cdea1b87841f1d4f","unresolved":true,"context_lines":[{"line_number":125,"context_line":"    #     )"},{"line_number":126,"context_line":"    #     self.assertIn("},{"line_number":127,"context_line":"    #         \u0027--os-volume-api-version 3.11 or greater is required\u0027, str(exc)"},{"line_number":128,"context_line":"    #     )"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"class TestVolumeGroupTypeDelete(TestVolumeGroupType):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9309d7ac_79faf6aa","line":128,"updated":"2025-08-19 10:05:23.000000000","message":"We need to keep this test. I\u0027ve left comments in the next file for how to migrate it. Again, there will be lots of examples of this kind of test in the compute unit tests.","commit_id":"b038fb7d6b0660c65242e41e0c2e6642a9a5053c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0059b8e27c7facebad05ff44cdea1b87841f1d4f","unresolved":true,"context_lines":[{"line_number":523,"context_line":""},{"line_number":524,"context_line":""},{"line_number":525,"context_line":"class TestVolumeGroupTypeShow(TestVolumeGroupType):"},{"line_number":526,"context_line":"    columns \u003d ("},{"line_number":527,"context_line":"        \u0027ID\u0027,"},{"line_number":528,"context_line":"        \u0027Name\u0027,"},{"line_number":529,"context_line":"        \u0027Description\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"05277d81_769dbb0c","line":526,"updated":"2025-08-19 10:05:23.000000000","message":"This only has one caller. Can you move it into the test?","commit_id":"b038fb7d6b0660c65242e41e0c2e6642a9a5053c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0059b8e27c7facebad05ff44cdea1b87841f1d4f","unresolved":true,"context_lines":[{"line_number":560,"context_line":"            ignore_missing\u003dFalse,"},{"line_number":561,"context_line":"        )"},{"line_number":562,"context_line":""},{"line_number":563,"context_line":"        expected_columns, expected_data \u003d _format_group_type("},{"line_number":564,"context_line":"            self.fake_volume_group_type"},{"line_number":565,"context_line":"        )"},{"line_number":566,"context_line":"        self.assertEqual(expected_columns, columns)"},{"line_number":567,"context_line":"        self.assertEqual(expected_data, data)"},{"line_number":568,"context_line":""},{"line_number":569,"context_line":"    def test_volume_group_type_show_pre_v311(self):"},{"line_number":570,"context_line":"        self.set_volume_api_version(\u00273.10\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"f3b64363_4f3003fe","line":567,"range":{"start_line":563,"start_character":0,"end_line":567,"end_character":45},"updated":"2025-08-19 10:05:23.000000000","message":"It would be better to hardcode the list of expected columns and data, IMHO.","commit_id":"b038fb7d6b0660c65242e41e0c2e6642a9a5053c"}],"openstackclient/volume/v3/volume_group_type.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0059b8e27c7facebad05ff44cdea1b87841f1d4f","unresolved":true,"context_lines":[{"line_number":93,"context_line":"    def take_action(self, parsed_args):"},{"line_number":94,"context_line":"        volume_client \u003d self.app.client_manager.volume"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        if volume_client.api_version \u003c api_versions.APIVersion(\u00273.11\u0027):"},{"line_number":97,"context_line":"            msg \u003d _("},{"line_number":98,"context_line":"                \"--os-volume-api-version 3.11 or greater is required to \""},{"line_number":99,"context_line":"                \"support the \u0027volume group type create\u0027 command\""},{"line_number":100,"context_line":"            )"},{"line_number":101,"context_line":"            raise exceptions.CommandError(msg)"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"        group_type \u003d volume_client.group_types.create("},{"line_number":104,"context_line":"            parsed_args.name, parsed_args.description, parsed_args.is_public"}],"source_content_type":"text/x-python","patch_set":2,"id":"91e0d1b4_1b0fef16","side":"PARENT","line":101,"range":{"start_line":96,"start_character":0,"end_line":101,"end_character":46},"updated":"2025-08-19 10:05:23.000000000","message":"You need to keep this. You can use the `supports_microversion` helper. There are quite a few examples in e.g. the compute code","commit_id":"88b59d89754238d8d56609950ff5cfc92344cf4c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0059b8e27c7facebad05ff44cdea1b87841f1d4f","unresolved":true,"context_lines":[{"line_number":91,"context_line":"        return parser"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    def take_action(self, parsed_args):"},{"line_number":94,"context_line":"        sdk \u003d self.app.client_manager.sdk_connection"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        group_type \u003d sdk.block_storage.create_group_type("},{"line_number":97,"context_line":"            name\u003dparsed_args.name,"},{"line_number":98,"context_line":"            description\u003dparsed_args.description,"},{"line_number":99,"context_line":"            is_public\u003dparsed_args.is_public,"},{"line_number":100,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":2,"id":"75fcbde3_b71e078a","line":97,"range":{"start_line":94,"start_character":0,"end_line":97,"end_character":0},"updated":"2025-08-19 10:05:23.000000000","message":"```suggestion\n        volume_client \u003d self.app.client_manager.sdk_connection.block_storage\n\n        group_type \u003d volume_client.create_group_type(\n            name\u003dparsed_args.name,\n```\n\nditto for the other commands below","commit_id":"b038fb7d6b0660c65242e41e0c2e6642a9a5053c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0059b8e27c7facebad05ff44cdea1b87841f1d4f","unresolved":true,"context_lines":[{"line_number":186,"context_line":"        )"},{"line_number":187,"context_line":"        return parser"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"    def take_action(self, parsed_args):"},{"line_number":190,"context_line":"        volume_client \u003d self.app.client_manager.volume"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"        if volume_client.api_version \u003c api_versions.APIVersion(\u00273.11\u0027):"},{"line_number":193,"context_line":"            msg \u003d _("},{"line_number":194,"context_line":"                \"--os-volume-api-version 3.11 or greater is required to \""},{"line_number":195,"context_line":"                \"support the \u0027volume group type set\u0027 command\""}],"source_content_type":"text/x-python","patch_set":2,"id":"30cf3c5f_e2badcc9","line":192,"range":{"start_line":189,"start_character":39,"end_line":192,"end_character":71},"updated":"2025-08-19 10:05:23.000000000","message":"See my note above. You can replace this with the `supports_microversion` helper from `openstack.utils`. There are quite a few examples in e.g. the compute code","commit_id":"b038fb7d6b0660c65242e41e0c2e6642a9a5053c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0059b8e27c7facebad05ff44cdea1b87841f1d4f","unresolved":true,"context_lines":[{"line_number":332,"context_line":"        # )"},{"line_number":333,"context_line":"        return parser"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"    def take_action(self, parsed_args):"},{"line_number":336,"context_line":"        volume_client \u003d self.app.client_manager.volume"},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"        if volume_client.api_version \u003c api_versions.APIVersion(\u00273.11\u0027):"},{"line_number":339,"context_line":"            msg \u003d _("},{"line_number":340,"context_line":"                \"--os-volume-api-version 3.11 or greater is required to \""},{"line_number":341,"context_line":"                \"support the \u0027volume group type list\u0027 command\""}],"source_content_type":"text/x-python","patch_set":2,"id":"b8f34b56_f3e20d42","line":338,"range":{"start_line":335,"start_character":39,"end_line":338,"end_character":71},"updated":"2025-08-19 10:05:23.000000000","message":"As above","commit_id":"b038fb7d6b0660c65242e41e0c2e6642a9a5053c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0059b8e27c7facebad05ff44cdea1b87841f1d4f","unresolved":true,"context_lines":[{"line_number":383,"context_line":"        )"},{"line_number":384,"context_line":"        return parser"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"    def take_action(self, parsed_args):"},{"line_number":387,"context_line":"        volume_client \u003d self.app.client_manager.volume"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"        if volume_client.api_version \u003c api_versions.APIVersion(\u00273.11\u0027):"},{"line_number":390,"context_line":"            msg \u003d _("},{"line_number":391,"context_line":"                \"--os-volume-api-version 3.11 or greater is required to \""},{"line_number":392,"context_line":"                \"support the \u0027volume group type show\u0027 command\""}],"source_content_type":"text/x-python","patch_set":2,"id":"365a5f36_b10052da","line":389,"range":{"start_line":386,"start_character":39,"end_line":389,"end_character":71},"updated":"2025-08-19 10:05:23.000000000","message":"As above","commit_id":"b038fb7d6b0660c65242e41e0c2e6642a9a5053c"}]}