)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"360171b400d7da5324f5603966229fd538f78f8b","unresolved":true,"context_lines":[{"line_number":7,"context_line":"Switch command server add volume to sdk."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This change is based on unmerged change:"},{"line_number":10,"context_line":"https://review.opendev.org/c/openstack/python-openstackclient/+/815769"},{"line_number":11,"context_line":"File tests.unit.volume.v2.fakes is modified to provide sdk volume fakes."},{"line_number":12,"context_line":"For test, setup_sdk_volumes_mock() method is created so that volumes"},{"line_number":13,"context_line":"are created in similar way as servers are created."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"79e7fe85_9801b7b6","line":10,"updated":"2021-11-03 12:01:33.000000000","message":"You don\u0027t need to say this. Gerrit captures the dependency chain here and reviewers can simply inspect that if needed.","commit_id":"057f9293e6977a21cf5201a8f3f9ef6abd1ed3af"},{"author":{"_account_id":34041,"name":"Diwei Zhu","email":"zhu.diw@northeastern.edu","username":"DiweiZhu"},"change_message_id":"9a13782cb84db4b71592583f827b46453eaf4acd","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Switch command server add volume to sdk."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This change is based on unmerged change:"},{"line_number":10,"context_line":"https://review.opendev.org/c/openstack/python-openstackclient/+/815769"},{"line_number":11,"context_line":"File tests.unit.volume.v2.fakes is modified to provide sdk volume fakes."},{"line_number":12,"context_line":"For test, setup_sdk_volumes_mock() method is created so that volumes"},{"line_number":13,"context_line":"are created in similar way as servers are created."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"a14677f5_cb77a470","line":10,"in_reply_to":"79e7fe85_9801b7b6","updated":"2021-11-16 19:08:09.000000000","message":"Done","commit_id":"057f9293e6977a21cf5201a8f3f9ef6abd1ed3af"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e7cb88313d500e32b3b068c6e9cf9f2857934aa6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"52b85c9d_e7048cc8","updated":"2021-11-03 12:45:07.000000000","message":"Could you also switch the \"remove volume\" command over? I suspect it\u0027ll be relatively simple to do. You could use a follow-up if you prefer","commit_id":"057f9293e6977a21cf5201a8f3f9ef6abd1ed3af"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"992f5891588a335ba77fd71b633ffc52fc541664","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"97b76e06_4a581a27","updated":"2021-11-03 12:02:19.000000000","message":"Oh, you also need a release note, so -1 for that. Easy fix though","commit_id":"057f9293e6977a21cf5201a8f3f9ef6abd1ed3af"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"360171b400d7da5324f5603966229fd538f78f8b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"4f5c0106_1567b5b8","updated":"2021-11-03 12:01:33.000000000","message":"This looks good to me. Nice work. I have a nit in the commit message and another style nit. You might want to fix these when you respin to fix the issues with the base patch but that\u0027s your call (they\u0027re nits)\n\nWhile reviewing this, I noticed that we were ignoring the result from the call to attach the volume, so I\u0027ve proposed [1] as a patch to add this missing feature. If that patch merges first then this will need a small bit of rework and vice versa. Nothing big though.\n\nIn case it\u0027s useful, here\u0027s how I tested this in a basic DevStack deployment:\n\n  # create a server\n  $ openstack server create --network private --flavor m1.medium --image cirros-0.5.2-x86_64-disk --wait test-server\n\n  # create a volume\n  $ openstack volume create --size 1 foo\n\n  # add a volume to the server\n  $ openstack server add volume test-server foo --enable-delete-on-termination --tag bar\n\n  # ensure the volume is present (you might also want to inspect nova-compute logs if it isn\u0027t)\n  $ openstack server list\n  $ openstack volume list\n  $ openstack server volume list test-server\n\n  # delete the server\n  $ openstack server delete test-server\n\n  # ensure the server and volume are gone (the volume should go because of the flag)\n  $ openstack server list\n  $ openstack volume list\n\nI then repeated the above but without the \u0027--enable-delete-on-termination\u0027 flag and ensure the volume was *not* deleted when the server was deleted.\n\n[1] https://review.opendev.org/c/openstack/python-openstackclient/+/816491","commit_id":"057f9293e6977a21cf5201a8f3f9ef6abd1ed3af"},{"author":{"_account_id":34041,"name":"Diwei Zhu","email":"zhu.diw@northeastern.edu","username":"DiweiZhu"},"change_message_id":"c313b21da7747d91b92eda11c175d3a19227fb4b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"38a4574b_0bea492f","in_reply_to":"4f5c0106_1567b5b8","updated":"2021-11-17 14:13:54.000000000","message":"When I test it like this, right after \u0027add volume\u0027, the volume status is \u0027attaching\u0027 and it\u0027s in the \u0027volume_attached\u0027 field of the server. However, several minutes later, the volume status becomes \u0027available\u0027 and it disappears from the \u0027volume_attached\u0027 field. \nThis happens both before and after the modification.\nThis is not the expected behavior right. What could be the possible cause of this?","commit_id":"057f9293e6977a21cf5201a8f3f9ef6abd1ed3af"},{"author":{"_account_id":34041,"name":"Diwei Zhu","email":"zhu.diw@northeastern.edu","username":"DiweiZhu"},"change_message_id":"9a13782cb84db4b71592583f827b46453eaf4acd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3a9ec98f_e3dcd678","in_reply_to":"52b85c9d_e7048cc8","updated":"2021-11-16 19:08:09.000000000","message":"Sure. I\u0027d like to use a follow-up for it.","commit_id":"057f9293e6977a21cf5201a8f3f9ef6abd1ed3af"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"87a8b4ec2e10ca54c2eed32eab08ef2cb3114c1b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"1a3d98f9_3b41bce2","updated":"2021-11-17 10:24:03.000000000","message":"So close! I missed the fact that you\u0027re still using a legacy FakeResource for the volume attachment resource, when in fact you should be using an openstacksdk-based object. Note that we\u0027re going to have to merge [1] to add e.g. BDM UUID information, unfortunately, so this particular change might have to wait a bit.\n\n[1] https://review.opendev.org/c/openstack/openstacksdk/+/817997","commit_id":"fd7c411e452dcda82785ed3f814eabb24fa30827"},{"author":{"_account_id":34041,"name":"Diwei Zhu","email":"zhu.diw@northeastern.edu","username":"DiweiZhu"},"change_message_id":"c8015b5fb1dd9b6a5d0147a7a2c5426c243f03d9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"d247edac_b4a7848b","updated":"2021-11-17 15:22:24.000000000","message":"recheck\nError says \u0027VolumeAttachment does not have delete_on_termination\u0027 in one of the unit test but things works fine on my local machine. If it fails again I will manually add that attribute to the object.","commit_id":"01f5f1813602df863eee72f3176a32657cbac72d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"0e64f130f5caa55ae43662bf74ac84f0a58f8037","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"f5eca194_e29f8689","updated":"2021-11-19 16:49:34.000000000","message":"(to be clear, you can ignore my comment about potentially simplifying things here: the release note is the only thing we _need_)","commit_id":"57252b65355ef1e5a67d4a95ac6ab2ecba99060f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e154bb393116bbd0631235f0cec4a9d970d61628","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"1a657c80_68144b89","updated":"2021-11-19 16:47:42.000000000","message":"Just need a release note and we\u0027re good, I think","commit_id":"57252b65355ef1e5a67d4a95ac6ab2ecba99060f"},{"author":{"_account_id":34041,"name":"Diwei Zhu","email":"zhu.diw@northeastern.edu","username":"DiweiZhu"},"change_message_id":"83beac8f0bbbd7f1311a2a01bf587f3e2e564dc4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"4d6a960f_4985ee37","in_reply_to":"f5eca194_e29f8689","updated":"2021-11-22 16:03:41.000000000","message":"Thanks. I see that what get_volume does is returning mock.Mock(side_effect\u003dvolumes). I found there\u0027s only two locations this find_* is used in test_server.py so I modified them both.","commit_id":"57252b65355ef1e5a67d4a95ac6ab2ecba99060f"}],"openstackclient/compute/v2/server.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"360171b400d7da5324f5603966229fd538f78f8b","unresolved":true,"context_lines":[{"line_number":552,"context_line":"        volume_client \u003d self.app.client_manager.sdk_connection.volume"},{"line_number":553,"context_line":""},{"line_number":554,"context_line":"        server \u003d compute_client.find_server("},{"line_number":555,"context_line":"            parsed_args.server, ignore_missing\u003dFalse)"},{"line_number":556,"context_line":"        volume \u003d volume_client.find_volume("},{"line_number":557,"context_line":"            parsed_args.volume, ignore_missing\u003dFalse)"},{"line_number":558,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"d73529a5_e5377c3b","line":555,"updated":"2021-11-03 12:01:33.000000000","message":"style nit: can you drag the bracket down?\n\n  server \u003d compute_client.find_server(\n      parsed_args.server, ignore_missing\u003dFalse,\n  )","commit_id":"057f9293e6977a21cf5201a8f3f9ef6abd1ed3af"},{"author":{"_account_id":34041,"name":"Diwei Zhu","email":"zhu.diw@northeastern.edu","username":"DiweiZhu"},"change_message_id":"9a13782cb84db4b71592583f827b46453eaf4acd","unresolved":false,"context_lines":[{"line_number":552,"context_line":"        volume_client \u003d self.app.client_manager.sdk_connection.volume"},{"line_number":553,"context_line":""},{"line_number":554,"context_line":"        server \u003d compute_client.find_server("},{"line_number":555,"context_line":"            parsed_args.server, ignore_missing\u003dFalse)"},{"line_number":556,"context_line":"        volume \u003d volume_client.find_volume("},{"line_number":557,"context_line":"            parsed_args.volume, ignore_missing\u003dFalse)"},{"line_number":558,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"b808435a_bb4b949b","line":555,"in_reply_to":"d73529a5_e5377c3b","updated":"2021-11-16 19:08:09.000000000","message":"Done","commit_id":"057f9293e6977a21cf5201a8f3f9ef6abd1ed3af"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"87a8b4ec2e10ca54c2eed32eab08ef2cb3114c1b","unresolved":false,"context_lines":[{"line_number":549,"context_line":""},{"line_number":550,"context_line":"    def take_action(self, parsed_args):"},{"line_number":551,"context_line":"        compute_client \u003d self.app.client_manager.sdk_connection.compute"},{"line_number":552,"context_line":"        volume_client \u003d self.app.client_manager.sdk_connection.volume"},{"line_number":553,"context_line":""},{"line_number":554,"context_line":"        server \u003d compute_client.find_server("},{"line_number":555,"context_line":"            parsed_args.server,"}],"source_content_type":"text/x-python","patch_set":4,"id":"587d2d7d_d352d202","line":552,"updated":"2021-11-17 10:24:03.000000000","message":"I think this will be our first use of the SDK for cinder stuff (assuming I searched for the the correct thing). Hurrah 🎉","commit_id":"fd7c411e452dcda82785ed3f814eabb24fa30827"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"87a8b4ec2e10ca54c2eed32eab08ef2cb3114c1b","unresolved":false,"context_lines":[{"line_number":614,"context_line":"            utils.get_item_properties("},{"line_number":615,"context_line":"                volume_attachment,"},{"line_number":616,"context_line":"                columns,"},{"line_number":617,"context_line":"            )"},{"line_number":618,"context_line":"        )"},{"line_number":619,"context_line":""},{"line_number":620,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"d8eb23df_f641439e","line":617,"updated":"2021-11-17 10:24:03.000000000","message":"style nit: You could put this on one line now, since you no longer need to wrap it:\n\n  utils.get_item_properties(volume_attachment, columns),","commit_id":"fd7c411e452dcda82785ed3f814eabb24fa30827"},{"author":{"_account_id":34041,"name":"Diwei Zhu","email":"zhu.diw@northeastern.edu","username":"DiweiZhu"},"change_message_id":"c313b21da7747d91b92eda11c175d3a19227fb4b","unresolved":false,"context_lines":[{"line_number":614,"context_line":"            utils.get_item_properties("},{"line_number":615,"context_line":"                volume_attachment,"},{"line_number":616,"context_line":"                columns,"},{"line_number":617,"context_line":"            )"},{"line_number":618,"context_line":"        )"},{"line_number":619,"context_line":""},{"line_number":620,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"4a0325c2_d9326fb2","line":617,"in_reply_to":"d8eb23df_f641439e","updated":"2021-11-17 14:13:54.000000000","message":"Done","commit_id":"fd7c411e452dcda82785ed3f814eabb24fa30827"}],"openstackclient/tests/unit/compute/v2/test_server.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"87a8b4ec2e10ca54c2eed32eab08ef2cb3114c1b","unresolved":true,"context_lines":[{"line_number":705,"context_line":"        self.servers \u003d self.setup_sdk_servers_mock(count\u003d1)"},{"line_number":706,"context_line":"        self.volumes \u003d self.setup_sdk_volumes_mock(count\u003d1)"},{"line_number":707,"context_line":"        self.volume_attachment \u003d \\"},{"line_number":708,"context_line":"            compute_fakes.FakeVolumeAttachment.create_one_volume_attachment()"},{"line_number":709,"context_line":"        self.volume_attachment.server_id \u003d self.servers[0].id"},{"line_number":710,"context_line":"        self.volume_attachment.volume_id \u003d self.volumes[0].id"},{"line_number":711,"context_line":"        self.volume_attachment.id \u003d self.volumes[0].id"}],"source_content_type":"text/x-python","patch_set":4,"id":"6f85359b_e3455f10","line":708,"updated":"2021-11-17 10:24:03.000000000","message":"This is an example of a novaclient-based fake resource but what you actually want here is a fake openstacksdk-based resource, right? (i.e. a variant of \u0027openstack.compute.v2.volume_attachment.VolumeAttachment\u0027). This would probably take the form of a \u0027create_one_sdk_volume_attachment\u0027 helper.","commit_id":"fd7c411e452dcda82785ed3f814eabb24fa30827"},{"author":{"_account_id":34041,"name":"Diwei Zhu","email":"zhu.diw@northeastern.edu","username":"DiweiZhu"},"change_message_id":"c313b21da7747d91b92eda11c175d3a19227fb4b","unresolved":false,"context_lines":[{"line_number":705,"context_line":"        self.servers \u003d self.setup_sdk_servers_mock(count\u003d1)"},{"line_number":706,"context_line":"        self.volumes \u003d self.setup_sdk_volumes_mock(count\u003d1)"},{"line_number":707,"context_line":"        self.volume_attachment \u003d \\"},{"line_number":708,"context_line":"            compute_fakes.FakeVolumeAttachment.create_one_volume_attachment()"},{"line_number":709,"context_line":"        self.volume_attachment.server_id \u003d self.servers[0].id"},{"line_number":710,"context_line":"        self.volume_attachment.volume_id \u003d self.volumes[0].id"},{"line_number":711,"context_line":"        self.volume_attachment.id \u003d self.volumes[0].id"}],"source_content_type":"text/x-python","patch_set":4,"id":"a883a881_b39766c5","line":708,"in_reply_to":"6f85359b_e3455f10","updated":"2021-11-17 14:13:54.000000000","message":"Done","commit_id":"fd7c411e452dcda82785ed3f814eabb24fa30827"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"87a8b4ec2e10ca54c2eed32eab08ef2cb3114c1b","unresolved":true,"context_lines":[{"line_number":707,"context_line":"        self.volume_attachment \u003d \\"},{"line_number":708,"context_line":"            compute_fakes.FakeVolumeAttachment.create_one_volume_attachment()"},{"line_number":709,"context_line":"        self.volume_attachment.server_id \u003d self.servers[0].id"},{"line_number":710,"context_line":"        self.volume_attachment.volume_id \u003d self.volumes[0].id"},{"line_number":711,"context_line":"        self.volume_attachment.id \u003d self.volumes[0].id"},{"line_number":712,"context_line":""},{"line_number":713,"context_line":"        self.sdk_client.create_volume_attachment.return_value \u003d \\"}],"source_content_type":"text/x-python","patch_set":4,"id":"331b084d_afa3fd1f","line":710,"updated":"2021-11-17 10:24:03.000000000","message":"nit: I _think_ you could set this via your call to \u0027create_one_volume_attachment\u0027?\n\n  self.volume_attachment \u003d \\\n      compute_fakes.FakeVolumeAttachment.create_one_volume_attachment(\n         server_id\u003dself.servers[0].id, volume_id\u003dself.volumes[0].id,\n      )\n\nthough of course, you won\u0027t be using \u0027create_one_volume_attachment\u0027 given the above","commit_id":"fd7c411e452dcda82785ed3f814eabb24fa30827"},{"author":{"_account_id":34041,"name":"Diwei Zhu","email":"zhu.diw@northeastern.edu","username":"DiweiZhu"},"change_message_id":"c313b21da7747d91b92eda11c175d3a19227fb4b","unresolved":false,"context_lines":[{"line_number":707,"context_line":"        self.volume_attachment \u003d \\"},{"line_number":708,"context_line":"            compute_fakes.FakeVolumeAttachment.create_one_volume_attachment()"},{"line_number":709,"context_line":"        self.volume_attachment.server_id \u003d self.servers[0].id"},{"line_number":710,"context_line":"        self.volume_attachment.volume_id \u003d self.volumes[0].id"},{"line_number":711,"context_line":"        self.volume_attachment.id \u003d self.volumes[0].id"},{"line_number":712,"context_line":""},{"line_number":713,"context_line":"        self.sdk_client.create_volume_attachment.return_value \u003d \\"}],"source_content_type":"text/x-python","patch_set":4,"id":"79a625e2_9cdaf92f","line":710,"in_reply_to":"331b084d_afa3fd1f","updated":"2021-11-17 14:13:54.000000000","message":"Done","commit_id":"fd7c411e452dcda82785ed3f814eabb24fa30827"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e154bb393116bbd0631235f0cec4a9d970d61628","unresolved":false,"context_lines":[{"line_number":164,"context_line":"            volume_fakes.FakeVolume.get_volumes("},{"line_number":165,"context_line":"                volumes,"},{"line_number":166,"context_line":"                0,"},{"line_number":167,"context_line":"            )"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        return volumes"},{"line_number":170,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"0b8d46da_ec102e36","line":167,"updated":"2021-11-19 16:47:42.000000000","message":"This is a bit confusing. All that this is effectively doing is:\n\n  self.sdk_volume_client.find_volume \u003d mock.Mock(side_effect\u003dvolumes)\n\nwhich I think is effectively equal to:\n\n  self.sdk_volume_client.find_volume.side_effect \u003d volumes\n\nsince \u0027self.sdk_volume_client.find_volume\u0027 should be a \u0027mock.Mock\u0027 object already.\n\nYou don\u0027t need to change anything, but it probably is worth mentioning that we can simplify things here","commit_id":"57252b65355ef1e5a67d4a95ac6ab2ecba99060f"}]}