)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"767f18ab2bd54590c348415e6330f3be93aee89a","unresolved":false,"context_lines":[{"line_number":4911,"context_line":"            # At this point we need to update the new volume attachment with"},{"line_number":4912,"context_line":"            # the host connector, which will give us back the new attachment"},{"line_number":4913,"context_line":"            # connection_info."},{"line_number":4914,"context_line":"            new_cinfo \u003d self.volume_api.attachment_update("},{"line_number":4915,"context_line":"                context, new_attachment_id, connector)"},{"line_number":4916,"context_line":""},{"line_number":4917,"context_line":"        old_cinfo \u003d jsonutils.loads(bdm[\u0027connection_info\u0027])"},{"line_number":4918,"context_line":"        if old_cinfo and \u0027serial\u0027 not in old_cinfo:"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_ede4a9ae","line":4915,"range":{"start_line":4914,"start_character":12,"end_line":4915,"end_character":54},"updated":"2017-07-25 01:43:01.000000000","message":"This is wrong because attachment_update is not returning a connection_info dict, it\u0027s returning a full attachment reference, so this should be:\n\nnew_cinfo \u003d self.volume_api.attachment_update(\n                context, new_attachment_id, connector)[\u0027connection_info\u0027]","commit_id":"552b38a7f99c02418198313c409ae8293b4f5446"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"f7922b1aedd09a3dc895a15697444b80bbaa7a24","unresolved":false,"context_lines":[{"line_number":4911,"context_line":"            # At this point we need to update the new volume attachment with"},{"line_number":4912,"context_line":"            # the host connector, which will give us back the new attachment"},{"line_number":4913,"context_line":"            # connection_info."},{"line_number":4914,"context_line":"            new_cinfo \u003d self.volume_api.attachment_update("},{"line_number":4915,"context_line":"                context, new_attachment_id, connector)"},{"line_number":4916,"context_line":""},{"line_number":4917,"context_line":"        old_cinfo \u003d jsonutils.loads(bdm[\u0027connection_info\u0027])"},{"line_number":4918,"context_line":"        if old_cinfo and \u0027serial\u0027 not in old_cinfo:"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_ba20f5b1","line":4915,"range":{"start_line":4914,"start_character":12,"end_line":4915,"end_character":54},"in_reply_to":"ff346bd7_ede4a9ae","updated":"2017-07-25 20:14:09.000000000","message":"Done","commit_id":"552b38a7f99c02418198313c409ae8293b4f5446"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8433079cb1d733dd2f79904706ae0591580a39b1","unresolved":false,"context_lines":[{"line_number":2223,"context_line":"        new_volume \u003d {"},{"line_number":2224,"context_line":"            \u0027id\u0027: uuids.new_volume_id, \u0027size\u0027: 2, \u0027status\u0027: \u0027reserved\u0027"},{"line_number":2225,"context_line":"        }"},{"line_number":2226,"context_line":"        attachment_update.return_volume \u003d \\"},{"line_number":2227,"context_line":"            \u0027{\"attachment_id\": \"fake\", {\"connection_info\": {\"\"data\": {}}}\u0027"},{"line_number":2228,"context_line":"        get_bdm.return_value \u003d bdm"},{"line_number":2229,"context_line":"        get_volume.side_effect \u003d (old_volume, new_volume)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_e1f6c9f9","line":2226,"range":{"start_line":2226,"start_character":26,"end_line":2226,"end_character":39},"updated":"2017-07-26 00:51:45.000000000","message":"return_value?\n\nSomething is clearly not working here - being asserted that is.","commit_id":"fd293d859fc198c8137cda468d62c3d0e1dc8088"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d9f0ceeebc7de8beb297231d73569660ef64c8c","unresolved":false,"context_lines":[{"line_number":2223,"context_line":"        new_volume \u003d {"},{"line_number":2224,"context_line":"            \u0027id\u0027: uuids.new_volume_id, \u0027size\u0027: 2, \u0027status\u0027: \u0027reserved\u0027"},{"line_number":2225,"context_line":"        }"},{"line_number":2226,"context_line":"        attachment_update.return_volume \u003d \\"},{"line_number":2227,"context_line":"            \u0027{\"attachment_id\": \"fake\", {\"connection_info\": {\"\"data\": {}}}\u0027"},{"line_number":2228,"context_line":"        get_bdm.return_value \u003d bdm"},{"line_number":2229,"context_line":"        get_volume.side_effect \u003d (old_volume, new_volume)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_e1b5a9ce","line":2226,"range":{"start_line":2226,"start_character":26,"end_line":2226,"end_character":39},"in_reply_to":"ff346bd7_e1f6c9f9","updated":"2017-07-26 00:59:41.000000000","message":"Oh I know why this passes. The return_value is called and returns a MagicMock by default, which handles the dict-like stuff so nothing blows up.","commit_id":"fd293d859fc198c8137cda468d62c3d0e1dc8088"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8433079cb1d733dd2f79904706ae0591580a39b1","unresolved":false,"context_lines":[{"line_number":2224,"context_line":"            \u0027id\u0027: uuids.new_volume_id, \u0027size\u0027: 2, \u0027status\u0027: \u0027reserved\u0027"},{"line_number":2225,"context_line":"        }"},{"line_number":2226,"context_line":"        attachment_update.return_volume \u003d \\"},{"line_number":2227,"context_line":"            \u0027{\"attachment_id\": \"fake\", {\"connection_info\": {\"\"data\": {}}}\u0027"},{"line_number":2228,"context_line":"        get_bdm.return_value \u003d bdm"},{"line_number":2229,"context_line":"        get_volume.side_effect \u003d (old_volume, new_volume)"},{"line_number":2230,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_61e2d931","line":2227,"range":{"start_line":2227,"start_character":12,"end_line":2227,"end_character":74},"updated":"2017-07-26 00:51:45.000000000","message":"Why is this a json string?","commit_id":"fd293d859fc198c8137cda468d62c3d0e1dc8088"}],"nova/tests/unit/volume/test_cinder.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8433079cb1d733dd2f79904706ae0591580a39b1","unresolved":false,"context_lines":[{"line_number":89,"context_line":"                    \u0027connection_info\u0027: self.connection_info}"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    def get(self, key, default\u003dNone):"},{"line_number":92,"context_line":"        return self.att[key]"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def __setitem__(self, key, value):"},{"line_number":95,"context_line":"        self.att[key] \u003d value"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_210971dc","line":92,"range":{"start_line":92,"start_character":15,"end_line":92,"end_character":28},"updated":"2017-07-26 00:51:45.000000000","message":"return self.att.get(key, default) ?","commit_id":"fd293d859fc198c8137cda468d62c3d0e1dc8088"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8433079cb1d733dd2f79904706ae0591580a39b1","unresolved":false,"context_lines":[{"line_number":338,"context_line":"    def test_attachment_update(self, mock_cinderclient):"},{"line_number":339,"context_line":"        \"\"\"Tests the happy path for updating a volume attachment.\"\"\""},{"line_number":340,"context_line":"        fake_attachment \u003d FakeAttachment()"},{"line_number":341,"context_line":"        expected_attachment_ref \u003d {\u0027id\u0027: uuids.attachment_id,"},{"line_number":342,"context_line":"            \u0027status\u0027: \u0027attached\u0027,"},{"line_number":343,"context_line":"            \u0027instance\u0027: uuids.instance_uuid,"},{"line_number":344,"context_line":"            \u0027volume_id\u0027: uuids.volume_id,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_0142ed10","line":341,"range":{"start_line":341,"start_character":35,"end_line":341,"end_character":60},"updated":"2017-07-26 00:51:45.000000000","message":"nit: drop this a line to align with the other keys","commit_id":"fd293d859fc198c8137cda468d62c3d0e1dc8088"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8433079cb1d733dd2f79904706ae0591580a39b1","unresolved":false,"context_lines":[{"line_number":340,"context_line":"        fake_attachment \u003d FakeAttachment()"},{"line_number":341,"context_line":"        expected_attachment_ref \u003d {\u0027id\u0027: uuids.attachment_id,"},{"line_number":342,"context_line":"            \u0027status\u0027: \u0027attached\u0027,"},{"line_number":343,"context_line":"            \u0027instance\u0027: uuids.instance_uuid,"},{"line_number":344,"context_line":"            \u0027volume_id\u0027: uuids.volume_id,"},{"line_number":345,"context_line":"            \u0027attached_at\u0027: fake_attachment.attached_at,"},{"line_number":346,"context_line":"            \u0027detached_at\u0027: None,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff346bd7_8155bdc8","line":343,"range":{"start_line":343,"start_character":24,"end_line":343,"end_character":43},"updated":"2017-07-26 00:51:45.000000000","message":"Just use fake_attachment.instance?","commit_id":"fd293d859fc198c8137cda468d62c3d0e1dc8088"}],"nova/volume/cinder.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f0103f15542ac0c57eba8942450c7d0ca5f43005","unresolved":false,"context_lines":[{"line_number":213,"context_line":"    return d"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"def _translate_attachment_ref(attachment_ref):"},{"line_number":217,"context_line":"    \"\"\"Building old style connection_info by adding the \u0027data\u0027 key back.\"\"\""},{"line_number":218,"context_line":"    data_keys \u003d attachment_ref.pop(\u0027connection_info\u0027, {})"},{"line_number":219,"context_line":"    if data_keys.get(\u0027driver_volume_type\u0027, None):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_04d23b5d","line":216,"range":{"start_line":216,"start_character":30,"end_line":216,"end_character":44},"updated":"2017-07-24 13:59:29.000000000","message":"This is not a connection_info dict, this is a VolumeAttachment object, correct? And if it is, then why are we setting a driver_volume_type field on it? And the \u0027data\u0027 key? Shouldn\u0027t we be updating VolumeAttachment.connection_info which is in the attachment_ref?","commit_id":"552b38a7f99c02418198313c409ae8293b4f5446"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"1c1ca5201e5546cd1655b6b7aa2415c6a5e8ea7a","unresolved":false,"context_lines":[{"line_number":213,"context_line":"    return d"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"def _translate_attachment_ref(attachment_ref):"},{"line_number":217,"context_line":"    \"\"\"Building old style connection_info by adding the \u0027data\u0027 key back.\"\"\""},{"line_number":218,"context_line":"    data_keys \u003d attachment_ref.pop(\u0027connection_info\u0027, {})"},{"line_number":219,"context_line":"    if data_keys.get(\u0027driver_volume_type\u0027, None):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_9ffd553c","line":216,"range":{"start_line":216,"start_character":30,"end_line":216,"end_character":44},"in_reply_to":"ff346bd7_04d23b5d","updated":"2017-07-24 15:54:25.000000000","message":"Added a comment on the object concern below.\n\nThe \u0027data\u0027 key is not part of connection_info but rather it contains all the info that the \u0027data\u0027 key earlier plus everything else.\n\nWe know that the driver_volume_type is on the connection_info level and not inside \u0027data\u0027, that\u0027s why we are handling it separately here. All the other things go under the \u0027data\u0027 key.","commit_id":"552b38a7f99c02418198313c409ae8293b4f5446"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8bbd45f3a7e13788580b7b1d73e69980eb49884e","unresolved":false,"context_lines":[{"line_number":215,"context_line":""},{"line_number":216,"context_line":"def _translate_attachment_ref(attachment_ref):"},{"line_number":217,"context_line":"    \"\"\"Building old style connection_info by adding the \u0027data\u0027 key back.\"\"\""},{"line_number":218,"context_line":"    data_keys \u003d attachment_ref.pop(\u0027connection_info\u0027, {})"},{"line_number":219,"context_line":"    if data_keys.get(\u0027driver_volume_type\u0027, None):"},{"line_number":220,"context_line":"        attachment_ref[\u0027driver_volume_type\u0027] \u003d data_keys[\u0027driver_volume_type\u0027]"},{"line_number":221,"context_line":"    attachment_ref[\u0027data\u0027] \u003d {}"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_2d2871e0","line":218,"range":{"start_line":218,"start_character":4,"end_line":218,"end_character":13},"updated":"2017-07-25 01:41:32.000000000","message":"Can we please call this connection_info? The \u0027data_keys\u0027 thing was really confusing me.","commit_id":"552b38a7f99c02418198313c409ae8293b4f5446"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f0103f15542ac0c57eba8942450c7d0ca5f43005","unresolved":false,"context_lines":[{"line_number":216,"context_line":"def _translate_attachment_ref(attachment_ref):"},{"line_number":217,"context_line":"    \"\"\"Building old style connection_info by adding the \u0027data\u0027 key back.\"\"\""},{"line_number":218,"context_line":"    data_keys \u003d attachment_ref.pop(\u0027connection_info\u0027, {})"},{"line_number":219,"context_line":"    if data_keys.get(\u0027driver_volume_type\u0027, None):"},{"line_number":220,"context_line":"        attachment_ref[\u0027driver_volume_type\u0027] \u003d data_keys[\u0027driver_volume_type\u0027]"},{"line_number":221,"context_line":"    attachment_ref[\u0027data\u0027] \u003d {}"},{"line_number":222,"context_line":"    for k, v in data_keys.items():"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_a4824f4e","line":219,"range":{"start_line":219,"start_character":41,"end_line":219,"end_character":47},"updated":"2017-07-24 13:59:29.000000000","message":"nit: defaulting to None is redundant.","commit_id":"552b38a7f99c02418198313c409ae8293b4f5446"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8bbd45f3a7e13788580b7b1d73e69980eb49884e","unresolved":false,"context_lines":[{"line_number":217,"context_line":"    \"\"\"Building old style connection_info by adding the \u0027data\u0027 key back.\"\"\""},{"line_number":218,"context_line":"    data_keys \u003d attachment_ref.pop(\u0027connection_info\u0027, {})"},{"line_number":219,"context_line":"    if data_keys.get(\u0027driver_volume_type\u0027, None):"},{"line_number":220,"context_line":"        attachment_ref[\u0027driver_volume_type\u0027] \u003d data_keys[\u0027driver_volume_type\u0027]"},{"line_number":221,"context_line":"    attachment_ref[\u0027data\u0027] \u003d {}"},{"line_number":222,"context_line":"    for k, v in data_keys.items():"},{"line_number":223,"context_line":"        if k !\u003d \"driver_volume_type\":"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_4d8615de","line":220,"range":{"start_line":220,"start_character":8,"end_line":220,"end_character":44},"updated":"2017-07-25 01:41:32.000000000","message":"What is going to use this at the top level? Why not just leave it part of the connection_info top level?\n\nSo if you start with something like this:\n\nattachment: {\n  connection_info: {\n    \u0027driver_volume_type\u0027: \u0027rbd\u0027,\n    \u0027foo\u0027: \u0027bar\u0027,\n    \u0027volume_id\u0027: \u00271234\u0027\n  }\n}\n\nAnd we want to turn it into:\n\nattachment: {\n  connection_info: {\n    \u0027driver_volume_type\u0027: \u0027rbd\u0027,\n    \u0027data\u0027: {\n      \u0027foo\u0027: \u0027bar\u0027,\n      \u0027volume_id\u0027: \u00271234\u0027\n    }\n  }\n}\n\nWhy can\u0027t we just do that? Because that\u0027s exactly how we use it in nova:\n\nhttps://github.com/openstack/nova/blob/master/nova/virt/libvirt/volume/net.py#L108","commit_id":"552b38a7f99c02418198313c409ae8293b4f5446"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"f7922b1aedd09a3dc895a15697444b80bbaa7a24","unresolved":false,"context_lines":[{"line_number":217,"context_line":"    \"\"\"Building old style connection_info by adding the \u0027data\u0027 key back.\"\"\""},{"line_number":218,"context_line":"    data_keys \u003d attachment_ref.pop(\u0027connection_info\u0027, {})"},{"line_number":219,"context_line":"    if data_keys.get(\u0027driver_volume_type\u0027, None):"},{"line_number":220,"context_line":"        attachment_ref[\u0027driver_volume_type\u0027] \u003d data_keys[\u0027driver_volume_type\u0027]"},{"line_number":221,"context_line":"    attachment_ref[\u0027data\u0027] \u003d {}"},{"line_number":222,"context_line":"    for k, v in data_keys.items():"},{"line_number":223,"context_line":"        if k !\u003d \"driver_volume_type\":"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_3a3525f8","line":220,"range":{"start_line":220,"start_character":8,"end_line":220,"end_character":44},"in_reply_to":"ff346bd7_0c234faa","updated":"2017-07-25 20:14:09.000000000","message":"Done","commit_id":"552b38a7f99c02418198313c409ae8293b4f5446"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"06e645554873a66025b065c7f81d5d9e54f87dd5","unresolved":false,"context_lines":[{"line_number":217,"context_line":"    \"\"\"Building old style connection_info by adding the \u0027data\u0027 key back.\"\"\""},{"line_number":218,"context_line":"    data_keys \u003d attachment_ref.pop(\u0027connection_info\u0027, {})"},{"line_number":219,"context_line":"    if data_keys.get(\u0027driver_volume_type\u0027, None):"},{"line_number":220,"context_line":"        attachment_ref[\u0027driver_volume_type\u0027] \u003d data_keys[\u0027driver_volume_type\u0027]"},{"line_number":221,"context_line":"    attachment_ref[\u0027data\u0027] \u003d {}"},{"line_number":222,"context_line":"    for k, v in data_keys.items():"},{"line_number":223,"context_line":"        if k !\u003d \"driver_volume_type\":"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_a90e752f","line":220,"range":{"start_line":220,"start_character":8,"end_line":220,"end_character":44},"in_reply_to":"ff346bd7_4d8615de","updated":"2017-07-25 17:55:18.000000000","message":"We do this above: attachment_ref.pop(\u0027connection_info\u0027, {})\n\nSo attachment_ref is basically turned into a connection_info dict with having \u0027driver_volume_type\u0027 top level and everything that ended up in the all in one connection_info dict lands under \u0027data\u0027. Naming is really confusing here, I\u0027m sorry about that.\n\nWould that work if we do a better naming and add a docstring here and keep returning the data structure from this one that Nova cares about by which I mean the connection_info dict?","commit_id":"552b38a7f99c02418198313c409ae8293b4f5446"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ad0f6827f8fd10e3df0bc6e8e2b4fb2032b52858","unresolved":false,"context_lines":[{"line_number":217,"context_line":"    \"\"\"Building old style connection_info by adding the \u0027data\u0027 key back.\"\"\""},{"line_number":218,"context_line":"    data_keys \u003d attachment_ref.pop(\u0027connection_info\u0027, {})"},{"line_number":219,"context_line":"    if data_keys.get(\u0027driver_volume_type\u0027, None):"},{"line_number":220,"context_line":"        attachment_ref[\u0027driver_volume_type\u0027] \u003d data_keys[\u0027driver_volume_type\u0027]"},{"line_number":221,"context_line":"    attachment_ref[\u0027data\u0027] \u003d {}"},{"line_number":222,"context_line":"    for k, v in data_keys.items():"},{"line_number":223,"context_line":"        if k !\u003d \"driver_volume_type\":"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_0c234faa","line":220,"range":{"start_line":220,"start_character":8,"end_line":220,"end_character":44},"in_reply_to":"ff346bd7_a90e752f","updated":"2017-07-25 18:26:49.000000000","message":"Correct, I understand now, but I don\u0027t like munging the attachment_ref[\u0027connection_info\u0027] into the attachment_ref itself. As a user of the volume API code below, I want to get an attachment_ref dict back, and know that there is a connection_info key within it and it has the same structure that the rest of the Nova code is used to, which is the top-level driver_volume_type and data keys.","commit_id":"552b38a7f99c02418198313c409ae8293b4f5446"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f0103f15542ac0c57eba8942450c7d0ca5f43005","unresolved":false,"context_lines":[{"line_number":550,"context_line":"        try:"},{"line_number":551,"context_line":"            attachment_ref \u003d cinderclient(context, \u00273.27\u0027).attachments.create("},{"line_number":552,"context_line":"                volume_id, connector, instance_id)"},{"line_number":553,"context_line":"            return _translate_attachment_ref(attachment_ref)"},{"line_number":554,"context_line":"        except cinder_exception.ClientException as ex:"},{"line_number":555,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":556,"context_line":"                LOG.error((\u0027Create attachment failed for volume \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_84dceb74","line":553,"range":{"start_line":553,"start_character":45,"end_line":553,"end_character":59},"updated":"2017-07-24 13:59:29.000000000","message":"This is a VolumeAttachment object, yes?","commit_id":"552b38a7f99c02418198313c409ae8293b4f5446"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"1c1ca5201e5546cd1655b6b7aa2415c6a5e8ea7a","unresolved":false,"context_lines":[{"line_number":550,"context_line":"        try:"},{"line_number":551,"context_line":"            attachment_ref \u003d cinderclient(context, \u00273.27\u0027).attachments.create("},{"line_number":552,"context_line":"                volume_id, connector, instance_id)"},{"line_number":553,"context_line":"            return _translate_attachment_ref(attachment_ref)"},{"line_number":554,"context_line":"        except cinder_exception.ClientException as ex:"},{"line_number":555,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":556,"context_line":"                LOG.error((\u0027Create attachment failed for volume \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_9ff3159f","line":553,"range":{"start_line":553,"start_character":45,"end_line":553,"end_character":59},"in_reply_to":"ff346bd7_84dceb74","updated":"2017-07-24 15:54:25.000000000","message":"It was always a dict. In the early days I was mistaken it for object, but Cinder always sends a dict in case of this function.","commit_id":"552b38a7f99c02418198313c409ae8293b4f5446"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8bbd45f3a7e13788580b7b1d73e69980eb49884e","unresolved":false,"context_lines":[{"line_number":550,"context_line":"        try:"},{"line_number":551,"context_line":"            attachment_ref \u003d cinderclient(context, \u00273.27\u0027).attachments.create("},{"line_number":552,"context_line":"                volume_id, connector, instance_id)"},{"line_number":553,"context_line":"            return _translate_attachment_ref(attachment_ref)"},{"line_number":554,"context_line":"        except cinder_exception.ClientException as ex:"},{"line_number":555,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":556,"context_line":"                LOG.error((\u0027Create attachment failed for volume \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_2da3d1a0","line":553,"range":{"start_line":553,"start_character":45,"end_line":553,"end_character":59},"in_reply_to":"ff346bd7_9ff3159f","updated":"2017-07-25 01:41:32.000000000","message":"OK so here:\n\nhttps://github.com/openstack/python-cinderclient/blob/master/cinderclient/v3/attachments.py#L34\n\nAnd the REST API returns this:\n\nhttps://github.com/openstack/cinder/blob/master/cinder/api/v3/views/attachments.py#L38\n\nWhich has a connection_info entry in it, which is what should get translated to the old style that nova understands in the helper method.\n\nWhat I don\u0027t understand is why the _translate_attachment_ref method isn\u0027t just working on the attachment_ref[\u0027connection_info\u0027] field.\n\n(later)\n\nnvm, I guess it is:\n\ndata_keys \u003d attachment_ref.pop(\u0027connection_info\u0027, {})","commit_id":"552b38a7f99c02418198313c409ae8293b4f5446"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f0103f15542ac0c57eba8942450c7d0ca5f43005","unresolved":false,"context_lines":[{"line_number":570,"context_line":"        :param connector: host connector dict. This is required when updating"},{"line_number":571,"context_line":"            a volume attachment. To terminate a connection, the volume"},{"line_number":572,"context_line":"            attachment for that connection must be deleted."},{"line_number":573,"context_line":"        :returns: connection_info dict, built from the"},{"line_number":574,"context_line":"            cinderclient.v3.attachments.VolumeAttachment object (this"},{"line_number":575,"context_line":"            gives us a backward compatible connection_info dict)"},{"line_number":576,"context_line":"        \"\"\""},{"line_number":577,"context_line":"        try:"},{"line_number":578,"context_line":"            attachment_ref \u003d cinderclient("}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_049b5bc9","line":575,"range":{"start_line":573,"start_character":8,"end_line":575,"end_character":64},"updated":"2017-07-24 13:59:29.000000000","message":"Rather than changing the return type on this method, can\u0027t we just fix the connection_info that\u0027s returned in the VolumeAttachment object? Then the calling code in the compute manager doesn\u0027t need to change at all.","commit_id":"552b38a7f99c02418198313c409ae8293b4f5446"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"1c1ca5201e5546cd1655b6b7aa2415c6a5e8ea7a","unresolved":false,"context_lines":[{"line_number":570,"context_line":"        :param connector: host connector dict. This is required when updating"},{"line_number":571,"context_line":"            a volume attachment. To terminate a connection, the volume"},{"line_number":572,"context_line":"            attachment for that connection must be deleted."},{"line_number":573,"context_line":"        :returns: connection_info dict, built from the"},{"line_number":574,"context_line":"            cinderclient.v3.attachments.VolumeAttachment object (this"},{"line_number":575,"context_line":"            gives us a backward compatible connection_info dict)"},{"line_number":576,"context_line":"        \"\"\""},{"line_number":577,"context_line":"        try:"},{"line_number":578,"context_line":"            attachment_ref \u003d cinderclient("}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_1f83a5ce","line":575,"range":{"start_line":573,"start_character":8,"end_line":575,"end_character":64},"in_reply_to":"ff346bd7_049b5bc9","updated":"2017-07-24 15:54:25.000000000","message":"We don\u0027t use the object that\u0027s coming out of this function almost at all. Whatever blob comes form Cinder that can be part of connection_info as it\u0027s a free form something.\n\nAs attachment_create returns a dict no matter what it doesn\u0027t make that much sense to keep the object here either in my opinion.","commit_id":"552b38a7f99c02418198313c409ae8293b4f5446"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8bbd45f3a7e13788580b7b1d73e69980eb49884e","unresolved":false,"context_lines":[{"line_number":570,"context_line":"        :param connector: host connector dict. This is required when updating"},{"line_number":571,"context_line":"            a volume attachment. To terminate a connection, the volume"},{"line_number":572,"context_line":"            attachment for that connection must be deleted."},{"line_number":573,"context_line":"        :returns: connection_info dict, built from the"},{"line_number":574,"context_line":"            cinderclient.v3.attachments.VolumeAttachment object (this"},{"line_number":575,"context_line":"            gives us a backward compatible connection_info dict)"},{"line_number":576,"context_line":"        \"\"\""},{"line_number":577,"context_line":"        try:"},{"line_number":578,"context_line":"            attachment_ref \u003d cinderclient("}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_cdee85b3","line":575,"range":{"start_line":573,"start_character":8,"end_line":575,"end_character":64},"in_reply_to":"ff346bd7_1f83a5ce","updated":"2017-07-25 01:41:32.000000000","message":"Right so it\u0027s an object for update in cinderclient:\n\nhttps://github.com/openstack/python-cinderclient/blob/master/cinderclient/v3/attachments.py#L67\n\nBut again, why isn\u0027t _translate_attachment_ref just working on the attachment_ref.to_dict()[\u0027connection_info\u0027] field?\n\nI\u0027d rather create and update are consistent and return a dict (I guess), since that\u0027s what\u0027s the lowest common thing we can return. But I don\u0027t like that create returns an attachment ref and update is trying to return just the connection_info, but again, I don\u0027t see how this makes any sense anyway since I think we\u0027re passing a full attachment record to _translate_attachment_ref, not a connection_info dict field from within the attachment record in the REST API response from Cinder.","commit_id":"552b38a7f99c02418198313c409ae8293b4f5446"}]}