)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d3b976a851572fe3bd1b39d53ba7d0524e68666c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"61a8742b_f3384edc","updated":"2021-11-25 14:55:30.000000000","message":"Nothing actually wrong with this except for a lack of a release note. I do have two tweak suggestions inline though. Let me know if you agree/disagree.\n\nIf you can add a release note I\u0027m +2","commit_id":"66d52cdfe322a8f55e542db768b9985bb5a58646"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"fbcdfb53d4f05cd948a2cf1d66d20a01bdd1c772","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"5f99a0e9_84bc262f","updated":"2021-11-25 14:57:39.000000000","message":"One other idea might be to move the functional test to a separate patch that you place _before_ this one. That way we can prove that this change doesn\u0027t have any user-visible impact, i.e. that we don\u0027t regress. Just an idea 😊","commit_id":"66d52cdfe322a8f55e542db768b9985bb5a58646"},{"author":{"_account_id":34041,"name":"Diwei Zhu","email":"zhu.diw@northeastern.edu","username":"DiweiZhu"},"change_message_id":"c355db66005ebc5d455d6f5fcc085bdc20984365","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"8671eb9e_13658fce","in_reply_to":"5f99a0e9_84bc262f","updated":"2021-11-26 14:45:05.000000000","message":"I think this is a good idea. Should I create a code review for the functional test, which will be rebased on master, and then rebase this one on it?\nI think I can rebase this one to the new patch using the UI. Is there a command line way to do this?\nAs the add volume patch is merged, is it possible to put the functional test before that one?","commit_id":"66d52cdfe322a8f55e542db768b9985bb5a58646"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c2ef0f7a51013163697d685587b99ce716759483","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"bbd035e4_113f6d25","in_reply_to":"8671eb9e_13658fce","updated":"2021-11-26 16:20:43.000000000","message":"\u003e I think this is a good idea. Should I create a code review for the functional test, which will be rebased on master, and then rebase this one on it?\n\nYup. I saw you had created the functional test change so I rebased this for you.\n\n\u003e I think I can rebase this one to the new patch using the UI. Is there a command line way to do this?\n\nYes, you can cherry-pick this change locally and then re-submit it for review.\n\n  git review -d 819473 # the functional test change\n  git review -x 817989 # this change\n  git review -fy\n\nAlternatively, you could check out both branches and then locally rebase this patch on top of the functional test change. One thing to note here is that git-review will automatically override a branch that has a conflicting name, so you\u0027ll have to rename the branches.\n\n\u003e As the add volume patch is merged, is it possible to put the functional test before that one?\n\nUnfortunately not. The change is merged. You\u0027d have to revert it and then remerge it (with a new Change ID) if you wanted to do this.","commit_id":"66d52cdfe322a8f55e542db768b9985bb5a58646"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c2ef0f7a51013163697d685587b99ce716759483","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"16158d83_672d0981","updated":"2021-11-26 16:20:43.000000000","message":"Nice work","commit_id":"fae293dd5218cf4ea03d0a4c44d17b97987dea12"},{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"7fdc00a240bf2fe0de804e0f26cdce4a381f983e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"1f2e83bf_872a1d07","updated":"2021-11-26 16:25:22.000000000","message":"cool","commit_id":"fae293dd5218cf4ea03d0a4c44d17b97987dea12"}],"openstackclient/compute/v2/server.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d3b976a851572fe3bd1b39d53ba7d0524e68666c","unresolved":true,"context_lines":[{"line_number":3813,"context_line":"                    volume_attachment,"},{"line_number":3814,"context_line":"                    server,"},{"line_number":3815,"context_line":"                )"},{"line_number":3816,"context_line":"                found \u003d True"},{"line_number":3817,"context_line":""},{"line_number":3818,"context_line":"        if not found:"},{"line_number":3819,"context_line":"            msg \u003d _(\u0027Target volume attachment not found.\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"823e5c6b_d8c8f338","line":3816,"updated":"2021-11-25 14:55:30.000000000","message":"The more typical, \"pythonic\" way to do this is:\n\n  for x in y:\n      if condition:\n          break\n  else:\n      raise Exception(\u0027foo\u0027)\n\nThe \u0027else\u0027 here only triggers if the loop completes successfully, i.e. \u0027break\u0027 or \u0027return\u0027 is not called. As such, the above would become:\n\n for volume_attachment in compute_client.volume_attachments(server):\n     if volume_attachment.volume_id \u003d\u003d volume.id:\n         compute_client.delete_volume_attachment(\n             volume_attachment,\n             server,\n         )\n         break\n  else:\n     msg \u003d _(\u0027Target volume attachment not found.\u0027)\n     raise exceptions.CommandError(msg)","commit_id":"66d52cdfe322a8f55e542db768b9985bb5a58646"},{"author":{"_account_id":34041,"name":"Diwei Zhu","email":"zhu.diw@northeastern.edu","username":"DiweiZhu"},"change_message_id":"c355db66005ebc5d455d6f5fcc085bdc20984365","unresolved":false,"context_lines":[{"line_number":3813,"context_line":"                    volume_attachment,"},{"line_number":3814,"context_line":"                    server,"},{"line_number":3815,"context_line":"                )"},{"line_number":3816,"context_line":"                found \u003d True"},{"line_number":3817,"context_line":""},{"line_number":3818,"context_line":"        if not found:"},{"line_number":3819,"context_line":"            msg \u003d _(\u0027Target volume attachment not found.\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"6a9f7a17_d28afe52","line":3816,"in_reply_to":"823e5c6b_d8c8f338","updated":"2021-11-26 14:45:05.000000000","message":"Done, thank you.","commit_id":"66d52cdfe322a8f55e542db768b9985bb5a58646"}],"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":"d3b976a851572fe3bd1b39d53ba7d0524e68666c","unresolved":true,"context_lines":[{"line_number":694,"context_line":""},{"line_number":695,"context_line":"        # Get the command object to test"},{"line_number":696,"context_line":"        self.cmd \u003d server.AddServerVolume(self.app, None)"},{"line_number":697,"context_line":"        self.cmd_remove \u003d server.RemoveServerVolume(self.app, None)"},{"line_number":698,"context_line":""},{"line_number":699,"context_line":"        self.servers \u003d self.setup_sdk_servers_mock(count\u003d1)"},{"line_number":700,"context_line":"        self.volumes \u003d self.setup_sdk_volumes_mock(count\u003d1)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9d4984e9_64191c7a","line":697,"updated":"2021-11-25 14:55:30.000000000","message":"This isn\u0027t typically how we do this, and while what you\u0027ve done here works, it might make more sense to avoid \"special flowers\" and do everything the same way. If there\u0027s a lot of shared data here, you can create a base test case that we can reuse. For example:\n\n  class TestServerVolume(TestServer):\n\n      def setUp(self):\n          super().setUp()\n\n          self.methods \u003d ...\n\n          self.servers \u003d ...\n          ...\n\n  class TestServerAddVolume(TestServerVolume):\n      def setUp(self):\n          super().setUp()\n          self.cmd \u003d server.AddServerVolume(self.app, None)\n\n      def test_server_add_volume(self):\n          ...\n\n\n  class TestServerRemoveVolume(TestServerVolume):\n      def setUp(self):\n          super().setUp()\n          self.cmd \u003d server.RemoveServerVolume(self.app, None)\n\n      def test_server_remove_volume(self):\n          ...","commit_id":"66d52cdfe322a8f55e542db768b9985bb5a58646"},{"author":{"_account_id":34041,"name":"Diwei Zhu","email":"zhu.diw@northeastern.edu","username":"DiweiZhu"},"change_message_id":"c355db66005ebc5d455d6f5fcc085bdc20984365","unresolved":false,"context_lines":[{"line_number":694,"context_line":""},{"line_number":695,"context_line":"        # Get the command object to test"},{"line_number":696,"context_line":"        self.cmd \u003d server.AddServerVolume(self.app, None)"},{"line_number":697,"context_line":"        self.cmd_remove \u003d server.RemoveServerVolume(self.app, None)"},{"line_number":698,"context_line":""},{"line_number":699,"context_line":"        self.servers \u003d self.setup_sdk_servers_mock(count\u003d1)"},{"line_number":700,"context_line":"        self.volumes \u003d self.setup_sdk_volumes_mock(count\u003d1)"}],"source_content_type":"text/x-python","patch_set":6,"id":"f012b302_86497cf9","line":697,"in_reply_to":"9d4984e9_64191c7a","updated":"2021-11-26 14:45:05.000000000","message":"Done","commit_id":"66d52cdfe322a8f55e542db768b9985bb5a58646"}]}