)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"895912e921c6a8d9be35f69d2a4a6ac8d6c033a7","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"274be344_90cb0741","updated":"2025-04-03 16:47:06.000000000","message":"+1 because im not sure how stable this is.\n\nthe repoducer more or less makes sense to me have you ran this in a loop locally to see if it sometimes fails?","commit_id":"a8721071823d945ed2d9062693be628497df08ac"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9953a96a01d7a74d1df2c9b73eb0100b007b4ea8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"10248069_9b1966a4","updated":"2025-04-03 17:07:16.000000000","message":"recheck lets see if this is stable in ci as well","commit_id":"a8721071823d945ed2d9062693be628497df08ac"}],"nova/tests/fixtures/cinder.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"895912e921c6a8d9be35f69d2a4a6ac8d6c033a7","unresolved":true,"context_lines":[{"line_number":484,"context_line":"        return attachment_ids"},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"    def create_vol_attachment(self, volume_id, instance_id):"},{"line_number":487,"context_line":"        \"\"\"Create a Cinder volume attachment that Nova is not aware of.\"\"\""},{"line_number":488,"context_line":"        attachment_id \u003d uuidutils.generate_uuid()"},{"line_number":489,"context_line":"        if self.attachment_error_id is not None:"},{"line_number":490,"context_line":"            attachment_id \u003d self.attachment_error_id"}],"source_content_type":"text/x-python","patch_set":1,"id":"6ab9017b_bf38a7de","line":487,"updated":"2025-04-03 16:47:06.000000000","message":"in reality that is somehting that shoudl never happen because we do not suprpot attaching volume to a nova instnace via cidner but ya this is jsut simulating the the cinder side of it not what happens if you do server volume atach.\n\nso it makes sesne to call out the scope of what we are simulating here.","commit_id":"a8721071823d945ed2d9062693be628497df08ac"}],"nova/tests/functional/regressions/test_bug_2016173.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"895912e921c6a8d9be35f69d2a4a6ac8d6c033a7","unresolved":true,"context_lines":[{"line_number":29,"context_line":"        super().setUp()"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"        def fail_detach(*args, **kwargs):"},{"line_number":32,"context_line":"            raise test.TestingException(\u0027failed\u0027)"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"        # Fake a volume detach failure from Cinder API."},{"line_number":35,"context_line":"        self.stub_out(\u0027nova.volume.cinder.API.attachment_delete\u0027, fail_detach)"}],"source_content_type":"text/x-python","patch_set":1,"id":"44fb8653_8a809c0b","line":32,"updated":"2025-04-03 16:47:06.000000000","message":"nit: \n\nthis is probably ok in this case but its generally better to use the correct type. i assume we are trying to emulate a 503  form the cidner or openstack clinet?\n\nthe reason i genreally avoid testing excptions is not to fall into case wehre we miss excpet blocks because the types mismatch.\n\nanyway for a repodcuer like this its ok but in genreal i liek to use the correct types if its obvious what those should be.\n\nthe bug does not have an actaul traceback so i cant actully sugest anything beteer then some generic client expction whic his not really any better then testingException.","commit_id":"a8721071823d945ed2d9062693be628497df08ac"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"895912e921c6a8d9be35f69d2a4a6ac8d6c033a7","unresolved":true,"context_lines":[{"line_number":46,"context_line":"            server[\u0027id\u0027],"},{"line_number":47,"context_line":"            {\u0027volumeAttachment\u0027: {\u0027volumeId\u0027: volume_id}})"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        def volume_in_use():"},{"line_number":50,"context_line":"            volume \u003d self.volume_api.get(self.ctxt, volume_id)"},{"line_number":51,"context_line":"            self.assertEqual(\u0027in-use\u0027, volume[\u0027status\u0027])"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        # Wait for the volume to be \"in-use\"."},{"line_number":54,"context_line":"        self._wait_for_assert(volume_in_use)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        # Detach the volume (which will fail internally for this test)."},{"line_number":57,"context_line":"        self.api.delete_server_volume(server[\u0027id\u0027], volume_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"2b75d04f_6319b608","line":54,"range":{"start_line":49,"start_character":0,"end_line":54,"end_character":44},"updated":"2025-04-03 16:47:06.000000000","message":"for this case you coudl use the integrated helper for volume attach or \nbetter add a _attch_volume helper that uses the attachment complete notifcaiotn","commit_id":"a8721071823d945ed2d9062693be628497df08ac"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"895912e921c6a8d9be35f69d2a4a6ac8d6c033a7","unresolved":true,"context_lines":[{"line_number":67,"context_line":"        # \"in-use\" after the detach fails."},{"line_number":68,"context_line":"        self.assertRaises("},{"line_number":69,"context_line":"            AssertionError, self._wait_for_assert, volume_in_use,"},{"line_number":70,"context_line":"            max_retries\u003d3)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7b6cd93c_4f4e7e6d","line":70,"updated":"2025-04-03 16:47:06.000000000","message":"oh ok i was trying ti think how we were getting an assertion error but tis rasied by _wait_for_assert when the predicate function volume_in_use fails.\n\n max_retries\u003d3 is being passed to _wait_for_assert rhater then volume_in_use.\n \n https://github.com/openstack/nova/blob/9d910ec4bf2a12baf3b5f0ec3bc41686413538fb/nova/tests/functional/integrated_helpers.py#L307-L320\n \n so ok this maskes sense.\n \n its not what i was expecting but i see how its working.\n \n normlaly instead of a reqy like this i generally perfer to try and hook the instance notification.\n \n is there any we can wait for like detach.end or something liek that ?.\n \n \n if we use the detach helper  for instances we normally do\n \n  self.notifier.wait_for_versioned_notifications(\n            \u0027instance.interface_detach.end\u0027)\n            \n internally and for shares we have\n \n    def _detach_share_with_error(self, server, share_id):\n        self.api.delete_server_share(server[\u0027id\u0027], share_id)\n        self.notifier.wait_for_versioned_notifications(\n            \u0027instance.share_detach.error\u0027)\n            \n but looking at voluems we use  _wait_for_volume_detach\n \n https://github.com/openstack/nova/blob/9d910ec4bf2a12baf3b5f0ec3bc41686413538fb/nova/tests/functional/integrated_helpers.py#L223-L238\n\n that doing a loop too so i assuem that means we are not sending notification \nproperly for volume detachment as we soudl use those if we were?\n\nlater\n-----\n\nso in the hapy path we do send attaach and detach notifications but we dont have a notificaiotn if we raise.\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/manager.py#L8157-L8185\n\ni guess im debating how racying this is likely to be in ci.\n\nits proably fine but we might want to consier adding helper for detach and or more notifciaton in the future.","commit_id":"a8721071823d945ed2d9062693be628497df08ac"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"96e5a7202f219ab6b6e79196a43bbfcadf0afaa4","unresolved":true,"context_lines":[{"line_number":67,"context_line":"        # \"in-use\" after the detach fails."},{"line_number":68,"context_line":"        self.assertRaises("},{"line_number":69,"context_line":"            AssertionError, self._wait_for_assert, volume_in_use,"},{"line_number":70,"context_line":"            max_retries\u003d3)"}],"source_content_type":"text/x-python","patch_set":1,"id":"6e9be811_6c64d277","line":70,"in_reply_to":"7b6cd93c_4f4e7e6d","updated":"2025-04-03 17:05:51.000000000","message":"{0} nova.tests.functional.regressions.test_bug_2016173.TestVolumeDetachRollback.test_server_create_volume_detach_fails [3.493906s] ... ok\n\n\u003d\u003d\u003d\u003d\u003d\u003d\nTotals\n\u003d\u003d\u003d\u003d\u003d\u003d\nRan: 100 tests in 700.9413 sec.\n - Passed: 100\n - Skipped: 0\n - Expected Fail: 0\n - Unexpected Success: 0\n - Failed: 0\nSum of execute time for each test: 353.3871 sec.\n\n\n\nso runnign it in a loop it seams to be pretty stable.\n\nand it does nto take that long to run so i guess this is fine as is.\n\nif you do want to refactor for other comment that fine but this seams to be ok as is.","commit_id":"a8721071823d945ed2d9062693be628497df08ac"}]}