)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":13671,"name":"Vladislav Belogrudov","email":"v.belogrudov@yadro.com","username":"vb"},"change_message_id":"e8ecd109789fe424ad21ce5a285fa5e8e212873d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"172c9b21_5e93a09b","updated":"2022-07-22 08:09:44.000000000","message":"recheck","commit_id":"bb06a5d63b7c06a5227c6e73aa42acaff9b79841"},{"author":{"_account_id":13671,"name":"Vladislav Belogrudov","email":"v.belogrudov@yadro.com","username":"vb"},"change_message_id":"2cb4ebd1ddeaf3de1bcbc5d6edd44af3e1d3dac6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"2db196f8_f8814408","updated":"2022-07-08 15:33:37.000000000","message":"recheck","commit_id":"bb06a5d63b7c06a5227c6e73aa42acaff9b79841"},{"author":{"_account_id":19234,"name":"Alexey Stupnikov","email":"aleksey.stupnikov@gmail.com","username":"astupnikov"},"change_message_id":"61f4d3776089b668dce38c054117a2efd76ac1fc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"27cae49b_51959e70","updated":"2022-10-03 13:42:04.000000000","message":"We already have similar check implemented in different library [1]. Can we follow same approach here and mention that this function is used in two places or move it to some common lib like nova/api/openstack/compute/volumes.py?\n\n[1]\nhttps://github.com/openstack/nova/blob/master/nova/compute/manager.py#L11015-L11026","commit_id":"b0418928828b590e1ac037a314aa2060b19e9641"},{"author":{"_account_id":13671,"name":"Vladislav Belogrudov","email":"v.belogrudov@yadro.com","username":"vb"},"change_message_id":"a10fd4d9cace8db5d33060b9ae28410254698de3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5f8f019b_c1cf985d","in_reply_to":"27cae49b_51959e70","updated":"2022-10-06 15:23:00.000000000","message":"thanks Alexey! Your comment made me thinking we would not need snapshot waiting in unquiesce() at all 😎 . We should wait snapshots always disregarding VM state.","commit_id":"b0418928828b590e1ac037a314aa2060b19e9641"},{"author":{"_account_id":19234,"name":"Alexey Stupnikov","email":"aleksey.stupnikov@gmail.com","username":"astupnikov"},"change_message_id":"493500e0902421e870937cd4246e155df549c3e6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"7e599f12_f7be50fd","updated":"2022-10-07 12:54:00.000000000","message":"Thank you Vladislav!","commit_id":"bdc62816c0d2498861ede3aeae33a7cbeeba8a29"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"31028c25613aacf430fdbcc65ba2c2c9c1dc7d58","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"e21b2839_09fb35f8","updated":"2023-10-20 14:36:50.000000000","message":"This is a huge change and one we should not be making at all, IMHO. This turns a current REST call from a constant-time \"do this long-running thing for me\" operation into an almost-certain-to-timeout blocking call. It breaks the REST API *and* the RPC API without any consideration for compatibility.\n\nI\u0027m marking this as -2 to make sure it sticks. This likely needs to be a spec, with discussion about how we can make something more visible without this heavy-handed approach. PTG is next week, so that would be a good time to have such a discussion.","commit_id":"78f5223fd4f8ae87e53c43240adf64650f977d8e"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"29625dce9a70a25daea73e829b8dda0c4d715183","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"d214178d_168aab1a","updated":"2023-09-13 10:37:34.000000000","message":"can you please add functional test for this as well.","commit_id":"78f5223fd4f8ae87e53c43240adf64650f977d8e"},{"author":{"_account_id":31016,"name":"Ivan Pchelintsev","email":"i.pchelintsev@yadro.com","username":"pcheli"},"change_message_id":"b2673845615d41ada6f3c0b430978d6b7996d33f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"f058f27a_03572255","updated":"2023-09-05 06:58:18.000000000","message":"recheck","commit_id":"78f5223fd4f8ae87e53c43240adf64650f977d8e"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"76ebef3f2a6fc6107611fd3a30ba87bd11b2ce43","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"ff2c6590_a29fb913","in_reply_to":"8ba73370_68858ad4","updated":"2023-09-19 06:07:07.000000000","message":"unless am wrong, then please correct me.\nreason of asking function test is, I understand with this change, wait for snapshot complete will be moved to api from compute.\nfrom CLI prespective, we do not wait for any api action to get complete, we ask compute to finish it and update the status later.\n\nso thats why I think a functional test should be added.\nthanks","commit_id":"78f5223fd4f8ae87e53c43240adf64650f977d8e"},{"author":{"_account_id":31016,"name":"Ivan Pchelintsev","email":"i.pchelintsev@yadro.com","username":"pcheli"},"change_message_id":"4666800eaa8092d77eb6bc45e72777140f270dc7","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"8ba73370_68858ad4","in_reply_to":"d214178d_168aab1a","updated":"2023-09-13 12:39:33.000000000","message":"The functionality is just moved from one place to another.\nIt is well-covered by unit-tests IMO.","commit_id":"78f5223fd4f8ae87e53c43240adf64650f977d8e"}],"nova/compute/api.py":[{"author":{"_account_id":19234,"name":"Alexey Stupnikov","email":"aleksey.stupnikov@gmail.com","username":"astupnikov"},"change_message_id":"493500e0902421e870937cd4246e155df549c3e6","unresolved":true,"context_lines":[{"line_number":3526,"context_line":"                            raise loopingcall.LoopingCallDone()"},{"line_number":3527,"context_line":"                        elif status !\u003d \u0027creating\u0027:"},{"line_number":3528,"context_line":"                            raise exception.VolumeSnapshotError()"},{"line_number":3529,"context_line":"                    timer \u003d loopingcall.FixedIntervalLoopingCall("},{"line_number":3530,"context_line":"                        _wait_snapshot)"},{"line_number":3531,"context_line":"                    timer.start(interval\u003d0.5).wait()"},{"line_number":3532,"context_line":"                return mapping"},{"line_number":3533,"context_line":"            # NOTE(tasker): No error handling is done in the above for loop."},{"line_number":3534,"context_line":"            # This means that if the snapshot fails and throws an exception"}],"source_content_type":"text/x-python","patch_set":6,"id":"290a0755_9f1f6b3d","line":3531,"range":{"start_line":3529,"start_character":0,"end_line":3531,"end_character":52},"updated":"2022-10-07 12:54:00.000000000","message":"Can we get into infinite loop here if something would go wrong on Cinder side? In any case I believe that it would be good to preserve original Error log message from manager.py.","commit_id":"bdc62816c0d2498861ede3aeae33a7cbeeba8a29"},{"author":{"_account_id":13671,"name":"Vladislav Belogrudov","email":"v.belogrudov@yadro.com","username":"vb"},"change_message_id":"4a460506edfde5347e41c87220ea1e31c94d2fc1","unresolved":false,"context_lines":[{"line_number":3526,"context_line":"                            raise loopingcall.LoopingCallDone()"},{"line_number":3527,"context_line":"                        elif status !\u003d \u0027creating\u0027:"},{"line_number":3528,"context_line":"                            raise exception.VolumeSnapshotError()"},{"line_number":3529,"context_line":"                    timer \u003d loopingcall.FixedIntervalLoopingCall("},{"line_number":3530,"context_line":"                        _wait_snapshot)"},{"line_number":3531,"context_line":"                    timer.start(interval\u003d0.5).wait()"},{"line_number":3532,"context_line":"                return mapping"},{"line_number":3533,"context_line":"            # NOTE(tasker): No error handling is done in the above for loop."},{"line_number":3534,"context_line":"            # This means that if the snapshot fails and throws an exception"}],"source_content_type":"text/x-python","patch_set":6,"id":"cc6565b6_3b03c3c8","line":3531,"range":{"start_line":3529,"start_character":0,"end_line":3531,"end_character":52},"in_reply_to":"290a0755_9f1f6b3d","updated":"2022-10-11 12:13:10.000000000","message":"I think it\u0027s responsibility of Cinder to timeout snapshot operation eventually or a user to cancel it.","commit_id":"bdc62816c0d2498861ede3aeae33a7cbeeba8a29"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"31028c25613aacf430fdbcc65ba2c2c9c1dc7d58","unresolved":true,"context_lines":[{"line_number":3534,"context_line":"                            )"},{"line_number":3535,"context_line":"                    timer \u003d loopingcall.FixedIntervalLoopingCall("},{"line_number":3536,"context_line":"                        _wait_snapshot)"},{"line_number":3537,"context_line":"                    timer.start(interval\u003d1).wait()"},{"line_number":3538,"context_line":"                return mapping"},{"line_number":3539,"context_line":"            # NOTE(tasker): No error handling is done in the above for loop."},{"line_number":3540,"context_line":"            # This means that if the snapshot fails and throws an exception"}],"source_content_type":"text/x-python","patch_set":8,"id":"8c403614_fdba1477","line":3537,"updated":"2023-10-20 14:36:50.000000000","message":"IMHO, moving this from compute manager to API is not what we should be doing here. You\u0027ve changed the semantic behavior of an RPC call without a version bump or any sort of compatibility adjustment.\n\nIf we wanted to wait, we should just make that a call instead of a cast (quiesce is already a call) so that the caller waits until it is complete. We could use the long timeout variant since that can take a while to make sure the RPC part doesn\u0027t timeout.\n\nThat all said, moving this to being a blocking operation is completely changing the REST API behavior here. What used to be a near constant-time call is now a long blocking call, which for some backends will almost certainly take longer than the HTTP request timeout of a normal client or a proxy. Such a change should generally be avoided, but definitely not made without a lot of care. Exposing the not-yet-complete-ness of such an operation should be handled in some way *other* than blocking the HTTP call for minutes (or longer).","commit_id":"78f5223fd4f8ae87e53c43240adf64650f977d8e"}],"nova/compute/rpcapi.py":[{"author":{"_account_id":19234,"name":"Alexey Stupnikov","email":"aleksey.stupnikov@gmail.com","username":"astupnikov"},"change_message_id":"493500e0902421e870937cd4246e155df549c3e6","unresolved":true,"context_lines":[{"line_number":1504,"context_line":"                server\u003d_compute_host(None, instance), version\u003dversion)"},{"line_number":1505,"context_line":"        return cctxt.call(ctxt, \u0027quiesce_instance\u0027, instance\u003dinstance)"},{"line_number":1506,"context_line":""},{"line_number":1507,"context_line":"    def unquiesce_instance(self, ctxt, instance):"},{"line_number":1508,"context_line":"        version \u003d self._ver(ctxt, \u00275.0\u0027)"},{"line_number":1509,"context_line":"        cctxt \u003d self.router.client(ctxt).prepare("},{"line_number":1510,"context_line":"                server\u003d_compute_host(None, instance), version\u003dversion)"},{"line_number":1511,"context_line":"        cctxt.cast(ctxt, \u0027unquiesce_instance\u0027, instance\u003dinstance)"},{"line_number":1512,"context_line":""},{"line_number":1513,"context_line":"    def trigger_crash_dump(self, ctxt, instance):"},{"line_number":1514,"context_line":"        version \u003d self._ver(ctxt, \u00275.0\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba4349f0_5bc93f4f","line":1511,"range":{"start_line":1507,"start_character":0,"end_line":1511,"end_character":65},"updated":"2022-10-07 12:54:00.000000000","message":"I am not 100% sure if this should be qualified as change in RPC API. If it could be, then there are some related actions to take...","commit_id":"bdc62816c0d2498861ede3aeae33a7cbeeba8a29"},{"author":{"_account_id":13671,"name":"Vladislav Belogrudov","email":"v.belogrudov@yadro.com","username":"vb"},"change_message_id":"4a460506edfde5347e41c87220ea1e31c94d2fc1","unresolved":false,"context_lines":[{"line_number":1504,"context_line":"                server\u003d_compute_host(None, instance), version\u003dversion)"},{"line_number":1505,"context_line":"        return cctxt.call(ctxt, \u0027quiesce_instance\u0027, instance\u003dinstance)"},{"line_number":1506,"context_line":""},{"line_number":1507,"context_line":"    def unquiesce_instance(self, ctxt, instance):"},{"line_number":1508,"context_line":"        version \u003d self._ver(ctxt, \u00275.0\u0027)"},{"line_number":1509,"context_line":"        cctxt \u003d self.router.client(ctxt).prepare("},{"line_number":1510,"context_line":"                server\u003d_compute_host(None, instance), version\u003dversion)"},{"line_number":1511,"context_line":"        cctxt.cast(ctxt, \u0027unquiesce_instance\u0027, instance\u003dinstance)"},{"line_number":1512,"context_line":""},{"line_number":1513,"context_line":"    def trigger_crash_dump(self, ctxt, instance):"},{"line_number":1514,"context_line":"        version \u003d self._ver(ctxt, \u00275.0\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"75765fff_894fe40b","line":1511,"range":{"start_line":1507,"start_character":0,"end_line":1511,"end_character":65},"in_reply_to":"ba4349f0_5bc93f4f","updated":"2022-10-11 12:13:10.000000000","message":"during upgrade we still are compatible with old compute hosts","commit_id":"bdc62816c0d2498861ede3aeae33a7cbeeba8a29"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"31028c25613aacf430fdbcc65ba2c2c9c1dc7d58","unresolved":true,"context_lines":[{"line_number":1519,"context_line":"        cctxt \u003d self.router.client(ctxt).prepare("},{"line_number":1520,"context_line":"                server\u003d_compute_host(None, instance), version\u003dversion)"},{"line_number":1521,"context_line":"        cctxt.cast(ctxt, \u0027unquiesce_instance\u0027, instance\u003dinstance,"},{"line_number":1522,"context_line":"                   mapping\u003dmapping)"},{"line_number":1523,"context_line":""},{"line_number":1524,"context_line":"    def trigger_crash_dump(self, ctxt, instance):"},{"line_number":1525,"context_line":"        version \u003d self._ver(ctxt, \u00275.0\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"499e8cff_52a8d6b7","side":"PARENT","line":1522,"updated":"2023-10-20 14:36:50.000000000","message":"You\u0027re making breaking RPC changes here without version control or compatibility.","commit_id":"a1fcdeee76f50db9c0383ef27825798e83d4c0b7"}]}