)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8f38c1bce53256129290f0f82b809fe2f22016b5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"2bd37b11_8b9ac0f5","updated":"2022-10-14 13:26:10.000000000","message":"i think a unit test reproducer would be just as valuable instead of this functional test. if you want to use the functional test framework then you shoudl move this to the regression folder. ","commit_id":"b524e759fc192e886a13fc8dd816dc2d6c641f86"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"621b756986e6c079e2fb86cc010b0880888f816a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"ae265fcf_559762cf","updated":"2022-11-29 10:11:11.000000000","message":"Looks good overall, just requested couple of styling changes. I have comment in the fix itself so I think you need to respin the series anyhow.","commit_id":"eb1397642b3610a61cf0396c16553992245e286d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a1530bb9bd088fd2ab40e3b1c0f65ccc7e773b7f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"630ca634_57f0ccd3","updated":"2022-12-01 08:16:15.000000000","message":"Thanks for having modified the styling.","commit_id":"50802572dcb5958d3843799092a41688742a096d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"799e16e802bfef67ebe10ad5286ca6879864c9a9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c83ec016_476cd900","updated":"2022-11-29 14:54:29.000000000","message":"thanks you!","commit_id":"50802572dcb5958d3843799092a41688742a096d"}],"nova/tests/fixtures/cinder.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"7016fa668b0389ab984d6e6b7162ff5e0fa99437","unresolved":true,"context_lines":[{"line_number":169,"context_line":"                volume[\u0027volume_image_metadata\u0027] \u003d {"},{"line_number":170,"context_line":"                    \"os_require_quiesce\": \"True\","},{"line_number":171,"context_line":"                    \"hw_qemu_guest_agent\": \"True\""},{"line_number":172,"context_line":"            }"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"            if volume_id \u003d\u003d self.IMAGE_WITH_TRAITS_BACKED_VOL:"},{"line_number":175,"context_line":"                volume[\u0027bootable\u0027] \u003d True"}],"source_content_type":"text/x-python","patch_set":3,"id":"108b601e_1047e64f","line":172,"updated":"2022-11-14 08:43:22.000000000","message":"the functional test could just check that for BFV instance we would return the same exception but here it reproduces the exact same call than the bug report, so I\u0027m OK.","commit_id":"eb1397642b3610a61cf0396c16553992245e286d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"7016fa668b0389ab984d6e6b7162ff5e0fa99437","unresolved":true,"context_lines":[{"line_number":349,"context_line":""},{"line_number":350,"context_line":"        def fake_get_absolute_limits(_self, context):"},{"line_number":351,"context_line":"            limits \u003d {\u0027totalSnapshotsUsed\u0027: 0, \u0027maxTotalSnapshots\u0027: -1}"},{"line_number":352,"context_line":"            return limits"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"        self.test.stub_out("},{"line_number":355,"context_line":"            \u0027nova.volume.cinder.API.attachment_create\u0027, fake_attachment_create)"}],"source_content_type":"text/x-python","patch_set":3,"id":"b6599942_c7b21c9f","line":352,"updated":"2022-11-14 08:43:22.000000000","message":"note to reviewers: this is required to be mocked as this is called before the quiesce call we mock.","commit_id":"eb1397642b3610a61cf0396c16553992245e286d"}],"nova/tests/functional/integrated_helpers.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"21f345285348dacc6a6943a26897e1bd7b065131","unresolved":true,"context_lines":[{"line_number":633,"context_line":"            return self._wait_for_state_change(server, \u0027SHUTOFF\u0027)"},{"line_number":634,"context_line":"        return server"},{"line_number":635,"context_line":""},{"line_number":636,"context_line":"    def _create_snapshot(self, server, snapshot_name):"},{"line_number":637,"context_line":"        \"\"\"Create server snapshot.\"\"\""},{"line_number":638,"context_line":"        self.api.post_server_action("},{"line_number":639,"context_line":"            server[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"1c1bb9af_ad3ccdfa","line":636,"range":{"start_line":636,"start_character":8,"end_line":636,"end_character":24},"updated":"2022-10-14 10:57:05.000000000","message":"nit: i proably would have called it _snapshot_server to make it allignt with the other server actions but this works","commit_id":"2464612c65102dc6eab543af4354758769a996a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"849a8f9488104da03d7dfe5b19c34a194cb72976","unresolved":true,"context_lines":[{"line_number":633,"context_line":"            return self._wait_for_state_change(server, \u0027SHUTOFF\u0027)"},{"line_number":634,"context_line":"        return server"},{"line_number":635,"context_line":""},{"line_number":636,"context_line":"    def _create_snapshot(self, server, snapshot_name):"},{"line_number":637,"context_line":"        \"\"\"Create server snapshot.\"\"\""},{"line_number":638,"context_line":"        self.api.post_server_action("},{"line_number":639,"context_line":"            server[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"d77984b0_eb98fb46","line":636,"range":{"start_line":636,"start_character":8,"end_line":636,"end_character":24},"in_reply_to":"1c1bb9af_ad3ccdfa","updated":"2022-10-14 13:03:55.000000000","message":"Fixed the name.\n\nIn this class, rest of the method perform operation on server, like start, evacuate, suspend, resize et-cetera, those who changes server.\n\nBut snapshot create, do not change server.\n\nShould we move it to some other class ? If yes, where can we put it ?","commit_id":"2464612c65102dc6eab543af4354758769a996a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"d6fe94d7e2022ebeb9143e7232442ea583bd5bac","unresolved":false,"context_lines":[{"line_number":633,"context_line":"            return self._wait_for_state_change(server, \u0027SHUTOFF\u0027)"},{"line_number":634,"context_line":"        return server"},{"line_number":635,"context_line":""},{"line_number":636,"context_line":"    def _create_snapshot(self, server, snapshot_name):"},{"line_number":637,"context_line":"        \"\"\"Create server snapshot.\"\"\""},{"line_number":638,"context_line":"        self.api.post_server_action("},{"line_number":639,"context_line":"            server[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"ee0b7c88_6a0b22e9","line":636,"range":{"start_line":636,"start_character":8,"end_line":636,"end_character":24},"in_reply_to":"9fee72b4_ae43ff8f","updated":"2022-10-17 09:35:29.000000000","message":"Ack","commit_id":"2464612c65102dc6eab543af4354758769a996a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8f38c1bce53256129290f0f82b809fe2f22016b5","unresolved":true,"context_lines":[{"line_number":633,"context_line":"            return self._wait_for_state_change(server, \u0027SHUTOFF\u0027)"},{"line_number":634,"context_line":"        return server"},{"line_number":635,"context_line":""},{"line_number":636,"context_line":"    def _create_snapshot(self, server, snapshot_name):"},{"line_number":637,"context_line":"        \"\"\"Create server snapshot.\"\"\""},{"line_number":638,"context_line":"        self.api.post_server_action("},{"line_number":639,"context_line":"            server[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fee72b4_ae43ff8f","line":636,"range":{"start_line":636,"start_character":8,"end_line":636,"end_character":24},"in_reply_to":"d77984b0_eb98fb46","updated":"2022-10-14 13:26:10.000000000","message":"its a operation on the server not the volume.\n\nand it does change the server state during the shapsthot the task state change on the service isntance several times.\n\nso no this is in the correctly location but this is an instnace/server action.\n\nhttps://docs.openstack.org/api-ref/compute/?expanded\u003dcreate-image-createimage-action-detail%2Ccreate-snapshot-detail#create-image-createimage-action\n\nthis is part of https://docs.openstack.org/api-ref/compute/#servers-run-an-action-servers-action","commit_id":"2464612c65102dc6eab543af4354758769a996a4"}],"nova/tests/functional/libvirt/test_libvirt.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"21f345285348dacc6a6943a26897e1bd7b065131","unresolved":true,"context_lines":[{"line_number":57,"context_line":"                client.OpenStackApiException,"},{"line_number":58,"context_line":"                self._create_snapshot, server, \"snapshot-1\""},{"line_number":59,"context_line":"            )"},{"line_number":60,"context_line":"        self.assertEqual(500, excep.response.status_code)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fb2c7edf_9c81852d","line":60,"updated":"2022-10-14 10:57:05.000000000","message":"why woudl this fail.\nin a real deployment if the server had the qemu guest agent installed it woudl succeed\n\nso this still looks invlaid to me.\n\nas i said in other review i do not think we should add a funcitonl test fo this.\n\nto repoduce the issue you either need to do fault injection by simulating mocking/poisonging the call to libvirt or you need to modify the fixture to read a class constant to determin if the image contians the qemu guest agent.\n\nwe can do either but it makes this closer to a unit test.","commit_id":"2464612c65102dc6eab543af4354758769a996a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8f38c1bce53256129290f0f82b809fe2f22016b5","unresolved":true,"context_lines":[{"line_number":57,"context_line":"                client.OpenStackApiException,"},{"line_number":58,"context_line":"                self._create_snapshot, server, \"snapshot-1\""},{"line_number":59,"context_line":"            )"},{"line_number":60,"context_line":"        self.assertEqual(500, excep.response.status_code)"}],"source_content_type":"text/x-python","patch_set":1,"id":"b38f1a47_64bc2089","line":60,"in_reply_to":"7bd20ee4_97c4e70d","updated":"2022-10-14 13:26:10.000000000","message":"yes this a repoducer but by mockign you are fecctivly just making this a junit test. we have some regression test like this but you should not really do this in the current file.\n\n\nif you want to mock like this you shoudl move it to the regressions module","commit_id":"2464612c65102dc6eab543af4354758769a996a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"d6fe94d7e2022ebeb9143e7232442ea583bd5bac","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                client.OpenStackApiException,"},{"line_number":58,"context_line":"                self._create_snapshot, server, \"snapshot-1\""},{"line_number":59,"context_line":"            )"},{"line_number":60,"context_line":"        self.assertEqual(500, excep.response.status_code)"}],"source_content_type":"text/x-python","patch_set":1,"id":"edd89386_feb710b2","line":60,"in_reply_to":"b38f1a47_64bc2089","updated":"2022-10-17 09:35:29.000000000","message":"Moved to regression","commit_id":"2464612c65102dc6eab543af4354758769a996a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"849a8f9488104da03d7dfe5b19c34a194cb72976","unresolved":true,"context_lines":[{"line_number":57,"context_line":"                client.OpenStackApiException,"},{"line_number":58,"context_line":"                self._create_snapshot, server, \"snapshot-1\""},{"line_number":59,"context_line":"            )"},{"line_number":60,"context_line":"        self.assertEqual(500, excep.response.status_code)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7bd20ee4_97c4e70d","line":60,"in_reply_to":"fb2c7edf_9c81852d","updated":"2022-10-14 13:03:55.000000000","message":"Updated fixture to add a constant IMAGE_BACKED_VOL_QUIESCE, which will set qemu guest agent properties, so libvirt fixture can call driver._set_quiesced\n\nAdded fsfreez mock to throw libvirtError, later on fail, API returns 500, as in this bug, which we can assert.\n\nI was trying to test whole set_quiesced functionality, so added in functional tests, but I think now its kind of become regression tests - repoducer.\nIn patch having fix of this issue, added a unit test which only tests the exception we added.\nhttps://review.opendev.org/c/openstack/nova/+/852171/","commit_id":"2464612c65102dc6eab543af4354758769a996a4"}],"nova/tests/functional/regressions/test_bug_1980720.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"621b756986e6c079e2fb86cc010b0880888f816a","unresolved":true,"context_lines":[{"line_number":21,"context_line":"from unittest import mock"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"class LibvirtDriverTests("},{"line_number":25,"context_line":"    base.ServersTestBase,"},{"line_number":26,"context_line":"    integrated_helpers.InstanceHelperMixin"},{"line_number":27,"context_line":"    ):"},{"line_number":28,"context_line":"    api_major_version \u003d \u0027v2.1\u0027"},{"line_number":29,"context_line":"    microversion \u003d \u0027latest\u0027"},{"line_number":30,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a51e69f_23166ea7","line":27,"range":{"start_line":24,"start_character":0,"end_line":27,"end_character":6},"updated":"2022-11-29 10:11:11.000000000","message":"better formatting would be:\n\nclass LibvirtDriverTests(\n    base.ServersTestBase,\n    integrated_helpers.InstanceHelperMixin\n):","commit_id":"eb1397642b3610a61cf0396c16553992245e286d"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"621b756986e6c079e2fb86cc010b0880888f816a","unresolved":true,"context_lines":[{"line_number":55,"context_line":"    def test_snapshot_quiesce_fail(self):"},{"line_number":56,"context_line":"        server \u003d self._create_server_with_block_device()"},{"line_number":57,"context_line":"        with mock.patch.object(nova_fixtures.libvirt.Domain,"},{"line_number":58,"context_line":"                \u0027fsFreeze\u0027) as mock_obj:"},{"line_number":59,"context_line":"            ex \u003d nova_fixtures.libvirt.libvirtError(\"Error\")"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"            mock_obj.side_effect \u003d ex"}],"source_content_type":"text/x-python","patch_set":3,"id":"1e8205c7_6aa2d858","line":58,"updated":"2022-11-29 10:11:11.000000000","message":"A better formatting would be:\n        with mock.patch.object(\n            nova_fixtures.libvirt.Domain, \"fsFreeze\"\n        ) as mock_obj:\n            ex \u003d nova_fixtures.libvirt.libvirtError(\"Error\")","commit_id":"eb1397642b3610a61cf0396c16553992245e286d"}]}