)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5fe690db04e9f38cbca8f3f15bcf7782c3bb86fb","unresolved":false,"context_lines":[{"line_number":7,"context_line":"fixtures: Track attachments within CinderFixtureNewAttachFlow"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Previously attachments ids were not tracked at all within the fixture"},{"line_number":10,"context_line":"with only the instance_uuid and volume_id stashed.This change should"},{"line_number":11,"context_line":"allow future functional tests to exercise bugs where attachments are"},{"line_number":12,"context_line":"deleted but not recreated."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5f7c97a3_add93681","line":10,"range":{"start_line":10,"start_character":49,"end_line":10,"end_character":50},"updated":"2018-08-02 13:31:58.000000000","message":"space","commit_id":"3d104d27ca1576b2609eba25ba4f7ee93baa54c2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b799191101e91af5491c2b117c73c11388c460d3","unresolved":false,"context_lines":[{"line_number":7,"context_line":"fixtures: Track attachments within CinderFixtureNewAttachFlow"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Previously attachments ids were not tracked at all within the fixture"},{"line_number":10,"context_line":"with only the instance_uuid and volume_id stashed.This change should"},{"line_number":11,"context_line":"allow future functional tests to exercise bugs where attachments are"},{"line_number":12,"context_line":"deleted but not recreated."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5f7c97a3_7549a88b","line":10,"range":{"start_line":10,"start_character":49,"end_line":10,"end_character":50},"in_reply_to":"5f7c97a3_add93681","updated":"2018-08-02 14:10:13.000000000","message":"Done","commit_id":"3d104d27ca1576b2609eba25ba4f7ee93baa54c2"}],"nova/tests/fixtures.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5fe690db04e9f38cbca8f3f15bcf7782c3bb86fb","unresolved":false,"context_lines":[{"line_number":1563,"context_line":"        self.swap_volume_instance_uuid \u003d None"},{"line_number":1564,"context_line":"        self.swap_volume_instance_error_uuid \u003d None"},{"line_number":1565,"context_line":"        self.attachment_error_id \u003d None"},{"line_number":1566,"context_line":"        # This is a map of instance UUIDs mapped to a list of volume IDs."},{"line_number":1567,"context_line":"        # This map gets updated on attach/detach operations."},{"line_number":1568,"context_line":"        self.attachments \u003d collections.defaultdict(list)"},{"line_number":1569,"context_line":""},{"line_number":1570,"context_line":"    def setUp(self):"},{"line_number":1571,"context_line":"        super(CinderFixtureNewAttachFlow, self).setUp()"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_2d30c653","line":1568,"range":{"start_line":1566,"start_character":8,"end_line":1568,"end_character":56},"updated":"2018-08-02 13:31:58.000000000","message":"Your re-use of this is a bit wonky given it\u0027s supposed to be a map of instance uuid to volume id, but you\u0027re tracking everything against a single \u0027attachment_id\u0027 key. But I see we also put an \u0027instance_uuid\u0027 key in here, which is confusing.","commit_id":"3d104d27ca1576b2609eba25ba4f7ee93baa54c2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5fe690db04e9f38cbca8f3f15bcf7782c3bb86fb","unresolved":false,"context_lines":[{"line_number":1657,"context_line":"            return {\u0027save_volume_id\u0027: new_volume_id}"},{"line_number":1658,"context_line":""},{"line_number":1659,"context_line":"        def _check_attachment_id(id):"},{"line_number":1660,"context_line":"            if id not in self.attachments[\u0027attachment_id\u0027]:"},{"line_number":1661,"context_line":"                raise exception.VolumeAttachmentNotFound(attachment_id\u003did)"},{"line_number":1662,"context_line":""},{"line_number":1663,"context_line":"        def fake_attachment_create(_self, context, volume_id, instance_uuid,"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_2d57665b","line":1660,"range":{"start_line":1660,"start_character":15,"end_line":1660,"end_character":17},"updated":"2018-08-02 13:31:58.000000000","message":"rename this to attachment_id to avoid shadowing a builtin.","commit_id":"3d104d27ca1576b2609eba25ba4f7ee93baa54c2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b799191101e91af5491c2b117c73c11388c460d3","unresolved":false,"context_lines":[{"line_number":1657,"context_line":"            return {\u0027save_volume_id\u0027: new_volume_id}"},{"line_number":1658,"context_line":""},{"line_number":1659,"context_line":"        def _check_attachment_id(id):"},{"line_number":1660,"context_line":"            if id not in self.attachments[\u0027attachment_id\u0027]:"},{"line_number":1661,"context_line":"                raise exception.VolumeAttachmentNotFound(attachment_id\u003did)"},{"line_number":1662,"context_line":""},{"line_number":1663,"context_line":"        def fake_attachment_create(_self, context, volume_id, instance_uuid,"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_f56b78eb","line":1660,"range":{"start_line":1660,"start_character":15,"end_line":1660,"end_character":17},"in_reply_to":"5f7c97a3_2d57665b","updated":"2018-08-02 14:10:13.000000000","message":"Done","commit_id":"3d104d27ca1576b2609eba25ba4f7ee93baa54c2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5fe690db04e9f38cbca8f3f15bcf7782c3bb86fb","unresolved":false,"context_lines":[{"line_number":1666,"context_line":"            if self.attachment_error_id is not None:"},{"line_number":1667,"context_line":"                attachment_id \u003d self.attachment_error_id"},{"line_number":1668,"context_line":"            attachment \u003d {\u0027id\u0027: attachment_id, \u0027connection_info\u0027: {\u0027data\u0027: {}}}"},{"line_number":1669,"context_line":"            self.attachments[\u0027instance_uuid\u0027].append(instance_uuid)"},{"line_number":1670,"context_line":"            self.attachments[\u0027attachment_id\u0027].append(attachment_id)"},{"line_number":1671,"context_line":"            self.attachments[instance_uuid].append(volume_id)"},{"line_number":1672,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_edface41","line":1669,"range":{"start_line":1669,"start_character":12,"end_line":1669,"end_character":67},"updated":"2018-08-02 13:31:58.000000000","message":"I guess this is here just as a special place to stash the instance_uuid so we can look up the attachment to delete in fake_attachment_delete. Seems really goofy though, I don\u0027t know why we wouldn\u0027t just add an attachment_to_instance dict which would get populated in fake_attachment_create and then we can do that lookup to get the instance via the attachment_id in fake_attachment_delete.","commit_id":"3d104d27ca1576b2609eba25ba4f7ee93baa54c2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"83a8abe84c3ff58346cf6773e362579308bea05e","unresolved":false,"context_lines":[{"line_number":1666,"context_line":"            if self.attachment_error_id is not None:"},{"line_number":1667,"context_line":"                attachment_id \u003d self.attachment_error_id"},{"line_number":1668,"context_line":"            attachment \u003d {\u0027id\u0027: attachment_id, \u0027connection_info\u0027: {\u0027data\u0027: {}}}"},{"line_number":1669,"context_line":"            self.attachments[\u0027instance_uuid\u0027].append(instance_uuid)"},{"line_number":1670,"context_line":"            self.attachments[\u0027attachment_id\u0027].append(attachment_id)"},{"line_number":1671,"context_line":"            self.attachments[instance_uuid].append(volume_id)"},{"line_number":1672,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_5b205b9e","line":1669,"range":{"start_line":1669,"start_character":12,"end_line":1669,"end_character":67},"in_reply_to":"5f7c97a3_356a50ec","updated":"2018-08-02 14:39:13.000000000","message":"Yeah that\u0027s fine.","commit_id":"3d104d27ca1576b2609eba25ba4f7ee93baa54c2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b799191101e91af5491c2b117c73c11388c460d3","unresolved":false,"context_lines":[{"line_number":1666,"context_line":"            if self.attachment_error_id is not None:"},{"line_number":1667,"context_line":"                attachment_id \u003d self.attachment_error_id"},{"line_number":1668,"context_line":"            attachment \u003d {\u0027id\u0027: attachment_id, \u0027connection_info\u0027: {\u0027data\u0027: {}}}"},{"line_number":1669,"context_line":"            self.attachments[\u0027instance_uuid\u0027].append(instance_uuid)"},{"line_number":1670,"context_line":"            self.attachments[\u0027attachment_id\u0027].append(attachment_id)"},{"line_number":1671,"context_line":"            self.attachments[instance_uuid].append(volume_id)"},{"line_number":1672,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_356a50ec","line":1669,"range":{"start_line":1669,"start_character":12,"end_line":1669,"end_character":67},"in_reply_to":"5f7c97a3_edface41","updated":"2018-08-02 14:10:13.000000000","message":"Yup agreed, I did look at rewriting this but it ends up breaking existing regression tests that are asserting on the contents of attachments. I\u0027d rather not backport all of that to Queens so I\u0027ve left it as is for now.","commit_id":"3d104d27ca1576b2609eba25ba4f7ee93baa54c2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5fe690db04e9f38cbca8f3f15bcf7782c3bb86fb","unresolved":false,"context_lines":[{"line_number":1667,"context_line":"                attachment_id \u003d self.attachment_error_id"},{"line_number":1668,"context_line":"            attachment \u003d {\u0027id\u0027: attachment_id, \u0027connection_info\u0027: {\u0027data\u0027: {}}}"},{"line_number":1669,"context_line":"            self.attachments[\u0027instance_uuid\u0027].append(instance_uuid)"},{"line_number":1670,"context_line":"            self.attachments[\u0027attachment_id\u0027].append(attachment_id)"},{"line_number":1671,"context_line":"            self.attachments[instance_uuid].append(volume_id)"},{"line_number":1672,"context_line":""},{"line_number":1673,"context_line":"            return attachment"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_8da31a44","line":1670,"range":{"start_line":1670,"start_character":12,"end_line":1670,"end_character":67},"updated":"2018-08-02 13:31:58.000000000","message":"Since this isn\u0027t scoped to a particular instance, or volume, or host, couldn\u0027t we just add a list variable and call it something like self.attachment_record_ids? Then we don\u0027t need the fake attachment_id key.","commit_id":"3d104d27ca1576b2609eba25ba4f7ee93baa54c2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b799191101e91af5491c2b117c73c11388c460d3","unresolved":false,"context_lines":[{"line_number":1667,"context_line":"                attachment_id \u003d self.attachment_error_id"},{"line_number":1668,"context_line":"            attachment \u003d {\u0027id\u0027: attachment_id, \u0027connection_info\u0027: {\u0027data\u0027: {}}}"},{"line_number":1669,"context_line":"            self.attachments[\u0027instance_uuid\u0027].append(instance_uuid)"},{"line_number":1670,"context_line":"            self.attachments[\u0027attachment_id\u0027].append(attachment_id)"},{"line_number":1671,"context_line":"            self.attachments[instance_uuid].append(volume_id)"},{"line_number":1672,"context_line":""},{"line_number":1673,"context_line":"            return attachment"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_b57500cc","line":1670,"range":{"start_line":1670,"start_character":12,"end_line":1670,"end_character":67},"in_reply_to":"5f7c97a3_8da31a44","updated":"2018-08-02 14:10:13.000000000","message":"Done","commit_id":"3d104d27ca1576b2609eba25ba4f7ee93baa54c2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"07da78cf0d259c07500d4750cfbffa92fb7e352f","unresolved":false,"context_lines":[{"line_number":1676,"context_line":"            instance_uuid \u003d self.attachments[\u0027instance_uuid\u0027][0]"},{"line_number":1677,"context_line":"            del self.attachments[instance_uuid][0]"},{"line_number":1678,"context_line":"            del self.attachments[\u0027instance_uuid\u0027][0]"},{"line_number":1679,"context_line":"            del self.attachments[\u0027attachment_id\u0027][0]"},{"line_number":1680,"context_line":"            if attachment_id \u003d\u003d CinderFixtureNewAttachFlow.SWAP_ERR_ATTACH_ID:"},{"line_number":1681,"context_line":"                self.swap_error \u003d True"},{"line_number":1682,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_1b018304","line":1679,"updated":"2018-08-02 14:38:46.000000000","message":"This should probably call _check_attachment_id first?","commit_id":"3d104d27ca1576b2609eba25ba4f7ee93baa54c2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"089471ffa8b690d7759e5dff8f60c35d35eee2c3","unresolved":false,"context_lines":[{"line_number":1598,"context_line":"        super(CinderFixtureNewAttachFlow, self).setUp()"},{"line_number":1599,"context_line":""},{"line_number":1600,"context_line":"        def fake_get(self_api, context, volume_id, microversion\u003dNone):"},{"line_number":1601,"context_line":"            if volume_id in self.volume_to_attachment:"},{"line_number":1602,"context_line":"                attachment_id \u003d self.volume_to_attachment[volume_id]"},{"line_number":1603,"context_line":"            else:"},{"line_number":1604,"context_line":"                attachment_id \u003d volume_id"},{"line_number":1605,"context_line":"            # Check for the special swap volumes."},{"line_number":1606,"context_line":"            if volume_id in (CinderFixture.SWAP_OLD_VOL,"},{"line_number":1607,"context_line":"                             CinderFixture.SWAP_ERR_OLD_VOL):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_5a5ff5e4","line":1604,"range":{"start_line":1601,"start_character":12,"end_line":1604,"end_character":41},"updated":"2018-08-03 18:50:03.000000000","message":"nit: this could be simplified:\n\nattachment_id \u003d self.volume_to_attachment.get(volume_id, volume_id)","commit_id":"c6979297b8f3fd6eebdd6fd3e6ac06a82a15a042"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"089471ffa8b690d7759e5dff8f60c35d35eee2c3","unresolved":false,"context_lines":[{"line_number":1699,"context_line":"                attachment_id \u003d self.attachment_error_id"},{"line_number":1700,"context_line":"            attachment \u003d {\u0027id\u0027: attachment_id, \u0027connection_info\u0027: {\u0027data\u0027: {}}}"},{"line_number":1701,"context_line":"            self.attachments[\u0027instance_uuid\u0027].append(instance_uuid)"},{"line_number":1702,"context_line":"            self.attachments[\u0027attachment_id\u0027].append(attachment_id)"},{"line_number":1703,"context_line":"            self.volume_to_attachment[volume_id] \u003d attachment_id"},{"line_number":1704,"context_line":"            self.attachments[instance_uuid].append(volume_id)"},{"line_number":1705,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_da72257d","line":1702,"range":{"start_line":1702,"start_character":12,"end_line":1702,"end_character":67},"updated":"2018-08-03 18:50:03.000000000","message":"I guess I thought you were going to stop doing this. My comment from PS3:\n\n\u003e Since this isn\u0027t scoped to a particular instance, or volume, or host, couldn\u0027t we just add a list variable and call it something like self.attachment_record_ids? Then we don\u0027t need the fake attachment_id key.","commit_id":"c6979297b8f3fd6eebdd6fd3e6ac06a82a15a042"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a68b3f6560d3c9b217bc166288653474cc988968","unresolved":false,"context_lines":[{"line_number":1400,"context_line":"        self.attachments \u003d collections.defaultdict(list)"},{"line_number":1401,"context_line":""},{"line_number":1402,"context_line":"    def volumes_for_instance(self, instance_uuid):"},{"line_number":1403,"context_line":"        return self.attachments[instance_uuid]"},{"line_number":1404,"context_line":""},{"line_number":1405,"context_line":"    def setUp(self):"},{"line_number":1406,"context_line":"        super(CinderFixture, self).setUp()"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_380973dc","line":1403,"range":{"start_line":1403,"start_character":20,"end_line":1403,"end_character":31},"updated":"2018-08-09 16:38:39.000000000","message":"Use .get(instance_uuid) here to avoid a KeyError?","commit_id":"7d05b99e317185fdfd4878e31ed5b7fe807ca0b0"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c135f4ce17085ceaa4b05207f27c7b23cd55fd3c","unresolved":false,"context_lines":[{"line_number":1400,"context_line":"        self.attachments \u003d collections.defaultdict(list)"},{"line_number":1401,"context_line":""},{"line_number":1402,"context_line":"    def volumes_for_instance(self, instance_uuid):"},{"line_number":1403,"context_line":"        return self.attachments[instance_uuid]"},{"line_number":1404,"context_line":""},{"line_number":1405,"context_line":"    def setUp(self):"},{"line_number":1406,"context_line":"        super(CinderFixture, self).setUp()"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_8530c5b9","line":1403,"range":{"start_line":1403,"start_character":20,"end_line":1403,"end_character":31},"in_reply_to":"3f79a3b5_380973dc","updated":"2018-10-17 16:06:37.000000000","message":"Done","commit_id":"7d05b99e317185fdfd4878e31ed5b7fe807ca0b0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a68b3f6560d3c9b217bc166288653474cc988968","unresolved":false,"context_lines":[{"line_number":1601,"context_line":"        for volume_id, attachments in self.volume_to_attachment.items():"},{"line_number":1602,"context_line":"            for _, _instance_uuid in attachments:"},{"line_number":1603,"context_line":"                if _instance_uuid \u003d\u003d instance_uuid:"},{"line_number":1604,"context_line":"                    yield volume_id"},{"line_number":1605,"context_line":"                    break"},{"line_number":1606,"context_line":""},{"line_number":1607,"context_line":"    def setUp(self):"},{"line_number":1608,"context_line":"        super(CinderFixtureNewAttachFlow, self).setUp()"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_586eaf95","line":1605,"range":{"start_line":1604,"start_character":20,"end_line":1605,"end_character":25},"updated":"2018-08-09 16:38:39.000000000","message":"Weird, why not just return volume_id?","commit_id":"7d05b99e317185fdfd4878e31ed5b7fe807ca0b0"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b235983ec2d994ec03c42c4683ce7b31ac112fb9","unresolved":false,"context_lines":[{"line_number":1601,"context_line":"        for volume_id, attachments in self.volume_to_attachment.items():"},{"line_number":1602,"context_line":"            for _, _instance_uuid in attachments:"},{"line_number":1603,"context_line":"                if _instance_uuid \u003d\u003d instance_uuid:"},{"line_number":1604,"context_line":"                    yield volume_id"},{"line_number":1605,"context_line":"                    break"},{"line_number":1606,"context_line":""},{"line_number":1607,"context_line":"    def setUp(self):"},{"line_number":1608,"context_line":"        super(CinderFixtureNewAttachFlow, self).setUp()"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_e23c2c2e","line":1605,"range":{"start_line":1604,"start_character":20,"end_line":1605,"end_character":25},"in_reply_to":"3f79a3b5_586eaf95","updated":"2018-10-16 15:38:17.000000000","message":"I\u0027m guessing it\u0027s because he wants to return an iterator so he can do:\n\n  self.assertIn(volume_id, self.cinder.volumes_for_instance(server_id))\n\n...in the next file. Perhaps\n\n  return iter([volume_id])\n\nwould suffice?","commit_id":"7d05b99e317185fdfd4878e31ed5b7fe807ca0b0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e668b05f9b5c6e3a84c1c0a1184bf30eef4c9491","unresolved":false,"context_lines":[{"line_number":1601,"context_line":"        for volume_id, attachments in self.volume_to_attachment.items():"},{"line_number":1602,"context_line":"            for _, _instance_uuid in attachments:"},{"line_number":1603,"context_line":"                if _instance_uuid \u003d\u003d instance_uuid:"},{"line_number":1604,"context_line":"                    yield volume_id"},{"line_number":1605,"context_line":"                    break"},{"line_number":1606,"context_line":""},{"line_number":1607,"context_line":"    def setUp(self):"},{"line_number":1608,"context_line":"        super(CinderFixtureNewAttachFlow, self).setUp()"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_7dbb64bd","line":1605,"range":{"start_line":1604,"start_character":20,"end_line":1605,"end_character":25},"in_reply_to":"3f79a3b5_6097973b","updated":"2018-10-22 19:21:23.000000000","message":"So return a set of unique volume_ids?","commit_id":"7d05b99e317185fdfd4878e31ed5b7fe807ca0b0"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c135f4ce17085ceaa4b05207f27c7b23cd55fd3c","unresolved":false,"context_lines":[{"line_number":1601,"context_line":"        for volume_id, attachments in self.volume_to_attachment.items():"},{"line_number":1602,"context_line":"            for _, _instance_uuid in attachments:"},{"line_number":1603,"context_line":"                if _instance_uuid \u003d\u003d instance_uuid:"},{"line_number":1604,"context_line":"                    yield volume_id"},{"line_number":1605,"context_line":"                    break"},{"line_number":1606,"context_line":""},{"line_number":1607,"context_line":"    def setUp(self):"},{"line_number":1608,"context_line":"        super(CinderFixtureNewAttachFlow, self).setUp()"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_6097973b","line":1605,"range":{"start_line":1604,"start_character":20,"end_line":1605,"end_character":25},"in_reply_to":"3f79a3b5_e23c2c2e","updated":"2018-10-17 16:06:37.000000000","message":"On second thoughts, this is correct. We\u0027ve got two loops here, the first through volumes and the second through that volume\u0027s attachments. We\u0027re breaking out of the second loop because a volume might be attached to an instance twice and we don\u0027t want to return volume_id more than one","commit_id":"7d05b99e317185fdfd4878e31ed5b7fe807ca0b0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a68b3f6560d3c9b217bc166288653474cc988968","unresolved":false,"context_lines":[{"line_number":1718,"context_line":"            return attachment"},{"line_number":1719,"context_line":""},{"line_number":1720,"context_line":"        def fake_attachment_delete(_self, context, attachment_id):"},{"line_number":1721,"context_line":"            _, attachment, attachments \u003d _find_attachment(attachment_id)"},{"line_number":1722,"context_line":"            attachments.remove(attachment)"},{"line_number":1723,"context_line":""},{"line_number":1724,"context_line":"            if attachment_id \u003d\u003d CinderFixtureNewAttachFlow.SWAP_ERR_ATTACH_ID:"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_589f8f5e","line":1721,"range":{"start_line":1721,"start_character":15,"end_line":1721,"end_character":25},"updated":"2018-08-09 16:38:39.000000000","message":"nit: attachment_id","commit_id":"7d05b99e317185fdfd4878e31ed5b7fe807ca0b0"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c135f4ce17085ceaa4b05207f27c7b23cd55fd3c","unresolved":false,"context_lines":[{"line_number":1718,"context_line":"            return attachment"},{"line_number":1719,"context_line":""},{"line_number":1720,"context_line":"        def fake_attachment_delete(_self, context, attachment_id):"},{"line_number":1721,"context_line":"            _, attachment, attachments \u003d _find_attachment(attachment_id)"},{"line_number":1722,"context_line":"            attachments.remove(attachment)"},{"line_number":1723,"context_line":""},{"line_number":1724,"context_line":"            if attachment_id \u003d\u003d CinderFixtureNewAttachFlow.SWAP_ERR_ATTACH_ID:"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_00d6e355","line":1721,"range":{"start_line":1721,"start_character":15,"end_line":1721,"end_character":25},"in_reply_to":"3f79a3b5_589f8f5e","updated":"2018-10-17 16:06:37.000000000","message":"Done","commit_id":"7d05b99e317185fdfd4878e31ed5b7fe807ca0b0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a68b3f6560d3c9b217bc166288653474cc988968","unresolved":false,"context_lines":[{"line_number":1726,"context_line":""},{"line_number":1727,"context_line":"        def fake_attachment_update(_self, context, attachment_id, connector,"},{"line_number":1728,"context_line":"                                   mountpoint\u003dNone):"},{"line_number":1729,"context_line":"            _find_attachment(attachment_id)"},{"line_number":1730,"context_line":"            attachment_ref \u003d {\u0027driver_volume_type\u0027: \u0027fake_type\u0027,"},{"line_number":1731,"context_line":"                              \u0027id\u0027: attachment_id,"},{"line_number":1732,"context_line":"                              \u0027connection_info\u0027: {\u0027data\u0027:"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_f8877b0e","line":1729,"updated":"2018-08-09 16:38:39.000000000","message":"nit: add a comment that this is validating that the attachment exists","commit_id":"7d05b99e317185fdfd4878e31ed5b7fe807ca0b0"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c135f4ce17085ceaa4b05207f27c7b23cd55fd3c","unresolved":false,"context_lines":[{"line_number":1726,"context_line":""},{"line_number":1727,"context_line":"        def fake_attachment_update(_self, context, attachment_id, connector,"},{"line_number":1728,"context_line":"                                   mountpoint\u003dNone):"},{"line_number":1729,"context_line":"            _find_attachment(attachment_id)"},{"line_number":1730,"context_line":"            attachment_ref \u003d {\u0027driver_volume_type\u0027: \u0027fake_type\u0027,"},{"line_number":1731,"context_line":"                              \u0027id\u0027: attachment_id,"},{"line_number":1732,"context_line":"                              \u0027connection_info\u0027: {\u0027data\u0027:"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_60ca37a1","line":1729,"in_reply_to":"3f79a3b5_f8877b0e","updated":"2018-10-17 16:06:37.000000000","message":"Done","commit_id":"7d05b99e317185fdfd4878e31ed5b7fe807ca0b0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a68b3f6560d3c9b217bc166288653474cc988968","unresolved":false,"context_lines":[{"line_number":1737,"context_line":"            return attachment_ref"},{"line_number":1738,"context_line":""},{"line_number":1739,"context_line":"        def fake_attachment_get(_self, context, attachment_id):"},{"line_number":1740,"context_line":"            _find_attachment(attachment_id)"},{"line_number":1741,"context_line":"            attachment_ref \u003d {\u0027driver_volume_type\u0027: \u0027fake_type\u0027,"},{"line_number":1742,"context_line":"                              \u0027id\u0027: attachment_id,"},{"line_number":1743,"context_line":"                              \u0027connection_info\u0027: {\u0027data\u0027:"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_b8790330","line":1740,"updated":"2018-08-09 16:38:39.000000000","message":"same","commit_id":"7d05b99e317185fdfd4878e31ed5b7fe807ca0b0"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c135f4ce17085ceaa4b05207f27c7b23cd55fd3c","unresolved":false,"context_lines":[{"line_number":1737,"context_line":"            return attachment_ref"},{"line_number":1738,"context_line":""},{"line_number":1739,"context_line":"        def fake_attachment_get(_self, context, attachment_id):"},{"line_number":1740,"context_line":"            _find_attachment(attachment_id)"},{"line_number":1741,"context_line":"            attachment_ref \u003d {\u0027driver_volume_type\u0027: \u0027fake_type\u0027,"},{"line_number":1742,"context_line":"                              \u0027id\u0027: attachment_id,"},{"line_number":1743,"context_line":"                              \u0027connection_info\u0027: {\u0027data\u0027:"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_40c5bb8f","line":1740,"in_reply_to":"3f79a3b5_b8790330","updated":"2018-10-17 16:06:37.000000000","message":"Done","commit_id":"7d05b99e317185fdfd4878e31ed5b7fe807ca0b0"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8554d585cdc3b0869355ea385849b1df67f055ff","unresolved":false,"context_lines":[{"line_number":1403,"context_line":"        # This map gets updated on attach/detach operations."},{"line_number":1404,"context_line":"        self.attachments \u003d collections.defaultdict(list)"},{"line_number":1405,"context_line":""},{"line_number":1406,"context_line":"    def volumes_for_instance(self, instance_uuid):"},{"line_number":1407,"context_line":"        return self.attachments.get(instance_uuid)"},{"line_number":1408,"context_line":""},{"line_number":1409,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_a9da29e3","line":1406,"range":{"start_line":1406,"start_character":8,"end_line":1406,"end_character":15},"updated":"2018-10-22 15:02:58.000000000","message":"nit: volume_ids_for_instance","commit_id":"319f2669219c8c3100c218bb6ae0f7990fd644c3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"8d674258c3eaa371762dec9fac5adefd3b473773","unresolved":false,"context_lines":[{"line_number":1403,"context_line":"        # This map gets updated on attach/detach operations."},{"line_number":1404,"context_line":"        self.attachments \u003d collections.defaultdict(list)"},{"line_number":1405,"context_line":""},{"line_number":1406,"context_line":"    def volumes_for_instance(self, instance_uuid):"},{"line_number":1407,"context_line":"        return self.attachments.get(instance_uuid)"},{"line_number":1408,"context_line":""},{"line_number":1409,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_0fcc7d32","line":1406,"range":{"start_line":1406,"start_character":8,"end_line":1406,"end_character":15},"in_reply_to":"3f79a3b5_a9da29e3","updated":"2018-10-22 16:21:19.000000000","message":"Done","commit_id":"319f2669219c8c3100c218bb6ae0f7990fd644c3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8554d585cdc3b0869355ea385849b1df67f055ff","unresolved":false,"context_lines":[{"line_number":1595,"context_line":"        self.swap_volume_instance_uuid \u003d None"},{"line_number":1596,"context_line":"        self.swap_volume_instance_error_uuid \u003d None"},{"line_number":1597,"context_line":"        self.attachment_error_id \u003d None"},{"line_number":1598,"context_line":"        # A map of volumes to a list of (attachment_id, instance_uuid)"},{"line_number":1599,"context_line":"        # Note that a volume can have multiple attachments even without"},{"line_number":1600,"context_line":"        # multi-attach, as some flows create a blank \u0027reservation\u0027 attachment"},{"line_number":1601,"context_line":"        # before deleting another attachment."}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_6993b100","line":1598,"range":{"start_line":1598,"start_character":69,"end_line":1598,"end_character":70},"updated":"2018-10-22 15:02:58.000000000","message":"nit: ).","commit_id":"319f2669219c8c3100c218bb6ae0f7990fd644c3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"8d674258c3eaa371762dec9fac5adefd3b473773","unresolved":false,"context_lines":[{"line_number":1595,"context_line":"        self.swap_volume_instance_uuid \u003d None"},{"line_number":1596,"context_line":"        self.swap_volume_instance_error_uuid \u003d None"},{"line_number":1597,"context_line":"        self.attachment_error_id \u003d None"},{"line_number":1598,"context_line":"        # A map of volumes to a list of (attachment_id, instance_uuid)"},{"line_number":1599,"context_line":"        # Note that a volume can have multiple attachments even without"},{"line_number":1600,"context_line":"        # multi-attach, as some flows create a blank \u0027reservation\u0027 attachment"},{"line_number":1601,"context_line":"        # before deleting another attachment."}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_8f716d10","line":1598,"range":{"start_line":1598,"start_character":69,"end_line":1598,"end_character":70},"in_reply_to":"3f79a3b5_6993b100","updated":"2018-10-22 16:21:19.000000000","message":"Done","commit_id":"319f2669219c8c3100c218bb6ae0f7990fd644c3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8554d585cdc3b0869355ea385849b1df67f055ff","unresolved":false,"context_lines":[{"line_number":1601,"context_line":"        # before deleting another attachment."},{"line_number":1602,"context_line":"        self.volume_to_attachment \u003d collections.defaultdict(list)"},{"line_number":1603,"context_line":""},{"line_number":1604,"context_line":"    def volumes_for_instance(self, instance_uuid):"},{"line_number":1605,"context_line":"        for volume_id, attachments in self.volume_to_attachment.items():"},{"line_number":1606,"context_line":"            for _, _instance_uuid in attachments:"},{"line_number":1607,"context_line":"                if _instance_uuid \u003d\u003d instance_uuid:"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_29e85979","line":1604,"range":{"start_line":1604,"start_character":8,"end_line":1604,"end_character":15},"updated":"2018-10-22 15:02:58.000000000","message":"same nit","commit_id":"319f2669219c8c3100c218bb6ae0f7990fd644c3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"8d674258c3eaa371762dec9fac5adefd3b473773","unresolved":false,"context_lines":[{"line_number":1601,"context_line":"        # before deleting another attachment."},{"line_number":1602,"context_line":"        self.volume_to_attachment \u003d collections.defaultdict(list)"},{"line_number":1603,"context_line":""},{"line_number":1604,"context_line":"    def volumes_for_instance(self, instance_uuid):"},{"line_number":1605,"context_line":"        for volume_id, attachments in self.volume_to_attachment.items():"},{"line_number":1606,"context_line":"            for _, _instance_uuid in attachments:"},{"line_number":1607,"context_line":"                if _instance_uuid \u003d\u003d instance_uuid:"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_6fc5f108","line":1604,"range":{"start_line":1604,"start_character":8,"end_line":1604,"end_character":15},"in_reply_to":"3f79a3b5_29e85979","updated":"2018-10-22 16:21:19.000000000","message":"Done","commit_id":"319f2669219c8c3100c218bb6ae0f7990fd644c3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8554d585cdc3b0869355ea385849b1df67f055ff","unresolved":false,"context_lines":[{"line_number":1636,"context_line":"                        else self.swap_volume_instance_error_uuid)"},{"line_number":1637,"context_line":""},{"line_number":1638,"context_line":"                    if attachments:"},{"line_number":1639,"context_line":"                        attachment_id, instance_uuid \u003d attachments[0]"},{"line_number":1640,"context_line":""},{"line_number":1641,"context_line":"                        volume.update({"},{"line_number":1642,"context_line":"                            \u0027status\u0027: \u0027in-use\u0027,"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_293939f0","line":1639,"range":{"start_line":1639,"start_character":66,"end_line":1639,"end_character":69},"updated":"2018-10-22 15:02:58.000000000","message":"not sure why we don\u0027t handle there being more than one here, but i guess it doesn\u0027t matter for existing tests","commit_id":"319f2669219c8c3100c218bb6ae0f7990fd644c3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8554d585cdc3b0869355ea385849b1df67f055ff","unresolved":false,"context_lines":[{"line_number":1725,"context_line":""},{"line_number":1726,"context_line":"        def fake_attachment_delete(_self, context, attachment_id):"},{"line_number":1727,"context_line":"            _, _attachment_id, attachments \u003d _find_attachment(attachment_id)"},{"line_number":1728,"context_line":"            attachments.remove(_attachment_id)"},{"line_number":1729,"context_line":""},{"line_number":1730,"context_line":"            if attachment_id \u003d\u003d CinderFixtureNewAttachFlow.SWAP_ERR_ATTACH_ID:"},{"line_number":1731,"context_line":"                self.swap_error \u003d True"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_742624bc","line":1728,"range":{"start_line":1728,"start_character":31,"end_line":1728,"end_character":45},"updated":"2018-10-22 15:02:58.000000000","message":"How does this work? Isn\u0027t attachments a list of 2-item tuples?","commit_id":"319f2669219c8c3100c218bb6ae0f7990fd644c3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"28da4e6fce378dfd04d226d1c3ed5e21c514b184","unresolved":false,"context_lines":[{"line_number":1725,"context_line":""},{"line_number":1726,"context_line":"        def fake_attachment_delete(_self, context, attachment_id):"},{"line_number":1727,"context_line":"            _, _attachment_id, attachments \u003d _find_attachment(attachment_id)"},{"line_number":1728,"context_line":"            attachments.remove(_attachment_id)"},{"line_number":1729,"context_line":""},{"line_number":1730,"context_line":"            if attachment_id \u003d\u003d CinderFixtureNewAttachFlow.SWAP_ERR_ATTACH_ID:"},{"line_number":1731,"context_line":"                self.swap_error \u003d True"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_4f26f524","line":1728,"range":{"start_line":1728,"start_character":31,"end_line":1728,"end_character":45},"in_reply_to":"3f79a3b5_742624bc","updated":"2018-10-22 16:07:11.000000000","message":"Oh I guess that is the 2-item tuple because of line 1711 returning the \"attachment\" variable, which is actually the tuple. Really confusing. We shouldn\u0027t call this variable attachment_id - I\u0027d rather _find_attachment return a 4-item tuple of:\n\nvolume_id, attachment_id, instance_uuid, attachments\n\nOr rename the _attachment_id variable here to something more obviously, like attachment_tuple or something.","commit_id":"319f2669219c8c3100c218bb6ae0f7990fd644c3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"addacd3a9e980e78e39a487fbc012f6ce113ead7","unresolved":false,"context_lines":[{"line_number":1725,"context_line":""},{"line_number":1726,"context_line":"        def fake_attachment_delete(_self, context, attachment_id):"},{"line_number":1727,"context_line":"            _, _attachment_id, attachments \u003d _find_attachment(attachment_id)"},{"line_number":1728,"context_line":"            attachments.remove(_attachment_id)"},{"line_number":1729,"context_line":""},{"line_number":1730,"context_line":"            if attachment_id \u003d\u003d CinderFixtureNewAttachFlow.SWAP_ERR_ATTACH_ID:"},{"line_number":1731,"context_line":"                self.swap_error \u003d True"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_efefc1af","line":1728,"range":{"start_line":1728,"start_character":31,"end_line":1728,"end_character":45},"in_reply_to":"3f79a3b5_742624bc","updated":"2018-10-22 16:08:32.000000000","message":"This is \u0027_attachment_id\u0027, i.e. the second item of the result tuple returned from \u0027_find_attachment\u0027. As seen above [1][2], this actually a two item tuple. This used to be called \u0027attachment\u0027 [3] and shouldn\u0027t have been renamed. I can undo that.\n\n[1] https://review.openstack.org/#/c/587013/15/nova/tests/fixtures.py@1711\n[2] https://review.openstack.org/#/c/587013/15/nova/tests/fixtures.py@1709\n[3] https://review.openstack.org/#/c/587013/12/nova/tests/fixtures.py@1721","commit_id":"319f2669219c8c3100c218bb6ae0f7990fd644c3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e668b05f9b5c6e3a84c1c0a1184bf30eef4c9491","unresolved":false,"context_lines":[{"line_number":1607,"context_line":"                if _instance_uuid \u003d\u003d instance_uuid:"},{"line_number":1608,"context_line":"                    # we might have multiple volumes attached to this instance"},{"line_number":1609,"context_line":"                    # so yield rather than return"},{"line_number":1610,"context_line":"                    yield volume_id"},{"line_number":1611,"context_line":"                    break"},{"line_number":1612,"context_line":""},{"line_number":1613,"context_line":"    def setUp(self):"},{"line_number":1614,"context_line":"        super(CinderFixtureNewAttachFlow, self).setUp()"}],"source_content_type":"text/x-python","patch_set":16,"id":"3f79a3b5_9d9d205a","line":1611,"range":{"start_line":1610,"start_character":20,"end_line":1611,"end_character":25},"updated":"2018-10-22 19:21:23.000000000","message":"I still don\u0027t really understand why this yields rather than return a list of volume_ids? I know it came up before:\n\nhttps://review.openstack.org/#/c/587013/12/nova/tests/fixtures.py@1605\n\nBut couldn\u0027t we just return a set?","commit_id":"bfdc6a0d5293e29d91d4755370c08769394f3ba7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8295072de15322e3bf180ae0adbce92cb83edfff","unresolved":false,"context_lines":[{"line_number":1607,"context_line":"                if _instance_uuid \u003d\u003d instance_uuid:"},{"line_number":1608,"context_line":"                    # we might have multiple volumes attached to this instance"},{"line_number":1609,"context_line":"                    # so yield rather than return"},{"line_number":1610,"context_line":"                    yield volume_id"},{"line_number":1611,"context_line":"                    break"},{"line_number":1612,"context_line":""},{"line_number":1613,"context_line":"    def setUp(self):"},{"line_number":1614,"context_line":"        super(CinderFixtureNewAttachFlow, self).setUp()"}],"source_content_type":"text/x-python","patch_set":16,"id":"3f79a3b5_bd4b3cb0","line":1611,"range":{"start_line":1610,"start_character":20,"end_line":1611,"end_character":25},"in_reply_to":"3f79a3b5_9d9d205a","updated":"2018-10-22 19:25:25.000000000","message":"Nevermind a set isn\u0027t really helpful since volume_to_attachment is already keyed by volume_id.","commit_id":"bfdc6a0d5293e29d91d4755370c08769394f3ba7"}]}