)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"abce670c86a8af66d4464a8b20441ad96f04ec54","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"cbe5913b_19df3897","updated":"2023-05-22 13:53:58.000000000","message":"I know this is WIP, but some comments anyway. Definitely appreciate the before/after two-phase reproducer approach like this, so thanks!","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"a14a06e41d1ee3108739ec8c6f26d668dba436a7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"f5789e9f_949ca861","updated":"2023-05-22 15:14:24.000000000","message":"thanks Dan for the detailed reviews. I will update the patch.\nAlso added a reply in few where I have doubts.","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"976bebc5e7e8cdbf0f44a25c1274b4a67abfeed1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"b45a78d9_54bd602b","updated":"2023-07-04 18:59:53.000000000","message":"recheck test_resize_servers_with_mlx5 test failed with db issue (unrelated)","commit_id":"8f5c8008e2c93e4796609b9f7d605d841555554c"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"14467f983e1aeabd976a8cf2fc6cac5e23606645","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":27,"id":"37bf8590_be55e886","updated":"2023-07-11 15:43:51.000000000","message":"recheck tempest admin test test_live_migration_with_trunk failed while waiting for port to become active, seems unrelated to cinder or volume attachments","commit_id":"361c1ccc8a733be1c2b99a7918db219b93d454b9"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"901a7329fa67c1de7ebb0144d1ae9c349d0d6606","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":27,"id":"e0bf751a_597d5d86","updated":"2023-07-11 09:20:27.000000000","message":"recheck tempest live migration test failed unrelated","commit_id":"361c1ccc8a733be1c2b99a7918db219b93d454b9"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"ccb0cf30c1c7efecafae40c28f918d95acdebfde","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":27,"id":"a624b677_0d4c6d8b","updated":"2023-07-12 10:47:49.000000000","message":"recheck tempest live migration test test_live_migration_with_trunk failing alot, its not related to this change. failing for other patches as well.","commit_id":"361c1ccc8a733be1c2b99a7918db219b93d454b9"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"3b1dddc1b693ca79536d32dbdf55b630a41a3f2a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":29,"id":"de46b94f_0263a7c0","updated":"2023-07-17 17:41:20.000000000","message":"recheck nova-multi-cell tempest test failed with SSH timeout (unrelated)","commit_id":"b20339369096657633c62e1222d063e0a5760a8f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f480f59037925dda06744c59fab10c2a58972225","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":30,"id":"4f34003c_116cdb38","updated":"2023-07-28 10:25:43.000000000","message":"I think I understand why you do it, but my concern is that IMHO you\u0027re adding more a lot of new modifications while they\u0027re needed for trying to reproduce the issue.","commit_id":"9c6138635107f3ba32e4d3da94738e38985d5fe9"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"a954dd892a97485e94dae3216f8ca48184808c01","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":30,"id":"95754a1b_1930c69e","updated":"2023-07-20 11:25:29.000000000","message":"recheck request timeout on wait_for_volume_attachment_remove_from_server passed locally","commit_id":"9c6138635107f3ba32e4d3da94738e38985d5fe9"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"62d491fb43d7dfc337c87bd549eaa297ccb87ea2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":33,"id":"48d66c2c_a613cd86","updated":"2023-08-22 05:14:05.000000000","message":"recheck SSHTimeout","commit_id":"13ff219821672746e5a805245361da8f78986068"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"d5dbb0e69500d0694419db8ae7efdfc0bf38c8de","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":33,"id":"e8f5e7b4_beb6602f","updated":"2023-08-23 07:24:54.000000000","message":"recheck SSHTimeout","commit_id":"13ff219821672746e5a805245361da8f78986068"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"250607a845f7f90cd380d61ea2a391a4f76ed20c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":33,"id":"136dd58d_7ce5b6a1","updated":"2023-07-31 11:36:48.000000000","message":"recheck tempest test failed with request timeout while create_network unrelated\nhttps://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_704/881457/33/check/grenade-skip-level-always/7043577/testr_results.html","commit_id":"13ff219821672746e5a805245361da8f78986068"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"c78cb1b688a7991befb30da2d178a9d83756b813","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":33,"id":"da30f79d_48c9fc24","updated":"2023-08-18 04:32:31.000000000","message":"recheck unrelated test failure","commit_id":"13ff219821672746e5a805245361da8f78986068"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d56349137f721787c126060daab43f0f06a43a94","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":34,"id":"b196a49a_6f319c0f","updated":"2023-08-25 16:50:10.000000000","message":"Thanks the fix looks good, but a bunch of extra junk is now in this patch?","commit_id":"00df387a99717f3eb148cddb62ddd009f51b5d03"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"903b3712a409630c27e8c9134271a9fad191df22","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":35,"id":"d09f4202_dbfb63bd","updated":"2023-08-29 14:34:13.000000000","message":"Again, as I said, I won\u0027t hold this series but I\u0027d appreciate if you could add a FUP on top of your series that would add the extra checks on the method calls that verify the output.","commit_id":"32ed205794e0676f9555b084b9774ab08a7d96ce"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5c99fd87e600fc4853ccb9210cbda401077b2b0a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":35,"id":"a62f9e3b_8e2f82eb","updated":"2023-08-31 16:47:38.000000000","message":"all patches are now approved so adding +w to the base patch","commit_id":"32ed205794e0676f9555b084b9774ab08a7d96ce"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"25b67181f4058cdfa007a2ca3f2b572b6e24bc07","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":35,"id":"8cd7987e_90700ba5","updated":"2023-08-30 17:36:56.000000000","message":"minor nit but this is fine i dont really have anything to add bar what sylvain already said.","commit_id":"32ed205794e0676f9555b084b9774ab08a7d96ce"}],"nova/compute/manager.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d56349137f721787c126060daab43f0f06a43a94","unresolved":true,"context_lines":[{"line_number":5636,"context_line":"        # in case _prep_resize fails."},{"line_number":5637,"context_line":"        instance_state \u003d instance.vm_state"},{"line_number":5638,"context_line":"        with self._error_out_instance_on_exception("},{"line_number":5639,"context_line":"                context, instance, instance_state\u003dinstance_state), \\"},{"line_number":5640,"context_line":"                errors_out_migration_ctxt(migration):"},{"line_number":5641,"context_line":""},{"line_number":5642,"context_line":"            self._send_prep_resize_notifications("}],"source_content_type":"text/x-python","patch_set":34,"id":"6c57bfd6_322f3e2d","line":5639,"updated":"2023-08-25 16:50:10.000000000","message":"This is unrelated to this patch right?","commit_id":"00df387a99717f3eb148cddb62ddd009f51b5d03"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"619f007ff429be6d15c35f23a49f48e57dca6ab6","unresolved":false,"context_lines":[{"line_number":5636,"context_line":"        # in case _prep_resize fails."},{"line_number":5637,"context_line":"        instance_state \u003d instance.vm_state"},{"line_number":5638,"context_line":"        with self._error_out_instance_on_exception("},{"line_number":5639,"context_line":"                context, instance, instance_state\u003dinstance_state), \\"},{"line_number":5640,"context_line":"                errors_out_migration_ctxt(migration):"},{"line_number":5641,"context_line":""},{"line_number":5642,"context_line":"            self._send_prep_resize_notifications("}],"source_content_type":"text/x-python","patch_set":34,"id":"5c4f62b8_e3d4bcf6","line":5639,"in_reply_to":"6c57bfd6_322f3e2d","updated":"2023-08-25 17:10:41.000000000","message":"reverted unrelated updates","commit_id":"00df387a99717f3eb148cddb62ddd009f51b5d03"}],"nova/tests/fixtures/cinder.py":[{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"0368725b05301bf27da4c8008be915d6db66e2c9","unresolved":true,"context_lines":[{"line_number":359,"context_line":"                    ) if instance_id \u003d\u003d attach[\u0027instance_uuid\u0027]])"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"            return _result"},{"line_number":362,"context_line":""},{"line_number":363,"context_line":"        def fake_get_all_volume_types(*args, **kwargs):"},{"line_number":364,"context_line":"            return [{"},{"line_number":365,"context_line":"                # This is used in the 2.67 API sample test."}],"source_content_type":"text/x-python","patch_set":17,"id":"c431e385_ea8e0a9c","line":362,"updated":"2023-06-08 14:20:40.000000000","message":"getting C901 from flake8\n\n`\u0027CinderFixture.setUp\u0027 is too complex (42)`\n\nI think its because of nested loop, any suggestions.\nor shall I ignore this with `# noqa: C901`","commit_id":"94db0e45c5f942549d85c6a8d6f3452072eb3a49"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f2d3bc2bb85cce1634be85c3ebd9582cacc9c54d","unresolved":true,"context_lines":[{"line_number":359,"context_line":"                    ) if instance_id \u003d\u003d attach[\u0027instance_uuid\u0027]])"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"            return _result"},{"line_number":362,"context_line":""},{"line_number":363,"context_line":"        def fake_get_all_volume_types(*args, **kwargs):"},{"line_number":364,"context_line":"            return [{"},{"line_number":365,"context_line":"                # This is used in the 2.67 API sample test."}],"source_content_type":"text/x-python","patch_set":17,"id":"cd410282_641a8924","line":362,"in_reply_to":"c431e385_ea8e0a9c","updated":"2023-06-08 16:06:15.000000000","message":"So we just discussed, but for posterity:\n\nPlease put a patch before this one that refactors this massive `setUp()` method to move the `fake_*` function definitions to the class. That will solve the complexity issue which you were just unlucky enough to trip over (not your fault).\n\nAlso if you could convert the legacy `stub_out` calls to mock patch fixtures, that would be great as well.","commit_id":"94db0e45c5f942549d85c6a8d6f3452072eb3a49"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"f33a5795a6d7d8f31513f268a50c701f5330958b","unresolved":false,"context_lines":[{"line_number":359,"context_line":"                    ) if instance_id \u003d\u003d attach[\u0027instance_uuid\u0027]])"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"            return _result"},{"line_number":362,"context_line":""},{"line_number":363,"context_line":"        def fake_get_all_volume_types(*args, **kwargs):"},{"line_number":364,"context_line":"            return [{"},{"line_number":365,"context_line":"                # This is used in the 2.67 API sample test."}],"source_content_type":"text/x-python","patch_set":17,"id":"f727528a_749948f1","line":362,"in_reply_to":"cd410282_641a8924","updated":"2023-06-12 13:57:01.000000000","message":"1 - Done\n2 - Also if you could convert the legacy stub_out calls to mock patch fixtures, that would be great as well - Inprogress","commit_id":"94db0e45c5f942549d85c6a8d6f3452072eb3a49"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f480f59037925dda06744c59fab10c2a58972225","unresolved":true,"context_lines":[{"line_number":451,"context_line":"        if self.attachment_error_id is not None:"},{"line_number":452,"context_line":"            attachment_id \u003d self.attachment_error_id"},{"line_number":453,"context_line":"        attachment \u003d {\u0027id\u0027: attachment_id}"},{"line_number":454,"context_line":"        # {vol_id:{att_id:{ id: att_id, instance_uuid: instance_uuid}}}"},{"line_number":455,"context_line":"        self.volume_to_attachment[volume_id][attachment_id] \u003d {"},{"line_number":456,"context_line":"            \u0027id\u0027: attachment_id,"},{"line_number":457,"context_line":"            \u0027instance_uuid\u0027: instance_id,"}],"source_content_type":"text/x-python","patch_set":30,"id":"cc2aa22b_bc81b5d8","line":454,"updated":"2023-07-28 10:25:43.000000000","message":"you shall remove this","commit_id":"9c6138635107f3ba32e4d3da94738e38985d5fe9"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"72eb3905d3e6b32ff1c42ba837c8d6011ff5a6ff","unresolved":false,"context_lines":[{"line_number":451,"context_line":"        if self.attachment_error_id is not None:"},{"line_number":452,"context_line":"            attachment_id \u003d self.attachment_error_id"},{"line_number":453,"context_line":"        attachment \u003d {\u0027id\u0027: attachment_id}"},{"line_number":454,"context_line":"        # {vol_id:{att_id:{ id: att_id, instance_uuid: instance_uuid}}}"},{"line_number":455,"context_line":"        self.volume_to_attachment[volume_id][attachment_id] \u003d {"},{"line_number":456,"context_line":"            \u0027id\u0027: attachment_id,"},{"line_number":457,"context_line":"            \u0027instance_uuid\u0027: instance_id,"}],"source_content_type":"text/x-python","patch_set":30,"id":"cf2f7ce9_4b4ee211","line":454,"in_reply_to":"cc2aa22b_bc81b5d8","updated":"2023-07-28 17:06:21.000000000","message":"Done","commit_id":"9c6138635107f3ba32e4d3da94738e38985d5fe9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9055f4036ed1de8545afad2f88bc73a9ff83c995","unresolved":true,"context_lines":[{"line_number":425,"context_line":"        for _, attachments in self.volume_to_attachment.items():"},{"line_number":426,"context_line":"            all_attachments.extend("},{"line_number":427,"context_line":"                [attach for attach in attachments.values("},{"line_number":428,"context_line":"                    ) if instance_id \u003d\u003d attach[\u0027instance_uuid\u0027]])"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"        return all_attachments"},{"line_number":431,"context_line":""}],"source_content_type":"text/x-python","patch_set":33,"id":"7fbebc69_a0961180","line":428,"range":{"start_line":428,"start_character":20,"end_line":428,"end_character":21},"updated":"2023-08-25 15:47:37.000000000","message":"Why did you split the closing paren onto the next line? That\u0027s very weird and confusing. It almost looks like this should close the `extend(` call, which it doesn\u0027t.\n\nIt\u0027s a common pattern to make comprehensions look like this:\n```\n[attach for attach in attachments.values()\n if instance_id \u003d\u003d attach[\u0027instance_uuid\u0027]]\n```\nThat should work and fit fine here.","commit_id":"13ff219821672746e5a805245361da8f78986068"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"d0b45d8531164a539f846b5e0bf4d9beb24e18c4","unresolved":false,"context_lines":[{"line_number":425,"context_line":"        for _, attachments in self.volume_to_attachment.items():"},{"line_number":426,"context_line":"            all_attachments.extend("},{"line_number":427,"context_line":"                [attach for attach in attachments.values("},{"line_number":428,"context_line":"                    ) if instance_id \u003d\u003d attach[\u0027instance_uuid\u0027]])"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"        return all_attachments"},{"line_number":431,"context_line":""}],"source_content_type":"text/x-python","patch_set":33,"id":"92c16545_a55e7e90","line":428,"range":{"start_line":428,"start_character":20,"end_line":428,"end_character":21},"in_reply_to":"7fbebc69_a0961180","updated":"2023-08-25 16:51:36.000000000","message":"Ack, Done, Thanks","commit_id":"13ff219821672746e5a805245361da8f78986068"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d56349137f721787c126060daab43f0f06a43a94","unresolved":false,"context_lines":[{"line_number":425,"context_line":"        for _, attachments in self.volume_to_attachment.items():"},{"line_number":426,"context_line":"            all_attachments.extend("},{"line_number":427,"context_line":"                [attach for attach in attachments.values("},{"line_number":428,"context_line":"                    ) if instance_id \u003d\u003d attach[\u0027instance_uuid\u0027]])"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"        return all_attachments"},{"line_number":431,"context_line":""}],"source_content_type":"text/x-python","patch_set":33,"id":"e37b1b91_c2103ed6","line":428,"range":{"start_line":428,"start_character":20,"end_line":428,"end_character":21},"in_reply_to":"7fbebc69_a0961180","updated":"2023-08-25 16:50:10.000000000","message":"Done","commit_id":"13ff219821672746e5a805245361da8f78986068"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"250607a845f7f90cd380d61ea2a391a4f76ed20c","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":33,"id":"37b2f2b2_0c2c6353","line":469,"updated":"2023-07-31 11:36:48.000000000","message":"`create_vol_attachment,\nget_vol_attachment\ndelete_vol_attachment`\nThe reason for adding the above here instead of reproducer is that API calls for these are already present in Cinder. API.","commit_id":"13ff219821672746e5a805245361da8f78986068"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"d0b45d8531164a539f846b5e0bf4d9beb24e18c4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":33,"id":"dfe493a0_4dc1250c","line":469,"in_reply_to":"37b2f2b2_0c2c6353","updated":"2023-08-25 16:51:36.000000000","message":"Done","commit_id":"13ff219821672746e5a805245361da8f78986068"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"25b67181f4058cdfa007a2ca3f2b572b6e24bc07","unresolved":true,"context_lines":[{"line_number":421,"context_line":"        if volume_id in self.volume_to_attachment:"},{"line_number":422,"context_line":"            return self.volume_to_attachment[volume_id]"},{"line_number":423,"context_line":""},{"line_number":424,"context_line":"        all_attachments \u003d []"},{"line_number":425,"context_line":"        for _, attachments in self.volume_to_attachment.items():"},{"line_number":426,"context_line":"            all_attachments.extend("},{"line_number":427,"context_line":"                [attach for attach in attachments.values()"},{"line_number":428,"context_line":"                 if instance_id \u003d\u003d attach[\u0027instance_uuid\u0027]])"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"        return all_attachments"},{"line_number":431,"context_line":""}],"source_content_type":"text/x-python","patch_set":35,"id":"a7df36b0_6626973c","line":428,"range":{"start_line":424,"start_character":0,"end_line":428,"end_character":60},"updated":"2023-08-30 17:36:56.000000000","message":"nit you shoudl prefer using list comprehsntion over calling exend in a loop.\n\nwhich would be somethign like this\n\n        all_attachments \u003d [\n        attach for _, attachments in self.volume_to_attachment.items()\n        for attach in attachments.values()\n        if instance_id \u003d\u003d attach[\u0027instance_uuid\u0027]\n        ]\n        \nthat said its almost identical to attachment_ids_for_instance\nbelow.\n\n\n    def attachment_ids_for_instance(self, instance_uuid):\n        attachment_ids \u003d []\n        for volume_id, attachments in self.volume_to_attachment.items():\n            for attachment in attachments.values():\n                if attachment[\u0027instance_uuid\u0027] \u003d\u003d instance_uuid:\n                    attachment_ids.append(attachment[\u0027id\u0027])\n        return attachment_ids\n        \n        \ni would probaly have added a attachments_for_instance  that just does\nthis and then reimplemted attachment_ids_for_instance as a filter over that but what you have is fine.\n\nthis could be don in a follwo up if we wanted too.","commit_id":"32ed205794e0676f9555b084b9774ab08a7d96ce"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"903b3712a409630c27e8c9134271a9fad191df22","unresolved":false,"context_lines":[{"line_number":427,"context_line":"                [attach for attach in attachments.values()"},{"line_number":428,"context_line":"                 if instance_id \u003d\u003d attach[\u0027instance_uuid\u0027]])"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"        return all_attachments"},{"line_number":431,"context_line":""},{"line_number":432,"context_line":"    def volume_ids_for_instance(self, instance_uuid):"},{"line_number":433,"context_line":"        for volume_id, attachments in self.volume_to_attachment.items():"}],"source_content_type":"text/x-python","patch_set":35,"id":"0d3cb14a_f3528935","line":430,"updated":"2023-08-29 14:34:13.000000000","message":"Just checked that this fake method behaves the same than the original method.\nTechnically, it also can return an exception if the client is unable to reach Cinder, but for the sake of this fixture, this is not needed to be faked here.","commit_id":"32ed205794e0676f9555b084b9774ab08a7d96ce"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"903b3712a409630c27e8c9134271a9fad191df22","unresolved":false,"context_lines":[{"line_number":446,"context_line":"                    attachment_ids.append(attachment[\u0027id\u0027])"},{"line_number":447,"context_line":"        return attachment_ids"},{"line_number":448,"context_line":""},{"line_number":449,"context_line":"    def create_vol_attachment(self, volume_id, instance_id):"},{"line_number":450,"context_line":"        attachment_id \u003d uuidutils.generate_uuid()"},{"line_number":451,"context_line":"        if self.attachment_error_id is not None:"},{"line_number":452,"context_line":"            attachment_id \u003d self.attachment_error_id"}],"source_content_type":"text/x-python","patch_set":35,"id":"654231ff_eabb2326","line":449,"updated":"2023-08-29 14:34:13.000000000","message":"in theory, all this below stub is unrelated to this patch and should be added on a different Gerrit change (probably where you need to test those methods), but I won\u0027t nitpick given the proximity of the FeatureFreeze.","commit_id":"32ed205794e0676f9555b084b9774ab08a7d96ce"}],"nova/tests/functional/compute/test_attached_volumes.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"abce670c86a8af66d4464a8b20441ad96f04ec54","unresolved":true,"context_lines":[{"line_number":19,"context_line":"# from nova.tests import fixtures as nova_fixtures"},{"line_number":20,"context_line":"# from unittest import mock"},{"line_number":21,"context_line":"# from nova.tests.functional.api import client"},{"line_number":22,"context_line":"# from oslo_concurrency import processutils"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class TestAttachedVolumes(integrated_helpers._IntegratedTestBase):"}],"source_content_type":"text/x-python","patch_set":6,"id":"cfcbad6e_d895c4fd","line":22,"updated":"2023-05-22 13:53:58.000000000","message":"I (very much) appreciate the before/after reproducer, but I think it would be cleaner to comment out just the assertion code you plan to enable after the fix. It\u0027s understandable that imports may need to change when you do that. So I would remove these, and the mock at L47 and add that in for the second patch.","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ec22b1cce692636eeed4c6862fe7f6bc9584df46","unresolved":true,"context_lines":[{"line_number":19,"context_line":"# from nova.tests import fixtures as nova_fixtures"},{"line_number":20,"context_line":"# from unittest import mock"},{"line_number":21,"context_line":"# from nova.tests.functional.api import client"},{"line_number":22,"context_line":"# from oslo_concurrency import processutils"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class TestAttachedVolumes(integrated_helpers._IntegratedTestBase):"}],"source_content_type":"text/x-python","patch_set":6,"id":"5cecb764_0b0bcdc9","line":22,"in_reply_to":"02cca64d_b010bdb7","updated":"2023-05-22 15:54:40.000000000","message":"Ack, so they should be removed before we merge.","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"872a48a6879aaf7b6f941bc1cab248ad778a65ab","unresolved":false,"context_lines":[{"line_number":19,"context_line":"# from nova.tests import fixtures as nova_fixtures"},{"line_number":20,"context_line":"# from unittest import mock"},{"line_number":21,"context_line":"# from nova.tests.functional.api import client"},{"line_number":22,"context_line":"# from oslo_concurrency import processutils"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class TestAttachedVolumes(integrated_helpers._IntegratedTestBase):"}],"source_content_type":"text/x-python","patch_set":6,"id":"a71056f1_5e0a8367","line":22,"in_reply_to":"5cecb764_0b0bcdc9","updated":"2023-05-23 10:58:20.000000000","message":"yes, before merge, all not required, commented imports will be removed\nDone","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"a14a06e41d1ee3108739ec8c6f26d668dba436a7","unresolved":true,"context_lines":[{"line_number":19,"context_line":"# from nova.tests import fixtures as nova_fixtures"},{"line_number":20,"context_line":"# from unittest import mock"},{"line_number":21,"context_line":"# from nova.tests.functional.api import client"},{"line_number":22,"context_line":"# from oslo_concurrency import processutils"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class TestAttachedVolumes(integrated_helpers._IntegratedTestBase):"}],"source_content_type":"text/x-python","patch_set":6,"id":"02cca64d_b010bdb7","line":22,"in_reply_to":"cfcbad6e_d895c4fd","updated":"2023-05-22 15:14:24.000000000","message":"I am trying to understand and fix, why instance is not going to ERROR state after reboot. so I tried different things which did not work, but I think should work, and left them commented to get reviews","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"abce670c86a8af66d4464a8b20441ad96f04ec54","unresolved":true,"context_lines":[{"line_number":29,"context_line":"    def setUp(self):"},{"line_number":30,"context_line":"        super(TestAttachedVolumes, self).setUp()"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        # self.useFixture(nova_fixtures.CinderFixture(self))"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"        self.admin_api \u003d self.api_fixture.admin_api"},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"4d6c8c3b_1edcbf4c","line":32,"updated":"2023-05-22 13:53:58.000000000","message":"Why is this commented out? The cinder-side behavior should be the same before and after the fix. It also looks like below that cinder is already mocked.","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"872a48a6879aaf7b6f941bc1cab248ad778a65ab","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    def setUp(self):"},{"line_number":30,"context_line":"        super(TestAttachedVolumes, self).setUp()"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        # self.useFixture(nova_fixtures.CinderFixture(self))"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"        self.admin_api \u003d self.api_fixture.admin_api"},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9b115325_c6cde0c3","line":32,"in_reply_to":"373102ae_39bd72e7","updated":"2023-05-23 10:58:20.000000000","message":"Done","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"a14a06e41d1ee3108739ec8c6f26d668dba436a7","unresolved":true,"context_lines":[{"line_number":29,"context_line":"    def setUp(self):"},{"line_number":30,"context_line":"        super(TestAttachedVolumes, self).setUp()"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        # self.useFixture(nova_fixtures.CinderFixture(self))"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"        self.admin_api \u003d self.api_fixture.admin_api"},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"4da0eadd_95bbe5fe","line":32,"in_reply_to":"4d6c8c3b_1edcbf4c","updated":"2023-05-22 15:14:24.000000000","message":"Without adding fixture also I am able use  CinderAPI.\nso, did not remove it and left it for comments.","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ec22b1cce692636eeed4c6862fe7f6bc9584df46","unresolved":true,"context_lines":[{"line_number":29,"context_line":"    def setUp(self):"},{"line_number":30,"context_line":"        super(TestAttachedVolumes, self).setUp()"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        # self.useFixture(nova_fixtures.CinderFixture(self))"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"        self.admin_api \u003d self.api_fixture.admin_api"},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"373102ae_39bd72e7","line":32,"in_reply_to":"4da0eadd_95bbe5fe","updated":"2023-05-22 15:54:40.000000000","message":"There\u0027s no reason to have it commented, so just remove it.","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"abce670c86a8af66d4464a8b20441ad96f04ec54","unresolved":true,"context_lines":[{"line_number":78,"context_line":"        # TODO(auniyal): verify from DB"},{"line_number":79,"context_line":"        # or test by reboot, so reboot should have made this vm to error"},{"line_number":80,"context_line":"        # didn\u0027t happend, need to mock it!!"},{"line_number":81,"context_line":"        # self.assertRaises(Exception, self._reboot_server("},{"line_number":82,"context_line":"        # server, hard\u003dTrue))"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        server \u003d self._reboot_server(server, hard\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":6,"id":"82fb7a6f_49fb89fd","line":81,"range":{"start_line":81,"start_character":28,"end_line":81,"end_character":37},"updated":"2023-05-22 13:53:58.000000000","message":"Please check for an actual exception. You should never assertRaises(Exception, ...) because even a typo in your code will pass the test.","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"4a5a970aff3fce37e08a0dc5a4f93db5a3ace7fd","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        # TODO(auniyal): verify from DB"},{"line_number":79,"context_line":"        # or test by reboot, so reboot should have made this vm to error"},{"line_number":80,"context_line":"        # didn\u0027t happend, need to mock it!!"},{"line_number":81,"context_line":"        # self.assertRaises(Exception, self._reboot_server("},{"line_number":82,"context_line":"        # server, hard\u003dTrue))"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        server \u003d self._reboot_server(server, hard\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":6,"id":"07112f44_9cf413b0","line":81,"range":{"start_line":81,"start_character":28,"end_line":81,"end_character":37},"in_reply_to":"2bda6c83_b30b9d71","updated":"2023-05-31 15:01:59.000000000","message":"Ack","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"a14a06e41d1ee3108739ec8c6f26d668dba436a7","unresolved":true,"context_lines":[{"line_number":78,"context_line":"        # TODO(auniyal): verify from DB"},{"line_number":79,"context_line":"        # or test by reboot, so reboot should have made this vm to error"},{"line_number":80,"context_line":"        # didn\u0027t happend, need to mock it!!"},{"line_number":81,"context_line":"        # self.assertRaises(Exception, self._reboot_server("},{"line_number":82,"context_line":"        # server, hard\u003dTrue))"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        server \u003d self._reboot_server(server, hard\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":6,"id":"f4e36607_b21e3231","line":81,"range":{"start_line":81,"start_character":28,"end_line":81,"end_character":37},"in_reply_to":"82fb7a6f_49fb89fd","updated":"2023-05-22 15:14:24.000000000","message":"so this is still pendig, I am confused on this part, reboot should have raised the exception or server should go to ERROR state, but its not going right now.\n\nlooking for suggestion here","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ec22b1cce692636eeed4c6862fe7f6bc9584df46","unresolved":true,"context_lines":[{"line_number":78,"context_line":"        # TODO(auniyal): verify from DB"},{"line_number":79,"context_line":"        # or test by reboot, so reboot should have made this vm to error"},{"line_number":80,"context_line":"        # didn\u0027t happend, need to mock it!!"},{"line_number":81,"context_line":"        # self.assertRaises(Exception, self._reboot_server("},{"line_number":82,"context_line":"        # server, hard\u003dTrue))"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        server \u003d self._reboot_server(server, hard\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":6,"id":"2bda6c83_b30b9d71","line":81,"range":{"start_line":81,"start_character":28,"end_line":81,"end_character":37},"in_reply_to":"f4e36607_b21e3231","updated":"2023-05-22 15:54:40.000000000","message":"In reality, the API call will not return an error because reboot is a cast, meaning the API will return before the reboot actually happens. However, this should be using CastAsCall in the base class, which makes everything synchronous. However, if you\u0027re never putting into error state, it\u0027s probably just passing straight through.","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"85cdd969ed21e511b7e32fdd02d1580273ce3743","unresolved":true,"context_lines":[{"line_number":81,"context_line":"        # self.assertRaises(Exception, self._reboot_server("},{"line_number":82,"context_line":"        # server, hard\u003dTrue))"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        server \u003d self._reboot_server(server, hard\u003dTrue)"},{"line_number":85,"context_line":"        # server is going to ACTIVE state after reboot"},{"line_number":86,"context_line":"        # import rpdb;rpdb.set_trace()"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"4246a554_316c5e6b","line":84,"updated":"2023-05-22 13:42:44.000000000","message":"this reboot do call compute.managers.reboot from https://github.com/openstack/nova/blob/master/nova/virt/fake.py#L229\n \nreboot should through error.","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ec22b1cce692636eeed4c6862fe7f6bc9584df46","unresolved":true,"context_lines":[{"line_number":81,"context_line":"        # self.assertRaises(Exception, self._reboot_server("},{"line_number":82,"context_line":"        # server, hard\u003dTrue))"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        server \u003d self._reboot_server(server, hard\u003dTrue)"},{"line_number":85,"context_line":"        # server is going to ACTIVE state after reboot"},{"line_number":86,"context_line":"        # import rpdb;rpdb.set_trace()"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"fa182447_ad16e8da","line":84,"in_reply_to":"0767eb3a_4ff20e73","updated":"2023-05-22 15:54:40.000000000","message":"This should be independent of the virt driver, AFAIK, since the code is now (properly) in compute manager. But Amit, compute manager is not called _from_ the driver -- the opposite. Compute manager calls the driver.","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5eccdab134af228b3e65ef989506068f45a20e0a","unresolved":true,"context_lines":[{"line_number":81,"context_line":"        # self.assertRaises(Exception, self._reboot_server("},{"line_number":82,"context_line":"        # server, hard\u003dTrue))"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        server \u003d self._reboot_server(server, hard\u003dTrue)"},{"line_number":85,"context_line":"        # server is going to ACTIVE state after reboot"},{"line_number":86,"context_line":"        # import rpdb;rpdb.set_trace()"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"0767eb3a_4ff20e73","line":84,"in_reply_to":"4246a554_316c5e6b","updated":"2023-05-22 14:58:06.000000000","message":"you need to use the right libvirt driver, not the fake one (or make sure to provide an exception)","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"872a48a6879aaf7b6f941bc1cab248ad778a65ab","unresolved":true,"context_lines":[{"line_number":81,"context_line":"        # self.assertRaises(Exception, self._reboot_server("},{"line_number":82,"context_line":"        # server, hard\u003dTrue))"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        server \u003d self._reboot_server(server, hard\u003dTrue)"},{"line_number":85,"context_line":"        # server is going to ACTIVE state after reboot"},{"line_number":86,"context_line":"        # import rpdb;rpdb.set_trace()"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"fd82cd50_426232c7","line":84,"in_reply_to":"fa182447_ad16e8da","updated":"2023-05-23 10:58:20.000000000","message":"Ack","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"4a5a970aff3fce37e08a0dc5a4f93db5a3ace7fd","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        # self.assertRaises(Exception, self._reboot_server("},{"line_number":82,"context_line":"        # server, hard\u003dTrue))"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        server \u003d self._reboot_server(server, hard\u003dTrue)"},{"line_number":85,"context_line":"        # server is going to ACTIVE state after reboot"},{"line_number":86,"context_line":"        # import rpdb;rpdb.set_trace()"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"964a45b0_cfe1cb0b","line":84,"in_reply_to":"fd82cd50_426232c7","updated":"2023-05-31 15:01:59.000000000","message":"Done","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"abce670c86a8af66d4464a8b20441ad96f04ec54","unresolved":true,"context_lines":[{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        server \u003d self._reboot_server(server, hard\u003dTrue)"},{"line_number":85,"context_line":"        # server is going to ACTIVE state after reboot"},{"line_number":86,"context_line":"        # import rpdb;rpdb.set_trace()"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":89,"context_line":"            ctxt, server[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"55ba4ceb_40804986","line":86,"updated":"2023-05-22 13:53:58.000000000","message":"Leftover debug to remove?","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"a14a06e41d1ee3108739ec8c6f26d668dba436a7","unresolved":false,"context_lines":[{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        server \u003d self._reboot_server(server, hard\u003dTrue)"},{"line_number":85,"context_line":"        # server is going to ACTIVE state after reboot"},{"line_number":86,"context_line":"        # import rpdb;rpdb.set_trace()"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":89,"context_line":"            ctxt, server[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"5c360eb6_118be8b2","line":86,"in_reply_to":"55ba4ceb_40804986","updated":"2023-05-22 15:14:24.000000000","message":"Ack","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"abce670c86a8af66d4464a8b20441ad96f04ec54","unresolved":true,"context_lines":[{"line_number":88,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":89,"context_line":"            ctxt, server[\u0027id\u0027])"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        self.assertEqual(volume_id, bdms[1].volume_id)"}],"source_content_type":"text/x-python","patch_set":6,"id":"cc050228_aebf4095","line":91,"updated":"2023-05-22 13:53:58.000000000","message":"I think there should be a second test here (it can build on the first) that does the test with two or three volumes attached, but where only one is *dangling*. Make sure that we do not purge the active ones and do purge the dangling one.","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5eccdab134af228b3e65ef989506068f45a20e0a","unresolved":true,"context_lines":[{"line_number":88,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":89,"context_line":"            ctxt, server[\u0027id\u0027])"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        self.assertEqual(volume_id, bdms[1].volume_id)"}],"source_content_type":"text/x-python","patch_set":6,"id":"aebe6ec9_7f4a677f","line":91,"in_reply_to":"cc050228_aebf4095","updated":"2023-05-22 14:58:06.000000000","message":"+1","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"a14a06e41d1ee3108739ec8c6f26d668dba436a7","unresolved":true,"context_lines":[{"line_number":88,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":89,"context_line":"            ctxt, server[\u0027id\u0027])"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        self.assertEqual(volume_id, bdms[1].volume_id)"}],"source_content_type":"text/x-python","patch_set":6,"id":"dc393443_c48b2fa2","line":91,"in_reply_to":"cc050228_aebf4095","updated":"2023-05-22 15:14:24.000000000","message":"Ack, will add, thanks","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"4a5a970aff3fce37e08a0dc5a4f93db5a3ace7fd","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":89,"context_line":"            ctxt, server[\u0027id\u0027])"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        self.assertEqual(volume_id, bdms[1].volume_id)"}],"source_content_type":"text/x-python","patch_set":6,"id":"ab514158_42fbb2dc","line":91,"in_reply_to":"dc393443_c48b2fa2","updated":"2023-05-31 15:01:59.000000000","message":"Done","commit_id":"04bf44a49745842513441d3ca4d78af115c59543"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2d6237243a66f78768475d3e529617e1bca91ab4","unresolved":true,"context_lines":[{"line_number":146,"context_line":"        # self.assertNotIn(volumes[1], bdms_list)"},{"line_number":147,"context_line":"        # self.assertIn(volumes[2], bdms_list)"},{"line_number":148,"context_line":"        # self.asserNotIn(volumes[3], bdms_list)"},{"line_number":149,"context_line":"        # self.assertEqual(2, len(bdms_list))"}],"source_content_type":"text/x-python","patch_set":9,"id":"3de6a793_cd5fbe82","line":149,"updated":"2023-05-24 17:19:13.000000000","message":"We also want a test case for an instance and a volume that were created directly in cinder that nova does not know about. We should not import that attachment into nova and it should not fool us that a stale BDM needs to be cleaned up.\n\nIt\u0027s arguable that we should delete any attachments we find for the volume in cinder that we don\u0027t have BDMs for. I\u0027m not sure my opinion on that, but we should maybe discuss it.","commit_id":"55ecd414ddda0137cf18ad4f4f526f158d5b06b9"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"f33a5795a6d7d8f31513f268a50c701f5330958b","unresolved":false,"context_lines":[{"line_number":146,"context_line":"        # self.assertNotIn(volumes[1], bdms_list)"},{"line_number":147,"context_line":"        # self.assertIn(volumes[2], bdms_list)"},{"line_number":148,"context_line":"        # self.asserNotIn(volumes[3], bdms_list)"},{"line_number":149,"context_line":"        # self.assertEqual(2, len(bdms_list))"}],"source_content_type":"text/x-python","patch_set":9,"id":"fcf9bf57_98118397","line":149,"in_reply_to":"05673099_7635bb94","updated":"2023-06-12 13:57:01.000000000","message":"Done","commit_id":"55ecd414ddda0137cf18ad4f4f526f158d5b06b9"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7022bcf2b22935158b2c18b125947100054a0a9c","unresolved":true,"context_lines":[{"line_number":146,"context_line":"        # self.assertNotIn(volumes[1], bdms_list)"},{"line_number":147,"context_line":"        # self.assertIn(volumes[2], bdms_list)"},{"line_number":148,"context_line":"        # self.asserNotIn(volumes[3], bdms_list)"},{"line_number":149,"context_line":"        # self.assertEqual(2, len(bdms_list))"}],"source_content_type":"text/x-python","patch_set":9,"id":"85d00d27_e2e3f3ef","line":149,"in_reply_to":"3de6a793_cd5fbe82","updated":"2023-05-24 18:15:40.000000000","message":"there are two was to look at that i guess.\n\nyou could as an admin use the exiting nova-manage commaand to recreate the attachemnts in cidner for anythien we know about.\n\n\nbut if we are going to do this on hard reboot then yes i would tend to agree , as an end user i would expect nova  to align the world view with its internal view.\n\nwe shoudl not be importing any volueme attaments form cinder on reboot and we shoud be removing any we do not know about.\n\nthat renforces the fact that we only allow managing volume attachments on an instance via the nova api.","commit_id":"55ecd414ddda0137cf18ad4f4f526f158d5b06b9"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"358c1e02442711e2fd65b2ae6f7fd2eab19953a6","unresolved":true,"context_lines":[{"line_number":146,"context_line":"        # self.assertNotIn(volumes[1], bdms_list)"},{"line_number":147,"context_line":"        # self.assertIn(volumes[2], bdms_list)"},{"line_number":148,"context_line":"        # self.asserNotIn(volumes[3], bdms_list)"},{"line_number":149,"context_line":"        # self.assertEqual(2, len(bdms_list))"}],"source_content_type":"text/x-python","patch_set":9,"id":"d3bf5064_9c6b1121","line":149,"in_reply_to":"40894fc1_6b9acce8","updated":"2023-06-05 12:24:43.000000000","message":"ACK, thanks.\n\n`openstack volume attachment create bl_1 vm_1 --os-volume-api-version 3.27`\ndoes update cinder.volume_attachment table but there is no update on Nova side\n\n\nList all attachments\nclient:\n`openstack volume attachment list --volume-id 05fb63be-c513-440a-95d5-c95751640005 --os-volume-api-version 3.44`\n\ncurl\n`curl -X GET http://10.0.110.64/volume/v3/2988fb3586d34965ada459a1ce736a92/attachments?volume_id\u003d05fb63be-c513-440a-95d5-c95751640005 -H \"X-Auth-Token: $TT\" -H \"OpenStack-API-Version: volume 3.44\" |  jq \u0027.\u0027`","commit_id":"55ecd414ddda0137cf18ad4f4f526f158d5b06b9"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"4a5a970aff3fce37e08a0dc5a4f93db5a3ace7fd","unresolved":true,"context_lines":[{"line_number":146,"context_line":"        # self.assertNotIn(volumes[1], bdms_list)"},{"line_number":147,"context_line":"        # self.assertIn(volumes[2], bdms_list)"},{"line_number":148,"context_line":"        # self.asserNotIn(volumes[3], bdms_list)"},{"line_number":149,"context_line":"        # self.assertEqual(2, len(bdms_list))"}],"source_content_type":"text/x-python","patch_set":9,"id":"d5cfcf22_8454a0ce","line":149,"in_reply_to":"85d00d27_e2e3f3ef","updated":"2023-05-31 15:01:59.000000000","message":"`We should not import that attachment into nova and it should not fool us that a stale BDM needs to be cleaned up`\n\nDan, I could not understand this fully (must be missing something), do you mean below:\n\n- create a volume; do not attach volume to any vm\n- get list of volumes from cinder\n- check if volumes are present in nova.bdm, they should not.\n\nbut as we(Nova) never attached the volume to VM, is it possible for other services to attach too ?\n\nhere in this scenario, from nova view, we are checking if instance has a stale BDM and deleting it only and not creating attachment (so no import to nova).\n\nSean mentioned, we can use nova-manage command to create/recreate attachment, in that case, nova will know about attachment and should do appropriate verification and update the DB.","commit_id":"55ecd414ddda0137cf18ad4f4f526f158d5b06b9"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"0368725b05301bf27da4c8008be915d6db66e2c9","unresolved":true,"context_lines":[{"line_number":146,"context_line":"        # self.assertNotIn(volumes[1], bdms_list)"},{"line_number":147,"context_line":"        # self.assertIn(volumes[2], bdms_list)"},{"line_number":148,"context_line":"        # self.asserNotIn(volumes[3], bdms_list)"},{"line_number":149,"context_line":"        # self.assertEqual(2, len(bdms_list))"}],"source_content_type":"text/x-python","patch_set":9,"id":"05673099_7635bb94","line":149,"in_reply_to":"d3bf5064_9c6b1121","updated":"2023-06-08 14:20:40.000000000","message":"Added test for same","commit_id":"55ecd414ddda0137cf18ad4f4f526f158d5b06b9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bb70238577dd01252b1548c456fab98e9dd5dd49","unresolved":true,"context_lines":[{"line_number":146,"context_line":"        # self.assertNotIn(volumes[1], bdms_list)"},{"line_number":147,"context_line":"        # self.assertIn(volumes[2], bdms_list)"},{"line_number":148,"context_line":"        # self.asserNotIn(volumes[3], bdms_list)"},{"line_number":149,"context_line":"        # self.assertEqual(2, len(bdms_list))"}],"source_content_type":"text/x-python","patch_set":9,"id":"40894fc1_6b9acce8","line":149,"in_reply_to":"d5cfcf22_8454a0ce","updated":"2023-05-31 15:09:31.000000000","message":"I\u0027m talking about the case where the following happens:\n\n1. Create a server\n2. Create a volume\n3. Create a volume attachment in cinder with instance_uuid set but that nova does not know about\n\nIn this case, we should delete that attachment in cinder because we do not know about it and it did not go through the nova attach API.","commit_id":"55ecd414ddda0137cf18ad4f4f526f158d5b06b9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a829fdf1033e3a8b75b0bcd614be3be6f218ca36","unresolved":true,"context_lines":[{"line_number":27,"context_line":"    def setUp(self):"},{"line_number":28,"context_line":"        super(TestAttachedVolumes, self).setUp()"},{"line_number":29,"context_line":"        self.admin_api \u003d self.api_fixture.admin_api"},{"line_number":30,"context_line":"        self.volume_api \u003d cinder.API()"},{"line_number":31,"context_line":"        self.start_compute()"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def _attach_volumes(self, server, vol_ids\u003dlist):"}],"source_content_type":"text/x-python","patch_set":11,"id":"104286ff_f9b71ae7","line":30,"updated":"2023-06-05 12:36:22.000000000","message":"You cannot use a real cinder client in the functional test as you don\u0027t have a running cinder service. You have to use the fixture https://github.com/openstack/nova/blob/master/nova/tests/fixtures/cinder.py instead.\n\nYour test base calls already includes the cinder fixture here: https://github.com/openstack/nova/blob/971809b4d4db39e98d6534531e55d5b8fcde49cb/nova/tests/functional/integrated_helpers.py#L1229","commit_id":"169e46313f56afd6b97f1be723b980b483750d48"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"0368725b05301bf27da4c8008be915d6db66e2c9","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    def setUp(self):"},{"line_number":28,"context_line":"        super(TestAttachedVolumes, self).setUp()"},{"line_number":29,"context_line":"        self.admin_api \u003d self.api_fixture.admin_api"},{"line_number":30,"context_line":"        self.volume_api \u003d cinder.API()"},{"line_number":31,"context_line":"        self.start_compute()"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def _attach_volumes(self, server, vol_ids\u003dlist):"}],"source_content_type":"text/x-python","patch_set":11,"id":"985b44e1_d97b2de2","line":30,"in_reply_to":"104286ff_f9b71ae7","updated":"2023-06-08 14:20:40.000000000","message":"Ack, Updating CinderFixture fixed the issue. thanks","commit_id":"169e46313f56afd6b97f1be723b980b483750d48"}],"nova/tests/functional/regressions/test_bug_1835822.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f480f59037925dda06744c59fab10c2a58972225","unresolved":true,"context_lines":[{"line_number":27,"context_line":"        self.useFixture(nova_fixtures.GlanceFixture(self))"},{"line_number":28,"context_line":"        self.useFixture(nova_fixtures.NeutronFixture(self))"},{"line_number":29,"context_line":"        self.useFixture(func_fixtures.PlacementFixture())"},{"line_number":30,"context_line":"        self.useFixture(nova_fixtures.CinderFixture(self))"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        self.api \u003d self.useFixture(nova_fixtures.OSAPIFixture("},{"line_number":33,"context_line":"            api_version\u003d\u0027v2.1\u0027)).api"}],"source_content_type":"text/x-python","patch_set":30,"id":"06a83524_720c5465","line":30,"updated":"2023-07-28 10:25:43.000000000","message":"why do you need to add this here ?","commit_id":"9c6138635107f3ba32e4d3da94738e38985d5fe9"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"72eb3905d3e6b32ff1c42ba837c8d6011ff5a6ff","unresolved":false,"context_lines":[{"line_number":27,"context_line":"        self.useFixture(nova_fixtures.GlanceFixture(self))"},{"line_number":28,"context_line":"        self.useFixture(nova_fixtures.NeutronFixture(self))"},{"line_number":29,"context_line":"        self.useFixture(func_fixtures.PlacementFixture())"},{"line_number":30,"context_line":"        self.useFixture(nova_fixtures.CinderFixture(self))"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        self.api \u003d self.useFixture(nova_fixtures.OSAPIFixture("},{"line_number":33,"context_line":"            api_version\u003d\u0027v2.1\u0027)).api"}],"source_content_type":"text/x-python","patch_set":30,"id":"2143abb0_8945975a","line":30,"in_reply_to":"06a83524_720c5465","updated":"2023-07-28 17:06:21.000000000","message":"Done","commit_id":"9c6138635107f3ba32e4d3da94738e38985d5fe9"}],"nova/tests/functional/test_instance_actions.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f480f59037925dda06744c59fab10c2a58972225","unresolved":true,"context_lines":[{"line_number":85,"context_line":"        self.useFixture(nova_fixtures.NeutronFixture(self))"},{"line_number":86,"context_line":"        self.useFixture(func_fixtures.PlacementFixture())"},{"line_number":87,"context_line":"        self.useFixture(nova_fixtures.RealPolicyFixture())"},{"line_number":88,"context_line":"        self.useFixture(nova_fixtures.CinderFixture(self))"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"        # Start the compute services."},{"line_number":91,"context_line":"        self.start_service(\u0027conductor\u0027)"}],"source_content_type":"text/x-python","patch_set":30,"id":"f08dea23_6b96366e","line":88,"updated":"2023-07-28 10:25:43.000000000","message":"ditto, I wonder why you need it here.","commit_id":"9c6138635107f3ba32e4d3da94738e38985d5fe9"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"72eb3905d3e6b32ff1c42ba837c8d6011ff5a6ff","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        self.useFixture(nova_fixtures.NeutronFixture(self))"},{"line_number":86,"context_line":"        self.useFixture(func_fixtures.PlacementFixture())"},{"line_number":87,"context_line":"        self.useFixture(nova_fixtures.RealPolicyFixture())"},{"line_number":88,"context_line":"        self.useFixture(nova_fixtures.CinderFixture(self))"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"        # Start the compute services."},{"line_number":91,"context_line":"        self.start_service(\u0027conductor\u0027)"}],"source_content_type":"text/x-python","patch_set":30,"id":"e04c1213_8ba9acc6","line":88,"in_reply_to":"f08dea23_6b96366e","updated":"2023-07-28 17:06:21.000000000","message":"Done","commit_id":"9c6138635107f3ba32e4d3da94738e38985d5fe9"}],"nova/tests/functional/test_server_faults.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f480f59037925dda06744c59fab10c2a58972225","unresolved":true,"context_lines":[{"line_number":33,"context_line":"        self.useFixture(func_fixtures.PlacementFixture())"},{"line_number":34,"context_line":"        self.useFixture(nova_fixtures.GlanceFixture(self))"},{"line_number":35,"context_line":"        self.useFixture(nova_fixtures.RealPolicyFixture())"},{"line_number":36,"context_line":"        self.useFixture(nova_fixtures.CinderFixture(self))"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"        # Start the compute services."},{"line_number":39,"context_line":"        self.start_service(\u0027conductor\u0027)"}],"source_content_type":"text/x-python","patch_set":30,"id":"6a9d860a_4954e4c5","line":36,"updated":"2023-07-28 10:25:43.000000000","message":"ditto","commit_id":"9c6138635107f3ba32e4d3da94738e38985d5fe9"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"72eb3905d3e6b32ff1c42ba837c8d6011ff5a6ff","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        self.useFixture(func_fixtures.PlacementFixture())"},{"line_number":34,"context_line":"        self.useFixture(nova_fixtures.GlanceFixture(self))"},{"line_number":35,"context_line":"        self.useFixture(nova_fixtures.RealPolicyFixture())"},{"line_number":36,"context_line":"        self.useFixture(nova_fixtures.CinderFixture(self))"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"        # Start the compute services."},{"line_number":39,"context_line":"        self.start_service(\u0027conductor\u0027)"}],"source_content_type":"text/x-python","patch_set":30,"id":"8a4a5615_0d8ddae4","line":36,"in_reply_to":"6a9d860a_4954e4c5","updated":"2023-07-28 17:06:21.000000000","message":"Done","commit_id":"9c6138635107f3ba32e4d3da94738e38985d5fe9"}],"nova/tests/unit/api/openstack/compute/test_server_topology.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d56349137f721787c126060daab43f0f06a43a94","unresolved":true,"context_lines":[{"line_number":73,"context_line":"                     \u0027siblings\u0027: [],"},{"line_number":74,"context_line":"                     \u0027vcpu_set\u0027: set([0, 1]),"},{"line_number":75,"context_line":"                     \u0027host_node\u0027: 0,"},{"line_number":76,"context_line":"                     \u0027cpu_pinning\u0027: {}}],"},{"line_number":77,"context_line":"                     \u0027pagesize_kb\u0027: 4}"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        inst \u003d objects.instance.Instance(uuid\u003dself.uuid, host\u003d\u0027123\u0027,"}],"source_content_type":"text/x-python","patch_set":34,"id":"bfde1302_85e7d925","line":76,"updated":"2023-08-25 16:50:10.000000000","message":"Also unrelated?","commit_id":"00df387a99717f3eb148cddb62ddd009f51b5d03"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"619f007ff429be6d15c35f23a49f48e57dca6ab6","unresolved":false,"context_lines":[{"line_number":73,"context_line":"                     \u0027siblings\u0027: [],"},{"line_number":74,"context_line":"                     \u0027vcpu_set\u0027: set([0, 1]),"},{"line_number":75,"context_line":"                     \u0027host_node\u0027: 0,"},{"line_number":76,"context_line":"                     \u0027cpu_pinning\u0027: {}}],"},{"line_number":77,"context_line":"                     \u0027pagesize_kb\u0027: 4}"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        inst \u003d objects.instance.Instance(uuid\u003dself.uuid, host\u003d\u0027123\u0027,"}],"source_content_type":"text/x-python","patch_set":34,"id":"0d4c6e51_3da135b0","line":76,"in_reply_to":"bfde1302_85e7d925","updated":"2023-08-25 17:10:41.000000000","message":"Done","commit_id":"00df387a99717f3eb148cddb62ddd009f51b5d03"}],"nova/tests/unit/volume/test_cinder.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"903b3712a409630c27e8c9134271a9fad191df22","unresolved":true,"context_lines":[{"line_number":760,"context_line":"        self.api.attachment_get_all(self.ctx, instance_id)"},{"line_number":761,"context_line":"        mock_cinderclient.assert_called_once_with(self.ctx, \u00273.44\u0027,"},{"line_number":762,"context_line":"                                                  skip_version_check\u003dTrue)"},{"line_number":763,"context_line":"        mock_attachment.list.assert_called_once_with(search_opts\u003dsearch_opts)"},{"line_number":764,"context_line":""},{"line_number":765,"context_line":"    @mock.patch(\u0027nova.volume.cinder.cinderclient\u0027)"},{"line_number":766,"context_line":"    def test_attachment_get_all_by_volume(self, mock_cinderclient):"}],"source_content_type":"text/x-python","patch_set":35,"id":"88f39abc_7af99a6c","line":763,"updated":"2023-08-29 14:34:13.000000000","message":"that\u0027s well written, but your unittest only checks the mocks (the calls) and not the returned objects. \nIn theory, your mock object (the MagicMock object L756) should propose a specific list of attachments so you could check that the returned object of attachment_get_all_by_instance() is actually an expected object which is a list of dicts.\n\nAgain, we\u0027re close to FeatureFreeze so I\u0027m reluctant to hold this series for such a testing gap, but you should ideally write a follow-up patch that will add this extra testing coverage on the output of the method you verify.","commit_id":"32ed205794e0676f9555b084b9774ab08a7d96ce"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"2cf0b76d7270ae6c00455aa3d50b5a00715272c8","unresolved":true,"context_lines":[{"line_number":760,"context_line":"        self.api.attachment_get_all(self.ctx, instance_id)"},{"line_number":761,"context_line":"        mock_cinderclient.assert_called_once_with(self.ctx, \u00273.44\u0027,"},{"line_number":762,"context_line":"                                                  skip_version_check\u003dTrue)"},{"line_number":763,"context_line":"        mock_attachment.list.assert_called_once_with(search_opts\u003dsearch_opts)"},{"line_number":764,"context_line":""},{"line_number":765,"context_line":"    @mock.patch(\u0027nova.volume.cinder.cinderclient\u0027)"},{"line_number":766,"context_line":"    def test_attachment_get_all_by_volume(self, mock_cinderclient):"}],"source_content_type":"text/x-python","patch_set":35,"id":"b8f7f71f_fe3a2bf3","line":763,"in_reply_to":"88f39abc_7af99a6c","updated":"2023-08-30 07:52:46.000000000","message":"Ack, will write a follow-up patch.\nthanks","commit_id":"32ed205794e0676f9555b084b9774ab08a7d96ce"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"903b3712a409630c27e8c9134271a9fad191df22","unresolved":true,"context_lines":[{"line_number":773,"context_line":"        self.api.attachment_get_all(self.ctx, volume_id\u003dvolume_id)"},{"line_number":774,"context_line":"        mock_cinderclient.assert_called_once_with(self.ctx, \u00273.44\u0027,"},{"line_number":775,"context_line":"                                                  skip_version_check\u003dTrue)"},{"line_number":776,"context_line":"        mock_attachment.list.assert_called_once_with(search_opts\u003dsearch_opts)"},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"    @mock.patch(\u0027nova.volume.cinder.cinderclient\u0027)"},{"line_number":779,"context_line":"    def test_attachment_get_all_failed(self, mock_cinderclient):"}],"source_content_type":"text/x-python","patch_set":35,"id":"bad61937_44be7836","line":776,"updated":"2023-08-29 14:34:13.000000000","message":"same concern here, I don\u0027t see where we check the output types of that method based on those params.","commit_id":"32ed205794e0676f9555b084b9774ab08a7d96ce"}],"nova/volume/cinder.py":[{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"358c1e02442711e2fd65b2ae6f7fd2eab19953a6","unresolved":true,"context_lines":[{"line_number":860,"context_line":"            search_opts[\u0027volume_id\u0027] \u003d volume_id"},{"line_number":861,"context_line":"        try:"},{"line_number":862,"context_line":"            return cinderclient("},{"line_number":863,"context_line":"                context, \u00273.44\u0027, skip_version_check\u003dTrue).attachments.list("},{"line_number":864,"context_line":"                    search_opts\u003dsearch_opts)"},{"line_number":865,"context_line":"        except cinder_exception.ClientException as ex:"},{"line_number":866,"context_line":"            LOG.error(\u0027Get all attachment failed. \u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"8577698e_ffc77ea1","line":863,"updated":"2023-06-05 12:24:43.000000000","message":"in tests, this cinderclient is throwing 401, passing admin_context too didn\u0027t work.\n\n \n`\nERROR [nova.volume.cinder] The [cinder] section of your nova configuration file must be configured for authentication with the block-storage service endpoint.\nERROR [nova.volume.cinder] Get all attachment failed. Error: Unknown auth type: None (HTTP 401) Code: 401`\n\nwhile calling API via OSC, working as expected.","commit_id":"169e46313f56afd6b97f1be723b980b483750d48"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"07760391a6c0667e2422865c02fce0f40b93160b","unresolved":true,"context_lines":[{"line_number":860,"context_line":"            search_opts[\u0027volume_id\u0027] \u003d volume_id"},{"line_number":861,"context_line":"        try:"},{"line_number":862,"context_line":"            return cinderclient("},{"line_number":863,"context_line":"                context, \u00273.44\u0027, skip_version_check\u003dTrue).attachments.list("},{"line_number":864,"context_line":"                    search_opts\u003dsearch_opts)"},{"line_number":865,"context_line":"        except cinder_exception.ClientException as ex:"},{"line_number":866,"context_line":"            LOG.error(\u0027Get all attachment failed. \u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"421c1935_501fdcd5","line":863,"in_reply_to":"3062fd60_86a24542","updated":"2023-06-08 14:26:25.000000000","message":"\u003e side question: when to use get_admin_context ?\n\nIt\u0027s only useful when making queries to our own database with admin-level permissions. Once inside our service, we don\u0027t check credentials anymore, so it\u0027s easy to just fake a context with admin\u003dTrue to query the database.","commit_id":"169e46313f56afd6b97f1be723b980b483750d48"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"0368725b05301bf27da4c8008be915d6db66e2c9","unresolved":true,"context_lines":[{"line_number":860,"context_line":"            search_opts[\u0027volume_id\u0027] \u003d volume_id"},{"line_number":861,"context_line":"        try:"},{"line_number":862,"context_line":"            return cinderclient("},{"line_number":863,"context_line":"                context, \u00273.44\u0027, skip_version_check\u003dTrue).attachments.list("},{"line_number":864,"context_line":"                    search_opts\u003dsearch_opts)"},{"line_number":865,"context_line":"        except cinder_exception.ClientException as ex:"},{"line_number":866,"context_line":"            LOG.error(\u0027Get all attachment failed. \u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"3062fd60_86a24542","line":863,"in_reply_to":"391f0954_bf2abc8c","updated":"2023-06-08 14:20:40.000000000","message":"ack, this was not related to admin acces but sending wrong data to Cinder API.\nupdated CinderFixture so now its not calling to Cinder API. thanks.\n\nside question: when to use get_admin_context ?","commit_id":"169e46313f56afd6b97f1be723b980b483750d48"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"f33a5795a6d7d8f31513f268a50c701f5330958b","unresolved":false,"context_lines":[{"line_number":860,"context_line":"            search_opts[\u0027volume_id\u0027] \u003d volume_id"},{"line_number":861,"context_line":"        try:"},{"line_number":862,"context_line":"            return cinderclient("},{"line_number":863,"context_line":"                context, \u00273.44\u0027, skip_version_check\u003dTrue).attachments.list("},{"line_number":864,"context_line":"                    search_opts\u003dsearch_opts)"},{"line_number":865,"context_line":"        except cinder_exception.ClientException as ex:"},{"line_number":866,"context_line":"            LOG.error(\u0027Get all attachment failed. \u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"ad882655_3cf1e95c","line":863,"in_reply_to":"421c1935_501fdcd5","updated":"2023-06-12 13:57:01.000000000","message":"ack, thanks Dan.","commit_id":"169e46313f56afd6b97f1be723b980b483750d48"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1389508246ac123cffa1b762ffd87ec5ddc3a88d","unresolved":true,"context_lines":[{"line_number":860,"context_line":"            search_opts[\u0027volume_id\u0027] \u003d volume_id"},{"line_number":861,"context_line":"        try:"},{"line_number":862,"context_line":"            return cinderclient("},{"line_number":863,"context_line":"                context, \u00273.44\u0027, skip_version_check\u003dTrue).attachments.list("},{"line_number":864,"context_line":"                    search_opts\u003dsearch_opts)"},{"line_number":865,"context_line":"        except cinder_exception.ClientException as ex:"},{"line_number":866,"context_line":"            LOG.error(\u0027Get all attachment failed. \u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"391f0954_bf2abc8c","line":863,"in_reply_to":"8577698e_ffc77ea1","updated":"2023-06-05 15:10:52.000000000","message":"`get_admin_context()` does not return a context you can use outside of nova. It literally has no credentials to be able to authenticate the cinder. You\u0027ll need to use nova\u0027s service user to authenticate.","commit_id":"169e46313f56afd6b97f1be723b980b483750d48"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f480f59037925dda06744c59fab10c2a58972225","unresolved":true,"context_lines":[{"line_number":839,"context_line":"                           \u0027msg\u0027: str(ex),"},{"line_number":840,"context_line":"                           \u0027code\u0027: getattr(ex, \u0027code\u0027, None)})"},{"line_number":841,"context_line":""},{"line_number":842,"context_line":"    def attachment_get_all(self, context, instance_id\u003dNone, volume_id\u003dNone):"},{"line_number":843,"context_line":"        \"\"\"Get all attchments by instance id or volume id"},{"line_number":844,"context_line":""},{"line_number":845,"context_line":"        :param context: The nova request context."}],"source_content_type":"text/x-python","patch_set":30,"id":"a798fea2_e9032533","line":842,"updated":"2023-07-28 10:25:43.000000000","message":"if this is a reproducer, why are you adding code in here ?\n\nI think I get it but you shall explain IMHO in the commit msg.","commit_id":"9c6138635107f3ba32e4d3da94738e38985d5fe9"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"72eb3905d3e6b32ff1c42ba837c8d6011ff5a6ff","unresolved":false,"context_lines":[{"line_number":839,"context_line":"                           \u0027msg\u0027: str(ex),"},{"line_number":840,"context_line":"                           \u0027code\u0027: getattr(ex, \u0027code\u0027, None)})"},{"line_number":841,"context_line":""},{"line_number":842,"context_line":"    def attachment_get_all(self, context, instance_id\u003dNone, volume_id\u003dNone):"},{"line_number":843,"context_line":"        \"\"\"Get all attchments by instance id or volume id"},{"line_number":844,"context_line":""},{"line_number":845,"context_line":"        :param context: The nova request context."}],"source_content_type":"text/x-python","patch_set":30,"id":"fcaaa7b6_380d6d37","line":842,"in_reply_to":"a798fea2_e9032533","updated":"2023-07-28 17:06:21.000000000","message":"ack, moved reproducer in a different patch.\nso this this patch is to add attachment_get_all function in Cinder.Api supporting tests.\nthanks.","commit_id":"9c6138635107f3ba32e4d3da94738e38985d5fe9"}]}