)]}'
{"cinder/tests/unit/attachments/test_attachments_api.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"426b8d95c487ca80b13c05b01478504577cd8b02","unresolved":false,"context_lines":[{"line_number":368,"context_line":"        attachment.connection_info \u003d {\u0027c\u0027: \u0027d\u0027}"},{"line_number":369,"context_line":"        attachment.connector \u003d {\u0027host\u0027: \u0027somehost\u0027}"},{"line_number":370,"context_line":"        with mock.patch(\u0027cinder.objects.Volume.get_by_id\u0027, return_value\u003dvref):"},{"line_number":371,"context_line":"            self.assertRaises(exception.Invalid,"},{"line_number":372,"context_line":"                              volume_api.API.attachment_update,"},{"line_number":373,"context_line":"                              self.volume_api,"},{"line_number":374,"context_line":"                              self.context,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_18df12d7","line":371,"range":{"start_line":371,"start_character":40,"end_line":371,"end_character":47},"updated":"2019-07-25 15:47:13.000000000","message":"tighten to InvalidVolume?","commit_id":"79186b23b1bb1aa63399efad87ed5b1d801f2a10"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c34e99278e4502f1a39849f33c827f2d4e0305cc","unresolved":false,"context_lines":[{"line_number":322,"context_line":"                return_value\u003d{})"},{"line_number":323,"context_line":"    @mock.patch(\u0027cinder.volume.rpcapi.VolumeAPI.attachment_update\u0027,"},{"line_number":324,"context_line":"                return_value\u003d{})"},{"line_number":325,"context_line":"    def test_attachment_update_duplicate(self, mock_va_update, mock_db_upd):"},{"line_number":326,"context_line":"        volume_params \u003d {\u0027status\u0027: \u0027available\u0027}"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"        vref \u003d tests_utils.create_volume(self.context,"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_0976bba5","line":325,"updated":"2019-07-29 14:46:45.000000000","message":"?: Why aren\u0027t we checking that any of these methods are being called with the right parameters?","commit_id":"d62b7f7c7da5a1a5a32a984ae85d3a7f5a548665"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c34e99278e4502f1a39849f33c827f2d4e0305cc","unresolved":false,"context_lines":[{"line_number":329,"context_line":"                                         deleted\u003d0,"},{"line_number":330,"context_line":"                                         **volume_params)"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"        vref \u003d tests_utils.attach_volume(self.context,"},{"line_number":333,"context_line":"                                         vref.id,"},{"line_number":334,"context_line":"                                         fake.UUID1,"},{"line_number":335,"context_line":"                                         \u0027somehost\u0027,"},{"line_number":336,"context_line":"                                         \u0027somemountpoint\u0027)"},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"        vref \u003d objects.Volume._from_db_object(self.context,"},{"line_number":339,"context_line":"                                              objects.Volume(),"},{"line_number":340,"context_line":"                                              vref)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        connector \u003d {\u0027host\u0027: \u0027somehost\u0027}"},{"line_number":343,"context_line":"        attachment \u003d fake_volume.volume_attachment_ovo(self.context)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_865ff83f","line":340,"range":{"start_line":332,"start_character":0,"end_line":340,"end_character":51},"updated":"2019-07-29 14:46:45.000000000","message":"?: Isn\u0027t this equivalent to:\n\n        tests_utils.attach_volume(self.context,\n                                  vref.id,\n                                  fake.UUID1,\n                                  \u0027somehost\u0027,\n                                  \u0027somemountpoint\u0027)\n\n        vref.refresh()","commit_id":"d62b7f7c7da5a1a5a32a984ae85d3a7f5a548665"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"0b2d26da27d52dd6fbc55502be80966d7a6f1544","unresolved":false,"context_lines":[{"line_number":329,"context_line":"                                         deleted\u003d0,"},{"line_number":330,"context_line":"                                         **volume_params)"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"        vref \u003d tests_utils.attach_volume(self.context,"},{"line_number":333,"context_line":"                                         vref.id,"},{"line_number":334,"context_line":"                                         fake.UUID1,"},{"line_number":335,"context_line":"                                         \u0027somehost\u0027,"},{"line_number":336,"context_line":"                                         \u0027somemountpoint\u0027)"},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"        vref \u003d objects.Volume._from_db_object(self.context,"},{"line_number":339,"context_line":"                                              objects.Volume(),"},{"line_number":340,"context_line":"                                              vref)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        connector \u003d {\u0027host\u0027: \u0027somehost\u0027}"},{"line_number":343,"context_line":"        attachment \u003d fake_volume.volume_attachment_ovo(self.context)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_49f3539f","line":340,"range":{"start_line":332,"start_character":0,"end_line":340,"end_character":51},"in_reply_to":"7faddb67_865ff83f","updated":"2019-07-29 15:03:57.000000000","message":"No, this just produces\n    AttributeError: \u0027Volume\u0027 object has no attribute \u0027refresh\u0027","commit_id":"d62b7f7c7da5a1a5a32a984ae85d3a7f5a548665"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c34e99278e4502f1a39849f33c827f2d4e0305cc","unresolved":false,"context_lines":[{"line_number":344,"context_line":"        attachment.volume_id \u003d vref.id"},{"line_number":345,"context_line":"        attachment.connection_info \u003d {\u0027a\u0027: \u0027b\u0027}"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"        vref.volume_attachment[0][\u0027connector\u0027] \u003d {\u0027host\u0027: \u0027someotherhost\u0027}"},{"line_number":348,"context_line":"        vref.volume_attachment[0][\u0027connection_info\u0027] \u003d {\u0027a\u0027: \u0027b\u0027}"},{"line_number":349,"context_line":"        with mock.patch(\u0027cinder.objects.Volume.get_by_id\u0027, return_value\u003dvref):"},{"line_number":350,"context_line":"            volume_api.API.attachment_update(self.volume_api,"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_c66c100a","line":347,"updated":"2019-07-29 14:46:45.000000000","message":"nit: this is an OVO, so better use `.connector`","commit_id":"d62b7f7c7da5a1a5a32a984ae85d3a7f5a548665"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c34e99278e4502f1a39849f33c827f2d4e0305cc","unresolved":false,"context_lines":[{"line_number":345,"context_line":"        attachment.connection_info \u003d {\u0027a\u0027: \u0027b\u0027}"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"        vref.volume_attachment[0][\u0027connector\u0027] \u003d {\u0027host\u0027: \u0027someotherhost\u0027}"},{"line_number":348,"context_line":"        vref.volume_attachment[0][\u0027connection_info\u0027] \u003d {\u0027a\u0027: \u0027b\u0027}"},{"line_number":349,"context_line":"        with mock.patch(\u0027cinder.objects.Volume.get_by_id\u0027, return_value\u003dvref):"},{"line_number":350,"context_line":"            volume_api.API.attachment_update(self.volume_api,"},{"line_number":351,"context_line":"                                             self.context,"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_4680208c","line":348,"updated":"2019-07-29 14:46:45.000000000","message":"ditto","commit_id":"d62b7f7c7da5a1a5a32a984ae85d3a7f5a548665"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c34e99278e4502f1a39849f33c827f2d4e0305cc","unresolved":false,"context_lines":[{"line_number":347,"context_line":"        vref.volume_attachment[0][\u0027connector\u0027] \u003d {\u0027host\u0027: \u0027someotherhost\u0027}"},{"line_number":348,"context_line":"        vref.volume_attachment[0][\u0027connection_info\u0027] \u003d {\u0027a\u0027: \u0027b\u0027}"},{"line_number":349,"context_line":"        with mock.patch(\u0027cinder.objects.Volume.get_by_id\u0027, return_value\u003dvref):"},{"line_number":350,"context_line":"            volume_api.API.attachment_update(self.volume_api,"},{"line_number":351,"context_line":"                                             self.context,"},{"line_number":352,"context_line":"                                             attachment,"},{"line_number":353,"context_line":"                                             connector)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_c613b065","line":350,"updated":"2019-07-29 14:46:45.000000000","message":"?: What are we doing here and why? Is this a test? If it is there should be some kind of check of the results of the test.","commit_id":"d62b7f7c7da5a1a5a32a984ae85d3a7f5a548665"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c34e99278e4502f1a39849f33c827f2d4e0305cc","unresolved":false,"context_lines":[{"line_number":353,"context_line":"                                             connector)"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"        # Update volume with another attachment"},{"line_number":356,"context_line":"        vref \u003d tests_utils.attach_volume(self.context,"},{"line_number":357,"context_line":"                                         vref.id,"},{"line_number":358,"context_line":"                                         fake.UUID2,"},{"line_number":359,"context_line":"                                         \u0027somehost2\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_a608f42c","line":356,"updated":"2019-07-29 14:46:45.000000000","message":"?: Is this a multi-attach volume? Or is this a live migration simulation?","commit_id":"d62b7f7c7da5a1a5a32a984ae85d3a7f5a548665"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c34e99278e4502f1a39849f33c827f2d4e0305cc","unresolved":false,"context_lines":[{"line_number":367,"context_line":"        vref.volume_attachment[1][\u0027connection_info\u0027] \u003d {\u0027c\u0027: \u0027d\u0027}"},{"line_number":368,"context_line":"        attachment \u003d fake_volume.volume_attachment_ovo(self.context)"},{"line_number":369,"context_line":"        attachment.volume_id \u003d vref.id"},{"line_number":370,"context_line":"        attachment.connection_info \u003d {\u0027c\u0027: \u0027d\u0027}"},{"line_number":371,"context_line":"        attachment.connector \u003d {\u0027host\u0027: \u0027somehost\u0027}"},{"line_number":372,"context_line":"        with mock.patch(\u0027cinder.objects.Volume.get_by_id\u0027, return_value\u003dvref):"},{"line_number":373,"context_line":"            self.assertRaises(exception.InvalidVolume,"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_a93be7c5","line":370,"updated":"2019-07-29 14:46:45.000000000","message":"-1: The attachment should not have the connection_info already, that\u0027s what the attachment_update method is supposed to do","commit_id":"d62b7f7c7da5a1a5a32a984ae85d3a7f5a548665"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c34e99278e4502f1a39849f33c827f2d4e0305cc","unresolved":false,"context_lines":[{"line_number":375,"context_line":"                              self.volume_api,"},{"line_number":376,"context_line":"                              self.context,"},{"line_number":377,"context_line":"                              attachment,"},{"line_number":378,"context_line":"                              connector)"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"    def test_attachment_create_creating_volume(self):"},{"line_number":381,"context_line":"        \"\"\"Test attachment_create on a creating volume.\"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_695e4f13","line":378,"updated":"2019-07-29 14:46:45.000000000","message":"Why is there no check that the RPC api call didn\u0027t happen?","commit_id":"d62b7f7c7da5a1a5a32a984ae85d3a7f5a548665"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5b08377d1c64b7c7081b9a86307834c3d7b54723","unresolved":false,"context_lines":[{"line_number":346,"context_line":""},{"line_number":347,"context_line":"        # This attachment will collide with the first"},{"line_number":348,"context_line":"        connector \u003d {\u0027host\u0027: \u0027somehost\u0027}"},{"line_number":349,"context_line":"        vref.volume_attachment[1][\u0027connector\u0027] \u003d {\u0027host\u0027: \u0027somehost\u0027}"},{"line_number":350,"context_line":"        vref.volume_attachment[1][\u0027connection_info\u0027] \u003d {\u0027c\u0027: \u0027d\u0027}"},{"line_number":351,"context_line":"        attachment \u003d fake_volume.volume_attachment_ovo(self.context)"},{"line_number":352,"context_line":"        attachment.volume_id \u003d vref.id"},{"line_number":353,"context_line":"        attachment.connector \u003d {\u0027host\u0027: \u0027somehost\u0027}"},{"line_number":354,"context_line":"        with mock.patch(\u0027cinder.objects.Volume.get_by_id\u0027, return_value\u003dvref):"},{"line_number":355,"context_line":"            self.assertRaises(exception.InvalidVolume,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_34f0aec9","line":352,"range":{"start_line":349,"start_character":0,"end_line":352,"end_character":38},"updated":"2019-07-29 15:48:52.000000000","message":"-1: We should not do this. The first 2 lines would be done by our call to attachment_update if it were successful, and the las 2 are not necessary, since we have the attachment OVO accessible as vref.volume_attachment[1].\n\nIt should be enough just passing on L359:\n\n   vref.volume_attachment[1],\n\ninstead of `attachment`.","commit_id":"87524240cf396cacc0215d13d861aedeac0bfe1f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5b08377d1c64b7c7081b9a86307834c3d7b54723","unresolved":false,"context_lines":[{"line_number":351,"context_line":"        attachment \u003d fake_volume.volume_attachment_ovo(self.context)"},{"line_number":352,"context_line":"        attachment.volume_id \u003d vref.id"},{"line_number":353,"context_line":"        attachment.connector \u003d {\u0027host\u0027: \u0027somehost\u0027}"},{"line_number":354,"context_line":"        with mock.patch(\u0027cinder.objects.Volume.get_by_id\u0027, return_value\u003dvref):"},{"line_number":355,"context_line":"            self.assertRaises(exception.InvalidVolume,"},{"line_number":356,"context_line":"                              volume_api.API.attachment_update,"},{"line_number":357,"context_line":"                              self.volume_api,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_14e012cc","line":354,"updated":"2019-07-29 15:48:52.000000000","message":"We should also mock `self.volume_api.API.volume_rpcapi.attachment_update` and confirm that it is not being called.","commit_id":"87524240cf396cacc0215d13d861aedeac0bfe1f"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"6e147b5c32ea2d4cdf63411146183900abe75ef1","unresolved":false,"context_lines":[{"line_number":351,"context_line":"        attachment \u003d fake_volume.volume_attachment_ovo(self.context)"},{"line_number":352,"context_line":"        attachment.volume_id \u003d vref.id"},{"line_number":353,"context_line":"        attachment.connector \u003d {\u0027host\u0027: \u0027somehost\u0027}"},{"line_number":354,"context_line":"        with mock.patch(\u0027cinder.objects.Volume.get_by_id\u0027, return_value\u003dvref):"},{"line_number":355,"context_line":"            self.assertRaises(exception.InvalidVolume,"},{"line_number":356,"context_line":"                              volume_api.API.attachment_update,"},{"line_number":357,"context_line":"                              self.volume_api,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_2facd78a","line":354,"in_reply_to":"7faddb67_14e012cc","updated":"2019-07-29 16:40:07.000000000","message":"Done","commit_id":"87524240cf396cacc0215d13d861aedeac0bfe1f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5b08377d1c64b7c7081b9a86307834c3d7b54723","unresolved":false,"context_lines":[{"line_number":353,"context_line":"        attachment.connector \u003d {\u0027host\u0027: \u0027somehost\u0027}"},{"line_number":354,"context_line":"        with mock.patch(\u0027cinder.objects.Volume.get_by_id\u0027, return_value\u003dvref):"},{"line_number":355,"context_line":"            self.assertRaises(exception.InvalidVolume,"},{"line_number":356,"context_line":"                              volume_api.API.attachment_update,"},{"line_number":357,"context_line":"                              self.volume_api,"},{"line_number":358,"context_line":"                              self.context,"},{"line_number":359,"context_line":"                              attachment,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_f4dc5619","line":356,"updated":"2019-07-29 15:48:52.000000000","message":"-1: We should be using `self.volume_api.API.attachment_update`","commit_id":"87524240cf396cacc0215d13d861aedeac0bfe1f"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"6e147b5c32ea2d4cdf63411146183900abe75ef1","unresolved":false,"context_lines":[{"line_number":353,"context_line":"        attachment.connector \u003d {\u0027host\u0027: \u0027somehost\u0027}"},{"line_number":354,"context_line":"        with mock.patch(\u0027cinder.objects.Volume.get_by_id\u0027, return_value\u003dvref):"},{"line_number":355,"context_line":"            self.assertRaises(exception.InvalidVolume,"},{"line_number":356,"context_line":"                              volume_api.API.attachment_update,"},{"line_number":357,"context_line":"                              self.volume_api,"},{"line_number":358,"context_line":"                              self.context,"},{"line_number":359,"context_line":"                              attachment,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_4feb53db","line":356,"in_reply_to":"7faddb67_f4dc5619","updated":"2019-07-29 16:40:07.000000000","message":"Done","commit_id":"87524240cf396cacc0215d13d861aedeac0bfe1f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5b08377d1c64b7c7081b9a86307834c3d7b54723","unresolved":false,"context_lines":[{"line_number":356,"context_line":"                              volume_api.API.attachment_update,"},{"line_number":357,"context_line":"                              self.volume_api,"},{"line_number":358,"context_line":"                              self.context,"},{"line_number":359,"context_line":"                              attachment,"},{"line_number":360,"context_line":"                              connector)"},{"line_number":361,"context_line":"        mock_va_update.assert_not_called()"},{"line_number":362,"context_line":"        mock_db_upd.assert_not_called()"}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_94f40289","line":359,"range":{"start_line":359,"start_character":30,"end_line":359,"end_character":40},"updated":"2019-07-29 15:48:52.000000000","message":"As mentioned above, this should be `vref.volume_attachment[1],`","commit_id":"87524240cf396cacc0215d13d861aedeac0bfe1f"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"6e147b5c32ea2d4cdf63411146183900abe75ef1","unresolved":false,"context_lines":[{"line_number":356,"context_line":"                              volume_api.API.attachment_update,"},{"line_number":357,"context_line":"                              self.volume_api,"},{"line_number":358,"context_line":"                              self.context,"},{"line_number":359,"context_line":"                              attachment,"},{"line_number":360,"context_line":"                              connector)"},{"line_number":361,"context_line":"        mock_va_update.assert_not_called()"},{"line_number":362,"context_line":"        mock_db_upd.assert_not_called()"}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_efbb1fe2","line":359,"range":{"start_line":359,"start_character":30,"end_line":359,"end_character":40},"in_reply_to":"7faddb67_94f40289","updated":"2019-07-29 16:40:07.000000000","message":"Done","commit_id":"87524240cf396cacc0215d13d861aedeac0bfe1f"}],"cinder/volume/api.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09d668a53a1bab2454372edb53a8b8a045ef6a00","unresolved":false,"context_lines":[{"line_number":2156,"context_line":"        attachment_ref.save()"},{"line_number":2157,"context_line":"        return attachment_ref"},{"line_number":2158,"context_line":""},{"line_number":2159,"context_line":"    @coordination.synchronized(\u0027{f_name}-{attachment_ref.volume_id}\u0027)"},{"line_number":2160,"context_line":"    def attachment_update(self, ctxt, attachment_ref, connector):"},{"line_number":2161,"context_line":"        \"\"\"Update an existing attachment record.\"\"\""},{"line_number":2162,"context_line":"        # Valid items to update (connector includes mode and mountpoint):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_3573f782","line":2159,"updated":"2019-07-25 14:40:24.000000000","message":"I think we should lock based on the host information contained on the connector.  That way we will not introduce an unnecessary bottleneck for multi-attach, right?","commit_id":"79186b23b1bb1aa63399efad87ed5b1d801f2a10"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09d668a53a1bab2454372edb53a8b8a045ef6a00","unresolved":false,"context_lines":[{"line_number":2174,"context_line":""},{"line_number":2175,"context_line":"        ctxt.authorize(attachment_policy.UPDATE_POLICY,"},{"line_number":2176,"context_line":"                       target_obj\u003dattachment_ref)"},{"line_number":2177,"context_line":""},{"line_number":2178,"context_line":"        volume_ref \u003d objects.Volume.get_by_id(ctxt, attachment_ref.volume_id)"},{"line_number":2179,"context_line":"        if \"error\" in volume_ref.status:"},{"line_number":2180,"context_line":"            msg \u003d (\u0027Volume attachments can not be updated if the volume \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_c3642c4c","line":2177,"updated":"2019-07-25 14:40:24.000000000","message":"nit: unnecessary change","commit_id":"79186b23b1bb1aa63399efad87ed5b1d801f2a10"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09d668a53a1bab2454372edb53a8b8a045ef6a00","unresolved":false,"context_lines":[{"line_number":2185,"context_line":"            LOG.error(msg)"},{"line_number":2186,"context_line":"            raise exception.InvalidVolume(reason\u003dmsg)"},{"line_number":2187,"context_line":""},{"line_number":2188,"context_line":"        if (len(volume_ref.volume_attachment) \u003e 1 and"},{"line_number":2189,"context_line":"            not (volume_ref.multiattach or"},{"line_number":2190,"context_line":"                 self._is_multiattach(volume_ref.volume_type))):"},{"line_number":2191,"context_line":"            connection_hosts \u003d set([a.connector[\u0027host\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_c31c2c79","line":2188,"updated":"2019-07-25 14:40:24.000000000","message":"This doesn\u0027t seem to prevent the race condition for multiattach.  And I think we wouldn\u0027t want to attach the same volume multiple times to the same instance on the same host even if it\u0027s a multi-attach volume.","commit_id":"79186b23b1bb1aa63399efad87ed5b1d801f2a10"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"09d668a53a1bab2454372edb53a8b8a045ef6a00","unresolved":false,"context_lines":[{"line_number":2190,"context_line":"                 self._is_multiattach(volume_ref.volume_type))):"},{"line_number":2191,"context_line":"            connection_hosts \u003d set([a.connector[\u0027host\u0027]"},{"line_number":2192,"context_line":"                                    for a in volume_ref.volume_attachment"},{"line_number":2193,"context_line":"                                    if a.connector])"},{"line_number":2194,"context_line":""},{"line_number":2195,"context_line":"            # Check whether all connection hosts are unique"},{"line_number":2196,"context_line":"            # Multiple attachments to different hosts is permitted to"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_83d394d5","line":2193,"updated":"2019-07-25 14:40:24.000000000","message":"-1: Don\u0027t build a list just to construct a set. Directly build the set using the generator.","commit_id":"79186b23b1bb1aa63399efad87ed5b1d801f2a10"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"237830bb5700fff732d872a4495a9d0f447e9372","unresolved":false,"context_lines":[{"line_number":2202,"context_line":"                for a in volume_ref.volume_attachment:"},{"line_number":2203,"context_line":"                    if not a.connection_info:"},{"line_number":2204,"context_line":"                        LOG.debug(\"removing attachment: %s\", a)"},{"line_number":2205,"context_line":"                        a.destroy()"},{"line_number":2206,"context_line":""},{"line_number":2207,"context_line":"                        msg \u003d _(\u0027duplicate connectors detected on volume \u0027"},{"line_number":2208,"context_line":"                                \u0027%(vol)s\u0027) % {\u0027vol\u0027: volume_ref.id}"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_38276e15","line":2205,"updated":"2019-07-25 15:40:23.000000000","message":"-1: This delete cannot happen here.  We must add a try...catch clause on line 2134 and delete it there.","commit_id":"79186b23b1bb1aa63399efad87ed5b1d801f2a10"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"1fefbb5d7929dbb873f510a4f32ffc2aea3ac497","unresolved":false,"context_lines":[{"line_number":2202,"context_line":"                for a in volume_ref.volume_attachment:"},{"line_number":2203,"context_line":"                    if not a.connection_info:"},{"line_number":2204,"context_line":"                        LOG.debug(\"removing attachment: %s\", a)"},{"line_number":2205,"context_line":"                        a.destroy()"},{"line_number":2206,"context_line":""},{"line_number":2207,"context_line":"                        msg \u003d _(\u0027duplicate connectors detected on volume \u0027"},{"line_number":2208,"context_line":"                                \u0027%(vol)s\u0027) % {\u0027vol\u0027: volume_ref.id}"}],"source_content_type":"text/x-python","patch_set":3,"id":"7faddb67_aeef9485","line":2205,"in_reply_to":"7faddb67_38276e15","updated":"2019-07-25 17:18:41.000000000","message":"Sorry, attachment_create doesn\u0027t call this method, so we don\u0027t need the try...catch, we just need to remove this destroy from here and let the caller do the cleanup.","commit_id":"79186b23b1bb1aa63399efad87ed5b1d801f2a10"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"de0c156cc4b15f084103012a75b527c7f8e29231","unresolved":false,"context_lines":[{"line_number":2135,"context_line":"                                                     volume_ref,"},{"line_number":2136,"context_line":"                                                     connector,"},{"line_number":2137,"context_line":"                                                     attachment_ref.id))"},{"line_number":2138,"context_line":""},{"line_number":2139,"context_line":"        attachment_ref.connection_info \u003d connection_info"},{"line_number":2140,"context_line":""},{"line_number":2141,"context_line":"        # Use of admin_metadata for RO settings is deprecated"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_a3dc6974","line":2138,"updated":"2019-07-29 10:13:30.000000000","message":"nit: Unrelated to the patch","commit_id":"1c68fe664a35099d8c20c767f4b4af012d208e17"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"23d5c353b7d07dfc7dc2fdf0cc03b1d8469e9357","unresolved":false,"context_lines":[{"line_number":2157,"context_line":"        attachment_ref.save()"},{"line_number":2158,"context_line":"        return attachment_ref"},{"line_number":2159,"context_line":""},{"line_number":2160,"context_line":"    @coordination.synchronized(\u0027{f_name}-{attachment_ref.volume_id}\u0027)"},{"line_number":2161,"context_line":"    def attachment_update(self, ctxt, attachment_ref, connector):"},{"line_number":2162,"context_line":"        \"\"\"Update an existing attachment record.\"\"\""},{"line_number":2163,"context_line":"        # Valid items to update (connector includes mode and mountpoint):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_37b529ef","line":2160,"updated":"2019-07-26 17:38:32.000000000","message":"I didn\u0027t yet tighten this lock to be per-host.","commit_id":"1c68fe664a35099d8c20c767f4b4af012d208e17"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"de0c156cc4b15f084103012a75b527c7f8e29231","unresolved":false,"context_lines":[{"line_number":2157,"context_line":"        attachment_ref.save()"},{"line_number":2158,"context_line":"        return attachment_ref"},{"line_number":2159,"context_line":""},{"line_number":2160,"context_line":"    @coordination.synchronized(\u0027{f_name}-{attachment_ref.volume_id}\u0027)"},{"line_number":2161,"context_line":"    def attachment_update(self, ctxt, attachment_ref, connector):"},{"line_number":2162,"context_line":"        \"\"\"Update an existing attachment record.\"\"\""},{"line_number":2163,"context_line":"        # Valid items to update (connector includes mode and mountpoint):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_430815d8","line":2160,"in_reply_to":"7faddb67_37b529ef","updated":"2019-07-29 10:13:30.000000000","message":"I think this should work and be sufficient:\n\n  @coordination.synchronized(\u0027{f_name}-{attachment_ref.volume_id}-{connector[host]}\u0027)","commit_id":"1c68fe664a35099d8c20c767f4b4af012d208e17"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"de0c156cc4b15f084103012a75b527c7f8e29231","unresolved":false,"context_lines":[{"line_number":2199,"context_line":"                                   for a in volume_ref.volume_attachment"},{"line_number":2200,"context_line":"                                   if a.connector)"},{"line_number":2201,"context_line":""},{"line_number":2202,"context_line":"            if len(connection_hosts) \u003c len(volume_ref.volume_attachment):"},{"line_number":2203,"context_line":"                # We raced, and all connection hosts are not unique"},{"line_number":2204,"context_line":"                # Remove one that doesn\u0027t have connection_info yet, and fail"},{"line_number":2205,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_03b9bde7","line":2202,"updated":"2019-07-29 10:13:30.000000000","message":"nit: I think this check is a little bit convoluted...\n\nTo me it would be clearer:\n\n if len(connection_hosts):\n\nBecause we know the volume doesn\u0027t accept multi-attach, and we know we should fail if we already have a volume_attachment that has completed the operation and set the connector.","commit_id":"1c68fe664a35099d8c20c767f4b4af012d208e17"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"de0c156cc4b15f084103012a75b527c7f8e29231","unresolved":false,"context_lines":[{"line_number":2200,"context_line":"                                   if a.connector)"},{"line_number":2201,"context_line":""},{"line_number":2202,"context_line":"            if len(connection_hosts) \u003c len(volume_ref.volume_attachment):"},{"line_number":2203,"context_line":"                # We raced, and all connection hosts are not unique"},{"line_number":2204,"context_line":"                # Remove one that doesn\u0027t have connection_info yet, and fail"},{"line_number":2205,"context_line":""},{"line_number":2206,"context_line":"                for a in volume_ref.volume_attachment:"},{"line_number":2207,"context_line":"                    if not a.connection_info:"},{"line_number":2208,"context_line":"                        msg \u003d _(\u0027duplicate connectors detected on volume \u0027"},{"line_number":2209,"context_line":"                                \u0027%(vol)s\u0027) % {\u0027vol\u0027: volume_ref.id}"},{"line_number":2210,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_e3aa6183","line":2207,"range":{"start_line":2203,"start_character":0,"end_line":2207,"end_character":45},"updated":"2019-07-29 10:13:30.000000000","message":"-1: All this is no longer necessary.","commit_id":"1c68fe664a35099d8c20c767f4b4af012d208e17"}]}