)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"1c8804963c5c41a573bbe242df890cd531276870","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"10301e1f_49180a38","updated":"2024-12-03 13:34:05.000000000","message":"Optional: update the api-ref (just the example, the response is defined to return an arbitrary \"properties\" object).\n\nDoes the policy for stores/detail need to be revised to allow the cinder service user to make the call?  (It\u0027s documented as admin-only.)","commit_id":"252d435cef1f71b823849c43ca4ff8eb2fa4b7ce"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"7488a382553d8b34c9d98b841c8e87a8d111f0bd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"0dbae8c6_a5e1b6dd","updated":"2024-12-03 13:38:13.000000000","message":"Thanks Brian!","commit_id":"252d435cef1f71b823849c43ca4ff8eb2fa4b7ce"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"3be40d51f686764b6500aa4440aae831228fbfdc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"4b7a4112_1a03436a","updated":"2024-12-03 12:10:48.000000000","message":"example response with this change\n\n$ glance stores-info --detail\n+----------+----------------------------------------------------------------------------------+\n| Property | Value                                                                            |\n+----------+----------------------------------------------------------------------------------+\n| stores   | [{\"id\": \"default_backend\", \"default\": \"true\", \"type\": \"rbd\", \"properties\":       |\n|          | {\"chunk_size\": 8388608, \"pool\": \"images\", \"thin_provisioning\": false, \"fsid\":    |\n|          | \"b3f1b7af-173c-4dcc-b1e4-541332ae55af\"}, \"weight\": 0}]                           |\n+----------+----------------------------------------------------------------------------------+","commit_id":"252d435cef1f71b823849c43ca4ff8eb2fa4b7ce"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"7488a382553d8b34c9d98b841c8e87a8d111f0bd","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"4f2401cb_07c1ee07","in_reply_to":"10301e1f_49180a38","updated":"2024-12-03 13:38:13.000000000","message":"Yes, that is also one of the work item I have. Currently I\u0027m trying to make everything work with admin and later will add support/testing with the service user.\nWill update the api-ref.","commit_id":"252d435cef1f71b823849c43ca4ff8eb2fa4b7ce"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b159f670ef42796ed827c9017f6ad3f28e823294","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2456b07d_0cae059c","in_reply_to":"4f2401cb_07c1ee07","updated":"2024-12-03 13:47:24.000000000","message":"Done","commit_id":"252d435cef1f71b823849c43ca4ff8eb2fa4b7ce"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"b50de34cec9b0ce8025ca9a366543ac576857786","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"fc1a457b_93f01481","updated":"2024-12-04 00:15:13.000000000","message":"I agree that it makes sense to do the policy changes as a followup. I left some comments inline that I think are more than just a nit, but see what you think.","commit_id":"e1f547c2cf11065a4afd05b8e4c97c8ee4fb1a19"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d72d5a8e3766defb9ec88fe2952045beef767e63","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"9d855f8f_4897cc83","updated":"2024-12-03 20:07:24.000000000","message":"Thanks Abhishek!","commit_id":"d975ebd21d1132db81a87392ac5dcbd4c740b658"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"4cfe407376e8225413edad3d4135ea0435a5bbe1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"9e6d7ccb_4b789517","updated":"2024-12-04 19:28:58.000000000","message":"Revisions LGTM.","commit_id":"ff313acaf28db195df8e8933e801a402837f4690"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"23a331ecade5de063bc60f7d054f24869950dbe2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"fc6778d5_ccf28052","updated":"2024-12-04 08:31:14.000000000","message":"Thanks Brian and Abhishek","commit_id":"ff313acaf28db195df8e8933e801a402837f4690"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"a9064bad4bb9ae8516da8a4fceee0b152a1e21d5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"f203b5c1_6495a42a","updated":"2024-12-04 15:53:30.000000000","message":"recheck openstack-tox-functional-py39 - unrelated failure, namely, glance.tests.functional.v2.test_images_import_locking.TestImageImportLocking.test_import_copy_bust_lock","commit_id":"ff313acaf28db195df8e8933e801a402837f4690"}],"glance/api/v2/discovery.py":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"5ba9ac6f16342a54f50360c8b63cd5aa917438b9","unresolved":true,"context_lines":[{"line_number":99,"context_line":"        url_prefix \u003d store_detail._url_prefix"},{"line_number":100,"context_line":"        # When fsid and pool info are not available,"},{"line_number":101,"context_line":"        # _url_prefix is same as prefix"},{"line_number":102,"context_line":"        if url_prefix and len(url_prefix) \u003e len(prefix):"},{"line_number":103,"context_line":"            # Remove last trailing forward slash"},{"line_number":104,"context_line":"            url_prefix \u003d ("},{"line_number":105,"context_line":"                url_prefix[:-1] if url_prefix.endswith(\u0027/\u0027) else url_prefix)"}],"source_content_type":"text/x-python","patch_set":2,"id":"32c59587_ed289207","line":102,"range":{"start_line":102,"start_character":11,"end_line":102,"end_character":21},"updated":"2024-12-03 15:18:55.000000000","message":"I think url_prefix will never be None, so no need to check it right?","commit_id":"e1f547c2cf11065a4afd05b8e4c97c8ee4fb1a19"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d72d5a8e3766defb9ec88fe2952045beef767e63","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        url_prefix \u003d store_detail._url_prefix"},{"line_number":100,"context_line":"        # When fsid and pool info are not available,"},{"line_number":101,"context_line":"        # _url_prefix is same as prefix"},{"line_number":102,"context_line":"        if url_prefix and len(url_prefix) \u003e len(prefix):"},{"line_number":103,"context_line":"            # Remove last trailing forward slash"},{"line_number":104,"context_line":"            url_prefix \u003d ("},{"line_number":105,"context_line":"                url_prefix[:-1] if url_prefix.endswith(\u0027/\u0027) else url_prefix)"}],"source_content_type":"text/x-python","patch_set":2,"id":"b32ab92e_1b85f00d","line":102,"range":{"start_line":102,"start_character":11,"end_line":102,"end_character":21},"in_reply_to":"32c59587_ed289207","updated":"2024-12-03 20:07:24.000000000","message":"I was trying to avoid the failure in case of single store[1] but as I can see we exit early if multi store is not configured.\n\n$ glance stores-info\nMulti Backend support is not enabled\n$ glance stores-info --detail\nMulti Backend support is not enabled\n\nAlso there is a bigger problem in case of single store since self._url_prefix is not even initialized.\nBut anyways, those are not the concerns here, will remove this.\n\n[1] https://github.com/openstack/glance_store/blob/master/glance_store/_drivers/rbd.py#L375-L376","commit_id":"e1f547c2cf11065a4afd05b8e4c97c8ee4fb1a19"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"5ba9ac6f16342a54f50360c8b63cd5aa917438b9","unresolved":true,"context_lines":[{"line_number":105,"context_line":"                url_prefix[:-1] if url_prefix.endswith(\u0027/\u0027) else url_prefix)"},{"line_number":106,"context_line":"            pieces \u003d url_prefix[len(prefix):].split(\u0027/\u0027)"},{"line_number":107,"context_line":"            # This check ensures that we have fsid and pool info in url"},{"line_number":108,"context_line":"            if len(pieces) \u003d\u003d 2:"},{"line_number":109,"context_line":"                fsid \u003d pieces[0]"},{"line_number":110,"context_line":"        return {"},{"line_number":111,"context_line":"            \u0027chunk_size\u0027: store_detail.chunk_size,"}],"source_content_type":"text/x-python","patch_set":2,"id":"2d58a021_f4652b15","line":108,"range":{"start_line":108,"start_character":12,"end_line":108,"end_character":32},"updated":"2024-12-03 15:18:55.000000000","message":"Again at line you are checking length so this will always be true.","commit_id":"e1f547c2cf11065a4afd05b8e4c97c8ee4fb1a19"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"e8e2e1c63f4480763a6b441fe88244e140445087","unresolved":true,"context_lines":[{"line_number":105,"context_line":"                url_prefix[:-1] if url_prefix.endswith(\u0027/\u0027) else url_prefix)"},{"line_number":106,"context_line":"            pieces \u003d url_prefix[len(prefix):].split(\u0027/\u0027)"},{"line_number":107,"context_line":"            # This check ensures that we have fsid and pool info in url"},{"line_number":108,"context_line":"            if len(pieces) \u003d\u003d 2:"},{"line_number":109,"context_line":"                fsid \u003d pieces[0]"},{"line_number":110,"context_line":"        return {"},{"line_number":111,"context_line":"            \u0027chunk_size\u0027: store_detail.chunk_size,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5492c5ef_8a9c57e9","line":108,"range":{"start_line":108,"start_character":12,"end_line":108,"end_character":32},"in_reply_to":"288c1e79_4aa61623","updated":"2024-12-04 05:09:00.000000000","message":"Since fsid is unique per backend there is no need to add store-id to the url in future as well.","commit_id":"e1f547c2cf11065a4afd05b8e4c97c8ee4fb1a19"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d72d5a8e3766defb9ec88fe2952045beef767e63","unresolved":true,"context_lines":[{"line_number":105,"context_line":"                url_prefix[:-1] if url_prefix.endswith(\u0027/\u0027) else url_prefix)"},{"line_number":106,"context_line":"            pieces \u003d url_prefix[len(prefix):].split(\u0027/\u0027)"},{"line_number":107,"context_line":"            # This check ensures that we have fsid and pool info in url"},{"line_number":108,"context_line":"            if len(pieces) \u003d\u003d 2:"},{"line_number":109,"context_line":"                fsid \u003d pieces[0]"},{"line_number":110,"context_line":"        return {"},{"line_number":111,"context_line":"            \u0027chunk_size\u0027: store_detail.chunk_size,"}],"source_content_type":"text/x-python","patch_set":2,"id":"533c68f1_10872459","line":108,"range":{"start_line":108,"start_character":12,"end_line":108,"end_character":32},"in_reply_to":"2d58a021_f4652b15","updated":"2024-12-03 20:07:24.000000000","message":"L#102 offers a weak check whereas L#108 is a stricter check to ensure that we have correct format of the url.\nIf we move code around (for example, change the url_prefix in the glance RBD store and add another value like rbd://\u003cfsid\u003e/\u003cpool\u003e/\u003cstore-id\u003e etc.) this check will fail ensuring that no regression is introduced whereas L#102 will pass with the above format change.\nPersonally I would like to keep this check since I\u0027ve seen code changes introducing regressions that go unnoticed because of improper checks.","commit_id":"e1f547c2cf11065a4afd05b8e4c97c8ee4fb1a19"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"b50de34cec9b0ce8025ca9a366543ac576857786","unresolved":true,"context_lines":[{"line_number":105,"context_line":"                url_prefix[:-1] if url_prefix.endswith(\u0027/\u0027) else url_prefix)"},{"line_number":106,"context_line":"            pieces \u003d url_prefix[len(prefix):].split(\u0027/\u0027)"},{"line_number":107,"context_line":"            # This check ensures that we have fsid and pool info in url"},{"line_number":108,"context_line":"            if len(pieces) \u003d\u003d 2:"},{"line_number":109,"context_line":"                fsid \u003d pieces[0]"},{"line_number":110,"context_line":"        return {"},{"line_number":111,"context_line":"            \u0027chunk_size\u0027: store_detail.chunk_size,"}],"source_content_type":"text/x-python","patch_set":2,"id":"288c1e79_4aa61623","line":108,"range":{"start_line":108,"start_character":12,"end_line":108,"end_character":32},"in_reply_to":"533c68f1_10872459","updated":"2024-12-04 00:15:13.000000000","message":"The looks correct, but it has a mix of assuming that the driver is setting the _url_prefix correctly and also checking some conditions (like at line 108).  Since the name of the data member begins with \u0027_\u0027, that implies that it\u0027s not really an exposed data member and the format could change without notifying consumers.  So I wonder if instead of checking only length, you should also check the string content at line 102, something like:\n\n```\n    if url_prefix and url_prefix.startswith(prefix) and len(url_prefix) \u003e len(prefix):\n    \n```\n\nI also suggest changing the comment at line 107 to something like \"we expect the rbd store\u0027s url format to look like \u0027rbd://%s/%s/\u0027 where the fsid is in the first position; if there are more than 2 pieces, then something has changed in the driver code, so we won\u0027t set the fsid\"","commit_id":"e1f547c2cf11065a4afd05b8e4c97c8ee4fb1a19"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"23a331ecade5de063bc60f7d054f24869950dbe2","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                url_prefix[:-1] if url_prefix.endswith(\u0027/\u0027) else url_prefix)"},{"line_number":106,"context_line":"            pieces \u003d url_prefix[len(prefix):].split(\u0027/\u0027)"},{"line_number":107,"context_line":"            # This check ensures that we have fsid and pool info in url"},{"line_number":108,"context_line":"            if len(pieces) \u003d\u003d 2:"},{"line_number":109,"context_line":"                fsid \u003d pieces[0]"},{"line_number":110,"context_line":"        return {"},{"line_number":111,"context_line":"            \u0027chunk_size\u0027: store_detail.chunk_size,"}],"source_content_type":"text/x-python","patch_set":2,"id":"8f3d0c95_8da0e4b2","line":108,"range":{"start_line":108,"start_character":12,"end_line":108,"end_character":32},"in_reply_to":"5492c5ef_8a9c57e9","updated":"2024-12-04 08:31:14.000000000","message":"Done","commit_id":"e1f547c2cf11065a4afd05b8e4c97c8ee4fb1a19"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"5ba9ac6f16342a54f50360c8b63cd5aa917438b9","unresolved":true,"context_lines":[{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    @staticmethod"},{"line_number":96,"context_line":"    def _get_rbd_properties(store_detail):"},{"line_number":97,"context_line":"        fsid \u003d None"},{"line_number":98,"context_line":"        prefix \u003d \u0027rbd://\u0027"},{"line_number":99,"context_line":"        url_prefix \u003d store_detail._url_prefix"},{"line_number":100,"context_line":"        # When fsid and pool info are not available,"},{"line_number":101,"context_line":"        # _url_prefix is same as prefix"},{"line_number":102,"context_line":"        if url_prefix and len(url_prefix) \u003e len(prefix):"},{"line_number":103,"context_line":"            # Remove last trailing forward slash"},{"line_number":104,"context_line":"            url_prefix \u003d ("},{"line_number":105,"context_line":"                url_prefix[:-1] if url_prefix.endswith(\u0027/\u0027) else url_prefix)"},{"line_number":106,"context_line":"            pieces \u003d url_prefix[len(prefix):].split(\u0027/\u0027)"},{"line_number":107,"context_line":"            # This check ensures that we have fsid and pool info in url"},{"line_number":108,"context_line":"            if len(pieces) \u003d\u003d 2:"},{"line_number":109,"context_line":"                fsid \u003d pieces[0]"},{"line_number":110,"context_line":"        return {"},{"line_number":111,"context_line":"            \u0027chunk_size\u0027: store_detail.chunk_size,"},{"line_number":112,"context_line":"            \u0027pool\u0027: store_detail.pool,"}],"source_content_type":"text/x-python","patch_set":2,"id":"6334146e_c64a4a1f","line":109,"range":{"start_line":97,"start_character":8,"end_line":109,"end_character":32},"updated":"2024-12-03 15:18:55.000000000","message":"I think you can extract this to a new method _get_fsid_from_url(url_prefix)","commit_id":"e1f547c2cf11065a4afd05b8e4c97c8ee4fb1a19"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d72d5a8e3766defb9ec88fe2952045beef767e63","unresolved":false,"context_lines":[{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    @staticmethod"},{"line_number":96,"context_line":"    def _get_rbd_properties(store_detail):"},{"line_number":97,"context_line":"        fsid \u003d None"},{"line_number":98,"context_line":"        prefix \u003d \u0027rbd://\u0027"},{"line_number":99,"context_line":"        url_prefix \u003d store_detail._url_prefix"},{"line_number":100,"context_line":"        # When fsid and pool info are not available,"},{"line_number":101,"context_line":"        # _url_prefix is same as prefix"},{"line_number":102,"context_line":"        if url_prefix and len(url_prefix) \u003e len(prefix):"},{"line_number":103,"context_line":"            # Remove last trailing forward slash"},{"line_number":104,"context_line":"            url_prefix \u003d ("},{"line_number":105,"context_line":"                url_prefix[:-1] if url_prefix.endswith(\u0027/\u0027) else url_prefix)"},{"line_number":106,"context_line":"            pieces \u003d url_prefix[len(prefix):].split(\u0027/\u0027)"},{"line_number":107,"context_line":"            # This check ensures that we have fsid and pool info in url"},{"line_number":108,"context_line":"            if len(pieces) \u003d\u003d 2:"},{"line_number":109,"context_line":"                fsid \u003d pieces[0]"},{"line_number":110,"context_line":"        return {"},{"line_number":111,"context_line":"            \u0027chunk_size\u0027: store_detail.chunk_size,"},{"line_number":112,"context_line":"            \u0027pool\u0027: store_detail.pool,"}],"source_content_type":"text/x-python","patch_set":2,"id":"8ced34ce_43e32e41","line":109,"range":{"start_line":97,"start_character":8,"end_line":109,"end_character":32},"in_reply_to":"6334146e_c64a4a1f","updated":"2024-12-03 20:07:24.000000000","message":"Done","commit_id":"e1f547c2cf11065a4afd05b8e4c97c8ee4fb1a19"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"f0941917607c4bf0c11571602e1debf0945900fe","unresolved":true,"context_lines":[{"line_number":96,"context_line":"    def _get_fsid_from_url(store_detail):"},{"line_number":97,"context_line":"        fsid \u003d None"},{"line_number":98,"context_line":"        prefix \u003d \u0027rbd://\u0027"},{"line_number":99,"context_line":"        url_prefix \u003d store_detail._url_prefix"},{"line_number":100,"context_line":"        # When fsid and pool info are not available,"},{"line_number":101,"context_line":"        # _url_prefix is same as prefix"},{"line_number":102,"context_line":"        if len(url_prefix) \u003e len(prefix):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3dd41162_78b22d2d","line":99,"range":{"start_line":99,"start_character":21,"end_line":99,"end_character":45},"updated":"2024-12-04 05:40:32.000000000","message":"also it does have public property `url_prefix`","commit_id":"d975ebd21d1132db81a87392ac5dcbd4c740b658"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"23a331ecade5de063bc60f7d054f24869950dbe2","unresolved":false,"context_lines":[{"line_number":96,"context_line":"    def _get_fsid_from_url(store_detail):"},{"line_number":97,"context_line":"        fsid \u003d None"},{"line_number":98,"context_line":"        prefix \u003d \u0027rbd://\u0027"},{"line_number":99,"context_line":"        url_prefix \u003d store_detail._url_prefix"},{"line_number":100,"context_line":"        # When fsid and pool info are not available,"},{"line_number":101,"context_line":"        # _url_prefix is same as prefix"},{"line_number":102,"context_line":"        if len(url_prefix) \u003e len(prefix):"}],"source_content_type":"text/x-python","patch_set":3,"id":"611e3f0e_3223825d","line":99,"range":{"start_line":99,"start_character":21,"end_line":99,"end_character":45},"in_reply_to":"3dd41162_78b22d2d","updated":"2024-12-04 08:31:14.000000000","message":"I see now we have this so it\u0027s better to use it.\n\nhttps://github.com/openstack/glance_store/blob/master/glance_store/driver.py#L86-L88","commit_id":"d975ebd21d1132db81a87392ac5dcbd4c740b658"}]}