)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"11e206f99470edf2410f394cefdd9f2007e079ca","unresolved":false,"context_lines":[{"line_number":24,"context_line":"Changes in the drivers are left to each driver"},{"line_number":25,"context_line":"maintainer to update."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Co-Authored-By: Thang Pham \u003cthang.g.pham@gmail.com\u003e"},{"line_number":28,"context_line":"Partial-Implements: blueprint cinder-objects"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Change-Id: I0ef693a9c3d72349563e7199aa27a0724f98a774"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"dada55a8_e3f2db99","line":27,"range":{"start_line":27,"start_character":0,"end_line":27,"end_character":51},"updated":"2016-07-18 12:41:08.000000000","message":"@Lisa: I think you should also add yourself here.","commit_id":"7080498bb3c19e18a2d22622df1f8b0b0226fff4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"720a6fd8160a8ba6f747b6832c32efd9cc6cb132","unresolved":false,"context_lines":[{"line_number":17,"context_line":"  and detach_volume() in file"},{"line_number":18,"context_line":"  cinder/tests/unit/test_volume.py"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"The following patch updates attach_volume"},{"line_number":21,"context_line":"API to use volume versionedobjects.  Changes were"},{"line_number":22,"context_line":"made to be backwards compatible with older RPC clients."},{"line_number":23,"context_line":"It only includes changes to the core cinder code."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"bacf61ea_703b39ec","line":20,"range":{"start_line":20,"start_character":0,"end_line":20,"end_character":20},"updated":"2016-07-28 10:46:04.000000000","message":"nit: This patch","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"3c11bc21057b1ba8bdee426a1b2e41154d015ad9","unresolved":false,"context_lines":[{"line_number":17,"context_line":"  and detach_volume() in file"},{"line_number":18,"context_line":"  cinder/tests/unit/test_volume.py"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"The following patch updates attach_volume"},{"line_number":21,"context_line":"API to use volume versionedobjects.  Changes were"},{"line_number":22,"context_line":"made to be backwards compatible with older RPC clients."},{"line_number":23,"context_line":"It only includes changes to the core cinder code."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"bacf61ea_13744ffb","line":20,"range":{"start_line":20,"start_character":0,"end_line":20,"end_character":20},"in_reply_to":"bacf61ea_703b39ec","updated":"2016-07-29 07:24:20.000000000","message":"Done","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"}],"cinder/db/sqlalchemy/api.py":[{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"0a464bdfa8136e690f9f72d3d8a073c507e684e3","unresolved":false,"context_lines":[{"line_number":1407,"context_line":"                          \u0027attached_host\u0027: host_name,"},{"line_number":1408,"context_line":"                          \u0027attach_time\u0027: timeutils.utcnow(),"},{"line_number":1409,"context_line":"                          \u0027attach_mode\u0027: attach_mode,"},{"line_number":1410,"context_line":"                          \u0027updated_at\u0027: literal_column(\u0027updated_at\u0027)}"},{"line_number":1411,"context_line":"        volume_attachment_ref.update(updated_values)"},{"line_number":1412,"context_line":"        volume_attachment_ref.save(session\u003dsession)"},{"line_number":1413,"context_line":"        del updated_values[\u0027updated_at\u0027]"}],"source_content_type":"text/x-python","patch_set":26,"id":"7a77a97e_fd4a8926","line":1410,"updated":"2016-11-22 07:51:02.000000000","message":"Bear me with more dump questions, it looks like \u0027updated_values\u0027 is only used to reset VolumeAttachment object.  How is it related to the Volume object which is to be returned to caller of finish_attach()?  That Volume object has already latest state of the volume.","commit_id":"85c9fefa29be379af14e1db744546c90121fc61b"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"8cf9183af1a7db7639d4710964e874063a02aed0","unresolved":false,"context_lines":[{"line_number":1407,"context_line":"                          \u0027attached_host\u0027: host_name,"},{"line_number":1408,"context_line":"                          \u0027attach_time\u0027: timeutils.utcnow(),"},{"line_number":1409,"context_line":"                          \u0027attach_mode\u0027: attach_mode,"},{"line_number":1410,"context_line":"                          \u0027updated_at\u0027: literal_column(\u0027updated_at\u0027)}"},{"line_number":1411,"context_line":"        volume_attachment_ref.update(updated_values)"},{"line_number":1412,"context_line":"        volume_attachment_ref.save(session\u003dsession)"},{"line_number":1413,"context_line":"        del updated_values[\u0027updated_at\u0027]"}],"source_content_type":"text/x-python","patch_set":26,"id":"7a77a97e_75c93bac","line":1410,"updated":"2016-11-22 01:10:40.000000000","message":"Is this necessary?","commit_id":"85c9fefa29be379af14e1db744546c90121fc61b"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"40ff4966d1d3888d2c72f7ed19a7b3e4a7d65291","unresolved":false,"context_lines":[{"line_number":1407,"context_line":"                          \u0027attached_host\u0027: host_name,"},{"line_number":1408,"context_line":"                          \u0027attach_time\u0027: timeutils.utcnow(),"},{"line_number":1409,"context_line":"                          \u0027attach_mode\u0027: attach_mode,"},{"line_number":1410,"context_line":"                          \u0027updated_at\u0027: literal_column(\u0027updated_at\u0027)}"},{"line_number":1411,"context_line":"        volume_attachment_ref.update(updated_values)"},{"line_number":1412,"context_line":"        volume_attachment_ref.save(session\u003dsession)"},{"line_number":1413,"context_line":"        del updated_values[\u0027updated_at\u0027]"}],"source_content_type":"text/x-python","patch_set":26,"id":"7a77a97e_75b3fb0a","line":1410,"in_reply_to":"7a77a97e_75c93bac","updated":"2016-11-22 01:33:50.000000000","message":"updated_values includes all the updated values for the volume object, and later returned to object method. In object method, it is used to update the corresponding volume object, so that we don\u0027t need to refresh and keep db volume and object volume consistent.","commit_id":"85c9fefa29be379af14e1db744546c90121fc61b"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"826f5e660ce383b0c085b390a89801ef9d89c8a5","unresolved":false,"context_lines":[{"line_number":1407,"context_line":"                          \u0027attached_host\u0027: host_name,"},{"line_number":1408,"context_line":"                          \u0027attach_time\u0027: timeutils.utcnow(),"},{"line_number":1409,"context_line":"                          \u0027attach_mode\u0027: attach_mode,"},{"line_number":1410,"context_line":"                          \u0027updated_at\u0027: literal_column(\u0027updated_at\u0027)}"},{"line_number":1411,"context_line":"        volume_attachment_ref.update(updated_values)"},{"line_number":1412,"context_line":"        volume_attachment_ref.save(session\u003dsession)"},{"line_number":1413,"context_line":"        del updated_values[\u0027updated_at\u0027]"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a74a57a_f43ad1e6","line":1410,"in_reply_to":"7a77a97e_fd4a8926","updated":"2016-11-28 03:44:54.000000000","message":"Yeah, it should be volume_attachment object. Please replace volume with volume_attachment in my first comments. Sorry!\nWhen we call attachment.finish_attach() in https://review.openstack.org/#/c/268608/27/cinder/volume/manager.py@1010 , the attachment object is updated with updated_values, no need to refresh. As later we may need to return attachment object","commit_id":"85c9fefa29be379af14e1db744546c90121fc61b"}],"cinder/objects/volume.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"72a90abe10a8f6de3a1e76a06d87d2d0fb78ecc1","unresolved":false,"context_lines":[{"line_number":512,"context_line":"                                              volume_id\u003dself.id)"},{"line_number":513,"context_line":"        attachment.create()"},{"line_number":514,"context_line":"        with self.obj_as_admin():"},{"line_number":515,"context_line":"            self.admin_metadata[\u0027attached_mode\u0027] \u003d attach_mode"},{"line_number":516,"context_line":"            self.save()"},{"line_number":517,"context_line":"        return attachment"},{"line_number":518,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"7a77a97e_94116760","line":515,"range":{"start_line":515,"start_character":0,"end_line":515,"end_character":62},"updated":"2016-11-22 11:08:44.000000000","message":"Hm, not really related to this particular patchset, but I\u0027ve just noticed that - what happens with multiattach here? This will get overwritten if we\u0027re doing multiattach with different modes… Is this a bug?","commit_id":"85c9fefa29be379af14e1db744546c90121fc61b"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"826f5e660ce383b0c085b390a89801ef9d89c8a5","unresolved":false,"context_lines":[{"line_number":512,"context_line":"                                              volume_id\u003dself.id)"},{"line_number":513,"context_line":"        attachment.create()"},{"line_number":514,"context_line":"        with self.obj_as_admin():"},{"line_number":515,"context_line":"            self.admin_metadata[\u0027attached_mode\u0027] \u003d attach_mode"},{"line_number":516,"context_line":"            self.save()"},{"line_number":517,"context_line":"        return attachment"},{"line_number":518,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"5a74a57a_fe793183","line":515,"range":{"start_line":515,"start_character":0,"end_line":515,"end_character":62},"in_reply_to":"7a77a97e_94116760","updated":"2016-11-28 03:44:54.000000000","message":"https://github.com/openstack/cinder/blob/master/cinder/volume/manager.py#L934-L938\n\nCinder has done above check.","commit_id":"85c9fefa29be379af14e1db744546c90121fc61b"}],"cinder/objects/volume_attachment.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"11e206f99470edf2410f394cefdd9f2007e079ca","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            self.obj_reset_changes()"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    @classmethod"},{"line_number":62,"context_line":"    def volume_attach(cls, context, values):"},{"line_number":63,"context_line":"        db_attachment \u003d db.volume_attach(context, values)"},{"line_number":64,"context_line":"        return cls._from_db_object(context, cls(context), db_attachment)"},{"line_number":65,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"dada55a8_23eeb38b","line":62,"range":{"start_line":62,"start_character":8,"end_line":62,"end_character":15},"updated":"2016-07-18 12:41:08.000000000","message":"This is VolumeAttachment object, so I think we can skip `volume_` prefix.","commit_id":"7080498bb3c19e18a2d22622df1f8b0b0226fff4"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"0c2a55b4094b81db4b7457252930e5defe0da97b","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            self.obj_reset_changes()"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    @classmethod"},{"line_number":62,"context_line":"    def volume_attach(cls, context, values):"},{"line_number":63,"context_line":"        db_attachment \u003d db.volume_attach(context, values)"},{"line_number":64,"context_line":"        return cls._from_db_object(context, cls(context), db_attachment)"},{"line_number":65,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"dada55a8_8e64b2e5","line":62,"range":{"start_line":62,"start_character":8,"end_line":62,"end_character":15},"in_reply_to":"dada55a8_23eeb38b","updated":"2016-07-20 02:20:21.000000000","message":"Agree.","commit_id":"7080498bb3c19e18a2d22622df1f8b0b0226fff4"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"11e206f99470edf2410f394cefdd9f2007e079ca","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        db_attachment \u003d db.volume_attach(context, values)"},{"line_number":64,"context_line":"        return cls._from_db_object(context, cls(context), db_attachment)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    @classmethod"},{"line_number":67,"context_line":"    def volume_attached(cls, context, attachment_id, instance_uuid, host_name,"},{"line_number":68,"context_line":"                        mount_point, attach_mode\u003d\u0027rw\u0027):"},{"line_number":69,"context_line":"        db_volume \u003d db.volume_attached(context, attachment_id, instance_uuid,"}],"source_content_type":"text/x-python","patch_set":10,"id":"dada55a8_365c3325","line":66,"range":{"start_line":66,"start_character":0,"end_line":66,"end_character":16},"updated":"2016-07-18 12:41:08.000000000","message":"Shouldn\u0027t this be a regular method? That way we wouldn\u0027t need to pass attachment_id into the method as we would have it in the object and only need to update the object inside.\n\nIt\u0027s possible that implementing this remark will make code a lot more complicated, so feel free to ignore it if that\u0027s the case in your opinion.","commit_id":"7080498bb3c19e18a2d22622df1f8b0b0226fff4"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"11e206f99470edf2410f394cefdd9f2007e079ca","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        return cls._from_db_object(context, cls(context), db_attachment)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    @classmethod"},{"line_number":67,"context_line":"    def volume_attached(cls, context, attachment_id, instance_uuid, host_name,"},{"line_number":68,"context_line":"                        mount_point, attach_mode\u003d\u0027rw\u0027):"},{"line_number":69,"context_line":"        db_volume \u003d db.volume_attached(context, attachment_id, instance_uuid,"},{"line_number":70,"context_line":"                                       host_name, mount_point, attach_mode)"},{"line_number":71,"context_line":"        return objects.Volume._from_db_object("}],"source_content_type":"text/x-python","patch_set":10,"id":"dada55a8_63d1cbcd","line":68,"range":{"start_line":67,"start_character":0,"end_line":68,"end_character":55},"updated":"2016-07-18 12:41:08.000000000","message":"I understand that this is analogy with the SQLAlchemy API naming, but can we give this method a more descriptive name? According to docstring:\n\n    \"\"\"This method updates a volume attachment entry.\n\n    This function saves the information related to a particular\n    attachment for a volume.  It also updates the volume record\n    to mark the volume as attached.\n\n    \"\"\"\n\nMaybe `mark_attached()` or `update_attached()`?","commit_id":"7080498bb3c19e18a2d22622df1f8b0b0226fff4"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"0c2a55b4094b81db4b7457252930e5defe0da97b","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        return cls._from_db_object(context, cls(context), db_attachment)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    @classmethod"},{"line_number":67,"context_line":"    def volume_attached(cls, context, attachment_id, instance_uuid, host_name,"},{"line_number":68,"context_line":"                        mount_point, attach_mode\u003d\u0027rw\u0027):"},{"line_number":69,"context_line":"        db_volume \u003d db.volume_attached(context, attachment_id, instance_uuid,"},{"line_number":70,"context_line":"                                       host_name, mount_point, attach_mode)"},{"line_number":71,"context_line":"        return objects.Volume._from_db_object("}],"source_content_type":"text/x-python","patch_set":10,"id":"dada55a8_2ede6611","line":68,"range":{"start_line":67,"start_character":0,"end_line":68,"end_character":55},"in_reply_to":"dada55a8_63d1cbcd","updated":"2016-07-20 02:20:21.000000000","message":"I prefer to update_attached.","commit_id":"7080498bb3c19e18a2d22622df1f8b0b0226fff4"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"b8d3e30fe1aad2cfa127436b85377400fb7f51d7","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        db_attachment \u003d db.volume_attach(context, values)"},{"line_number":64,"context_line":"        return cls._from_db_object(context, cls(context), db_attachment)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update_attached(self, context, instance_uuid, host_name,"},{"line_number":67,"context_line":"                        mount_point, attach_mode\u003d\u0027rw\u0027):"},{"line_number":68,"context_line":"        db_volume \u003d db.volume_attached(context, self.id, instance_uuid,"},{"line_number":69,"context_line":"                                       host_name, mount_point, attach_mode)"}],"source_content_type":"text/x-python","patch_set":11,"id":"dada55a8_13db81ed","line":66,"range":{"start_line":66,"start_character":30,"end_line":66,"end_character":37},"updated":"2016-07-20 13:36:33.000000000","message":"Why do we need `context` here? Is it impossible to use self._context? If there\u0027s a problem with elevating it, then please take a look how it\u0027s approached elsewhere: [1], [2].\n\n[1] https://github.com/openstack/cinder/blob/916540622622e9424eea69964de0441a86e07c25/cinder/objects/volume.py#L329-L330\n[2] https://github.com/openstack/cinder/blob/504b466295f3d97307216095f450ff8421796319/cinder/objects/backup.py#L124-L125","commit_id":"6d4dc57ed3b1929e4efc396d55aaaeb037ea0075"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"766153be6e3472fb84c7c6174c81266d6ff523e9","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        db_attachment \u003d db.volume_attach(context, values)"},{"line_number":64,"context_line":"        return cls._from_db_object(context, cls(context), db_attachment)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update_attached(self, context, instance_uuid, host_name,"},{"line_number":67,"context_line":"                        mount_point, attach_mode\u003d\u0027rw\u0027):"},{"line_number":68,"context_line":"        db_volume \u003d db.volume_attached(context, self.id, instance_uuid,"},{"line_number":69,"context_line":"                                       host_name, mount_point, attach_mode)"}],"source_content_type":"text/x-python","patch_set":11,"id":"dada55a8_e231955b","line":66,"range":{"start_line":66,"start_character":30,"end_line":66,"end_character":37},"in_reply_to":"dada55a8_13db81ed","updated":"2016-07-21 01:58:12.000000000","message":"Yes, we can use self._context. Currently there is no elevating problem as it is only used once in manager and whose context is elevated.","commit_id":"6d4dc57ed3b1929e4efc396d55aaaeb037ea0075"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"720a6fd8160a8ba6f747b6832c32efd9cc6cb132","unresolved":false,"context_lines":[{"line_number":58,"context_line":"            db.volume_attachment_update(self._context, self.id, updates)"},{"line_number":59,"context_line":"            self.obj_reset_changes()"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    @classmethod"},{"line_number":62,"context_line":"    def attach(cls, context, values):"},{"line_number":63,"context_line":"        db_attachment \u003d db.volume_attach(context, values)"},{"line_number":64,"context_line":"        return cls._from_db_object(context, cls(context), db_attachment)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update_attached(self, instance_uuid, host_name,"},{"line_number":67,"context_line":"                        mount_point, attach_mode\u003d\u0027rw\u0027):"},{"line_number":68,"context_line":"        db_volume \u003d db.volume_attached(self._context, self.id, instance_uuid,"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_b6b5aace","line":65,"range":{"start_line":61,"start_character":0,"end_line":65,"end_character":0},"updated":"2016-07-28 10:46:04.000000000","message":"-1: This should be in the volume object instead, and there it should not be a class method, but an instance method and internally use these values:\n\n values \u003d {\u0027volume_id\u0027: self.id,\n           \u0027attach_status\u0027: \u0027attaching\u0027}\n\nAnd it would be better to change the method name to begin_attach or something like that.","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"3c11bc21057b1ba8bdee426a1b2e41154d015ad9","unresolved":false,"context_lines":[{"line_number":58,"context_line":"            db.volume_attachment_update(self._context, self.id, updates)"},{"line_number":59,"context_line":"            self.obj_reset_changes()"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    @classmethod"},{"line_number":62,"context_line":"    def attach(cls, context, values):"},{"line_number":63,"context_line":"        db_attachment \u003d db.volume_attach(context, values)"},{"line_number":64,"context_line":"        return cls._from_db_object(context, cls(context), db_attachment)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update_attached(self, instance_uuid, host_name,"},{"line_number":67,"context_line":"                        mount_point, attach_mode\u003d\u0027rw\u0027):"},{"line_number":68,"context_line":"        db_volume \u003d db.volume_attached(self._context, self.id, instance_uuid,"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_7b648748","line":65,"range":{"start_line":61,"start_character":0,"end_line":65,"end_character":0},"in_reply_to":"bacf61ea_b6b5aace","updated":"2016-07-29 07:24:20.000000000","message":"Moved it to volume.","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"720a6fd8160a8ba6f747b6832c32efd9cc6cb132","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        db_attachment \u003d db.volume_attach(context, values)"},{"line_number":64,"context_line":"        return cls._from_db_object(context, cls(context), db_attachment)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update_attached(self, instance_uuid, host_name,"},{"line_number":67,"context_line":"                        mount_point, attach_mode\u003d\u0027rw\u0027):"},{"line_number":68,"context_line":"        db_volume \u003d db.volume_attached(self._context, self.id, instance_uuid,"},{"line_number":69,"context_line":"                                       host_name, mount_point, attach_mode)"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_99825951","line":66,"updated":"2016-07-28 10:46:04.000000000","message":"-1: This name is confusing, because it doesn\u0027t convey the difference between save and the update.  A better name is needed \"finish_attachment\", \"complete_attachment\" or whatever you can come up with.","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"3c11bc21057b1ba8bdee426a1b2e41154d015ad9","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        db_attachment \u003d db.volume_attach(context, values)"},{"line_number":64,"context_line":"        return cls._from_db_object(context, cls(context), db_attachment)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update_attached(self, instance_uuid, host_name,"},{"line_number":67,"context_line":"                        mount_point, attach_mode\u003d\u0027rw\u0027):"},{"line_number":68,"context_line":"        db_volume \u003d db.volume_attached(self._context, self.id, instance_uuid,"},{"line_number":69,"context_line":"                                       host_name, mount_point, attach_mode)"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_1b562b5c","line":66,"in_reply_to":"bacf61ea_99825951","updated":"2016-07-29 07:24:20.000000000","message":"Updated it to finish_attachment.","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"720a6fd8160a8ba6f747b6832c32efd9cc6cb132","unresolved":false,"context_lines":[{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update_attached(self, instance_uuid, host_name,"},{"line_number":67,"context_line":"                        mount_point, attach_mode\u003d\u0027rw\u0027):"},{"line_number":68,"context_line":"        db_volume \u003d db.volume_attached(self._context, self.id, instance_uuid,"},{"line_number":69,"context_line":"                                       host_name, mount_point, attach_mode)"},{"line_number":70,"context_line":"        return objects.Volume._from_db_object("},{"line_number":71,"context_line":"            self._context, objects.Volume(), db_volume)"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_7f52e514","line":68,"updated":"2016-07-28 10:46:04.000000000","message":"-1: We are changing the attachment DB entry but not updating the OVO accordingly, so we end up with an out of sync OVO.","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"3c11bc21057b1ba8bdee426a1b2e41154d015ad9","unresolved":false,"context_lines":[{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update_attached(self, instance_uuid, host_name,"},{"line_number":67,"context_line":"                        mount_point, attach_mode\u003d\u0027rw\u0027):"},{"line_number":68,"context_line":"        db_volume \u003d db.volume_attached(self._context, self.id, instance_uuid,"},{"line_number":69,"context_line":"                                       host_name, mount_point, attach_mode)"},{"line_number":70,"context_line":"        return objects.Volume._from_db_object("},{"line_number":71,"context_line":"            self._context, objects.Volume(), db_volume)"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_e1456870","line":68,"in_reply_to":"bacf61ea_7f52e514","updated":"2016-07-29 07:24:20.000000000","message":"Exactly yes. \n\nI use refresh to sync db\u0027s data. I noticed that this seems a bad idea. \n\nWhen I would like to move logic in db.sqlalchemy.api.volume_attached to this method, I noticed that volume update and attachment update are in same session, which prevents the intermediate state got in some extent. As a result I kept the logic in the db interface.","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"270082dbe0912a8cb7e1109fc705de8702c01e1e","unresolved":false,"context_lines":[{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update_attached(self, instance_uuid, host_name,"},{"line_number":67,"context_line":"                        mount_point, attach_mode\u003d\u0027rw\u0027):"},{"line_number":68,"context_line":"        db_volume \u003d db.volume_attached(self._context, self.id, instance_uuid,"},{"line_number":69,"context_line":"                                       host_name, mount_point, attach_mode)"},{"line_number":70,"context_line":"        return objects.Volume._from_db_object("},{"line_number":71,"context_line":"            self._context, objects.Volume(), db_volume)"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_edd70feb","line":68,"in_reply_to":"bacf61ea_e1456870","updated":"2016-07-29 12:22:25.000000000","message":"But you can still return the values that have been changed, and use those values here.","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"12b49d6d2e9c65619ef1fa1df9718b2f4170beb3","unresolved":false,"context_lines":[{"line_number":66,"context_line":"                                       instance_uuid, host_name,"},{"line_number":67,"context_line":"                                       mount_point, attach_mode)"},{"line_number":68,"context_line":"        # Refresh status as above db operation changes attachment."},{"line_number":69,"context_line":"        self.refresh()"},{"line_number":70,"context_line":"        return objects.Volume._from_db_object(self._context,"},{"line_number":71,"context_line":"                                              objects.Volume(),"},{"line_number":72,"context_line":"                                              db_volume)"}],"source_content_type":"text/x-python","patch_set":13,"id":"bacf61ea_2de127d1","line":69,"updated":"2016-07-29 12:22:37.000000000","message":"-1: Don\u0027t do a refresh when you can avoid it.  You can change volume_attached to return a tuple with the new volume ORM instance and the changes made to the volume_attachment, that way you don\u0027t need to do yet another DB query.","commit_id":"037bae0c9adaa89cf03f95ea043004133e6b6e5c"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"11f0f159ee1cceb8adcb9eeda75d034aecce5092","unresolved":false,"context_lines":[{"line_number":66,"context_line":"                                       instance_uuid, host_name,"},{"line_number":67,"context_line":"                                       mount_point, attach_mode)"},{"line_number":68,"context_line":"        # Refresh status as above db operation changes attachment."},{"line_number":69,"context_line":"        self.refresh()"},{"line_number":70,"context_line":"        return objects.Volume._from_db_object(self._context,"},{"line_number":71,"context_line":"                                              objects.Volume(),"},{"line_number":72,"context_line":"                                              db_volume)"}],"source_content_type":"text/x-python","patch_set":13,"id":"bacf61ea_f564b4b9","line":69,"in_reply_to":"bacf61ea_2de127d1","updated":"2016-08-01 07:56:42.000000000","message":"Done","commit_id":"037bae0c9adaa89cf03f95ea043004133e6b6e5c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"24e6bf93da8ed03fa2816d87148e49513d0baca9","unresolved":false,"context_lines":[{"line_number":63,"context_line":"    def finish_attach(self, instance_uuid, host_name,"},{"line_number":64,"context_line":"                      mount_point, attach_mode\u003d\u0027rw\u0027):"},{"line_number":65,"context_line":"        with self.obj_as_admin():"},{"line_number":66,"context_line":"            (db_volume, updated_values) \u003d db.volume_attached("},{"line_number":67,"context_line":"                self._context, self.id,"},{"line_number":68,"context_line":"                instance_uuid, host_name,"},{"line_number":69,"context_line":"                mount_point, attach_mode)"}],"source_content_type":"text/x-python","patch_set":16,"id":"bacf61ea_3491d553","line":66,"range":{"start_line":66,"start_character":12,"end_line":66,"end_character":39},"updated":"2016-08-02 15:59:42.000000000","message":"nit: You don\u0027t need the parenthesis","commit_id":"11bed8d41658996fd2bef953e8906447c5554559"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"4de5ffb1ba7075a951daa54e7c36cef0a0a754f5","unresolved":false,"context_lines":[{"line_number":63,"context_line":"    def finish_attach(self, instance_uuid, host_name,"},{"line_number":64,"context_line":"                      mount_point, attach_mode\u003d\u0027rw\u0027):"},{"line_number":65,"context_line":"        with self.obj_as_admin():"},{"line_number":66,"context_line":"            (db_volume, updated_values) \u003d db.volume_attached("},{"line_number":67,"context_line":"                self._context, self.id,"},{"line_number":68,"context_line":"                instance_uuid, host_name,"},{"line_number":69,"context_line":"                mount_point, attach_mode)"}],"source_content_type":"text/x-python","patch_set":16,"id":"bacf61ea_7ff2ec07","line":66,"range":{"start_line":66,"start_character":12,"end_line":66,"end_character":39},"in_reply_to":"bacf61ea_3491d553","updated":"2016-08-03 09:20:57.000000000","message":"Done","commit_id":"11bed8d41658996fd2bef953e8906447c5554559"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"24e6bf93da8ed03fa2816d87148e49513d0baca9","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                self._context, self.id,"},{"line_number":68,"context_line":"                instance_uuid, host_name,"},{"line_number":69,"context_line":"                mount_point, attach_mode)"},{"line_number":70,"context_line":"        # Refresh status as above db operation changes attachment."},{"line_number":71,"context_line":"        self.update(updated_values)"},{"line_number":72,"context_line":"        self.obj_reset_changes(updated_values.keys())"},{"line_number":73,"context_line":"        return objects.Volume._from_db_object(self._context,"}],"source_content_type":"text/x-python","patch_set":16,"id":"bacf61ea_3aafff7e","line":70,"range":{"start_line":70,"start_character":10,"end_line":70,"end_character":17},"updated":"2016-08-02 15:59:42.000000000","message":"nit: This comment should probably be updated","commit_id":"11bed8d41658996fd2bef953e8906447c5554559"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"4de5ffb1ba7075a951daa54e7c36cef0a0a754f5","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                self._context, self.id,"},{"line_number":68,"context_line":"                instance_uuid, host_name,"},{"line_number":69,"context_line":"                mount_point, attach_mode)"},{"line_number":70,"context_line":"        # Refresh status as above db operation changes attachment."},{"line_number":71,"context_line":"        self.update(updated_values)"},{"line_number":72,"context_line":"        self.obj_reset_changes(updated_values.keys())"},{"line_number":73,"context_line":"        return objects.Volume._from_db_object(self._context,"}],"source_content_type":"text/x-python","patch_set":16,"id":"bacf61ea_5ff728f5","line":70,"range":{"start_line":70,"start_character":10,"end_line":70,"end_character":17},"in_reply_to":"bacf61ea_3aafff7e","updated":"2016-08-03 09:20:57.000000000","message":"Done","commit_id":"11bed8d41658996fd2bef953e8906447c5554559"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"24e6bf93da8ed03fa2816d87148e49513d0baca9","unresolved":false,"context_lines":[{"line_number":68,"context_line":"                instance_uuid, host_name,"},{"line_number":69,"context_line":"                mount_point, attach_mode)"},{"line_number":70,"context_line":"        # Refresh status as above db operation changes attachment."},{"line_number":71,"context_line":"        self.update(updated_values)"},{"line_number":72,"context_line":"        self.obj_reset_changes(updated_values.keys())"},{"line_number":73,"context_line":"        return objects.Volume._from_db_object(self._context,"},{"line_number":74,"context_line":"                                              objects.Volume(),"},{"line_number":75,"context_line":"                                              db_volume)"}],"source_content_type":"text/x-python","patch_set":16,"id":"bacf61ea_bac3ef37","line":72,"range":{"start_line":71,"start_character":0,"end_line":72,"end_character":53},"updated":"2016-08-02 15:59:42.000000000","message":"Thank you for making this improvement!","commit_id":"11bed8d41658996fd2bef953e8906447c5554559"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"4de5ffb1ba7075a951daa54e7c36cef0a0a754f5","unresolved":false,"context_lines":[{"line_number":68,"context_line":"                instance_uuid, host_name,"},{"line_number":69,"context_line":"                mount_point, attach_mode)"},{"line_number":70,"context_line":"        # Refresh status as above db operation changes attachment."},{"line_number":71,"context_line":"        self.update(updated_values)"},{"line_number":72,"context_line":"        self.obj_reset_changes(updated_values.keys())"},{"line_number":73,"context_line":"        return objects.Volume._from_db_object(self._context,"},{"line_number":74,"context_line":"                                              objects.Volume(),"},{"line_number":75,"context_line":"                                              db_volume)"}],"source_content_type":"text/x-python","patch_set":16,"id":"bacf61ea_bfdc6475","line":72,"range":{"start_line":71,"start_character":0,"end_line":72,"end_character":53},"in_reply_to":"bacf61ea_bac3ef37","updated":"2016-08-03 09:20:57.000000000","message":"My pleasure!","commit_id":"11bed8d41658996fd2bef953e8906447c5554559"}],"cinder/tests/unit/api/contrib/test_admin_actions.py":[{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"f6d72f7051138dc97495ac6daf7d631fd4aec36d","unresolved":false,"context_lines":[{"line_number":991,"context_line":""},{"line_number":992,"context_line":"        values \u003d {\u0027status\u0027: \u0027attaching\u0027,"},{"line_number":993,"context_line":"                  \u0027instance_uuid\u0027: fakes.get_fake_uuid()}"},{"line_number":994,"context_line":"        db.volume_admin_metadata_update(self.ctx, volume[\u0027id\u0027],"},{"line_number":995,"context_line":"                                        {\"attached_mode\": \u0027rw\u0027}, False)"},{"line_number":996,"context_line":"        volume.update(values)"},{"line_number":997,"context_line":"        volume.save()"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba63e191_b6c0d87d","line":994,"updated":"2016-01-19 17:10:43.000000000","message":"self.ctx is an admin, so you should be able to use:\n\nvolume.admin_metadata \u003d {\"attached_mode\": \u0027rw\u0027}\n\nThis will be saved when you use volume.save() in L997 below.","commit_id":"ecfc2120c2fe7597dbce3b38ba668e5641cfb71b"},{"author":{"_account_id":14274,"name":"XinXiaohui","email":"xiaohui.xin@intel.com","username":"XinXiaohui"},"change_message_id":"1516e762f9d73f7d2d4de32b7c1eb7ffb04a04cb","unresolved":false,"context_lines":[{"line_number":991,"context_line":""},{"line_number":992,"context_line":"        values \u003d {\u0027status\u0027: \u0027attaching\u0027,"},{"line_number":993,"context_line":"                  \u0027instance_uuid\u0027: fakes.get_fake_uuid()}"},{"line_number":994,"context_line":"        db.volume_admin_metadata_update(self.ctx, volume[\u0027id\u0027],"},{"line_number":995,"context_line":"                                        {\"attached_mode\": \u0027rw\u0027}, False)"},{"line_number":996,"context_line":"        volume.update(values)"},{"line_number":997,"context_line":"        volume.save()"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba63e191_5b5ba354","line":994,"in_reply_to":"ba63e191_b6c0d87d","updated":"2016-01-20 09:09:40.000000000","message":"Done","commit_id":"ecfc2120c2fe7597dbce3b38ba668e5641cfb71b"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"1052794eb48ed6409c7e2f560808a661148e1f22","unresolved":false,"context_lines":[{"line_number":990,"context_line":"                                                \u0027size\u0027: 1})"},{"line_number":991,"context_line":""},{"line_number":992,"context_line":"        values \u003d {\u0027status\u0027: \u0027attaching\u0027,"},{"line_number":993,"context_line":"                  \u0027instance_uuid\u0027: fakes.get_fake_uuid()}"},{"line_number":994,"context_line":"        volume.admin_metadata \u003d {\"attached_mode\": \u0027rw\u0027}"},{"line_number":995,"context_line":"        volume.update(values)"},{"line_number":996,"context_line":"        volume.save()"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a68dd71_7bada730","line":993,"updated":"2016-01-20 13:18:23.000000000","message":"You can directly assign the volume value here too - \nvolume.status \u003d \u0027attaching\u0027\nvolume.instance_uuid \u003d fakes.get_fake_uuid()","commit_id":"20a8ff6993457f544e3c3b398f3bf33487a3f8bc"},{"author":{"_account_id":14274,"name":"XinXiaohui","email":"xiaohui.xin@intel.com","username":"XinXiaohui"},"change_message_id":"f9e716879533c10492fa9a8f487b66188630c2f6","unresolved":false,"context_lines":[{"line_number":990,"context_line":"                                                \u0027size\u0027: 1})"},{"line_number":991,"context_line":""},{"line_number":992,"context_line":"        values \u003d {\u0027status\u0027: \u0027attaching\u0027,"},{"line_number":993,"context_line":"                  \u0027instance_uuid\u0027: fakes.get_fake_uuid()}"},{"line_number":994,"context_line":"        volume.admin_metadata \u003d {\"attached_mode\": \u0027rw\u0027}"},{"line_number":995,"context_line":"        volume.update(values)"},{"line_number":996,"context_line":"        volume.save()"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a68dd71_5bc75a12","line":993,"in_reply_to":"9a68dd71_7bada730","updated":"2016-01-21 02:34:12.000000000","message":"Done","commit_id":"20a8ff6993457f544e3c3b398f3bf33487a3f8bc"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"9837fa783fc06db7073a10e5211fa9eb2f038c04","unresolved":false,"context_lines":[{"line_number":1001,"context_line":"                          self.volume_api.attach,"},{"line_number":1002,"context_line":"                          self.ctx,"},{"line_number":1003,"context_line":"                          volume,"},{"line_number":1004,"context_line":"                          values[\u0027instance_uuid\u0027],"},{"line_number":1005,"context_line":"                          None,"},{"line_number":1006,"context_line":"                          mountpoint,"},{"line_number":1007,"context_line":"                          \u0027ro\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a68dd71_2ab61bf9","line":1004,"updated":"2016-01-25 16:30:00.000000000","message":"You could use fakes.get_fake_uuid() here and remove the values dict completely above.","commit_id":"b1f1ad4f19a80b54848e430b0b31eff3afbcf897"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"720a6fd8160a8ba6f747b6832c32efd9cc6cb132","unresolved":false,"context_lines":[{"line_number":1035,"context_line":"        volume \u003d self._create_volume(self.ctx, {\u0027provider_location\u0027: \u0027\u0027,"},{"line_number":1036,"context_line":"                                                \u0027size\u0027: 1})"},{"line_number":1037,"context_line":""},{"line_number":1038,"context_line":"        volume.status \u003d \u0027attaching\u0027"},{"line_number":1039,"context_line":"        volume.instance_uuid \u003d fake.INSTANCE_ID"},{"line_number":1040,"context_line":"        volume.admin_metadata \u003d {\"attached_mode\": \u0027rw\u0027}"},{"line_number":1041,"context_line":"        volume.save()"},{"line_number":1042,"context_line":"        mountpoint \u003d \u0027/dev/vbd\u0027"},{"line_number":1043,"context_line":"        self.assertRaises(exception.InvalidVolume,"},{"line_number":1044,"context_line":"                          self.volume_api.attach,"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_19730982","line":1041,"range":{"start_line":1038,"start_character":8,"end_line":1041,"end_character":21},"updated":"2016-07-28 10:46:04.000000000","message":"I believe this should be passed in the updates parameter of _create_volume in L1035.","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"3c11bc21057b1ba8bdee426a1b2e41154d015ad9","unresolved":false,"context_lines":[{"line_number":1035,"context_line":"        volume \u003d self._create_volume(self.ctx, {\u0027provider_location\u0027: \u0027\u0027,"},{"line_number":1036,"context_line":"                                                \u0027size\u0027: 1})"},{"line_number":1037,"context_line":""},{"line_number":1038,"context_line":"        volume.status \u003d \u0027attaching\u0027"},{"line_number":1039,"context_line":"        volume.instance_uuid \u003d fake.INSTANCE_ID"},{"line_number":1040,"context_line":"        volume.admin_metadata \u003d {\"attached_mode\": \u0027rw\u0027}"},{"line_number":1041,"context_line":"        volume.save()"},{"line_number":1042,"context_line":"        mountpoint \u003d \u0027/dev/vbd\u0027"},{"line_number":1043,"context_line":"        self.assertRaises(exception.InvalidVolume,"},{"line_number":1044,"context_line":"                          self.volume_api.attach,"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_c148248a","line":1041,"range":{"start_line":1038,"start_character":8,"end_line":1041,"end_character":21},"in_reply_to":"bacf61ea_19730982","updated":"2016-07-29 07:24:20.000000000","message":"Done","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"}],"cinder/tests/unit/objects/test_volume.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"12b49d6d2e9c65619ef1fa1df9718b2f4170beb3","unresolved":false,"context_lines":[{"line_number":400,"context_line":"        volume \u003d fake_volume.fake_volume_obj(self.context)"},{"line_number":401,"context_line":"        values \u003d {\u0027volume_id\u0027: volume.id,"},{"line_number":402,"context_line":"                  \u0027attach_status\u0027: \u0027attaching\u0027}"},{"line_number":403,"context_line":"        db_attachment \u003d fake_volume.fake_db_volume_attachment("},{"line_number":404,"context_line":"            **values)"},{"line_number":405,"context_line":"        volume_attach.return_value \u003d db_attachment"},{"line_number":406,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"bacf61ea_ad3b5737","line":403,"updated":"2016-07-29 12:22:37.000000000","message":"nit: it\u0027s cleaner just passing them as keyword arguments:\n\n db_attachment \u003d fake_volume.fake_db_volume_attachment(\n     volume_id\u003dvolume.id, attach_status\u003d\u0027attaching\u0027)","commit_id":"037bae0c9adaa89cf03f95ea043004133e6b6e5c"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"11f0f159ee1cceb8adcb9eeda75d034aecce5092","unresolved":false,"context_lines":[{"line_number":400,"context_line":"        volume \u003d fake_volume.fake_volume_obj(self.context)"},{"line_number":401,"context_line":"        values \u003d {\u0027volume_id\u0027: volume.id,"},{"line_number":402,"context_line":"                  \u0027attach_status\u0027: \u0027attaching\u0027}"},{"line_number":403,"context_line":"        db_attachment \u003d fake_volume.fake_db_volume_attachment("},{"line_number":404,"context_line":"            **values)"},{"line_number":405,"context_line":"        volume_attach.return_value \u003d db_attachment"},{"line_number":406,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"bacf61ea_3512cc09","line":403,"in_reply_to":"bacf61ea_ad3b5737","updated":"2016-08-01 07:56:42.000000000","message":"Done","commit_id":"037bae0c9adaa89cf03f95ea043004133e6b6e5c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"24e6bf93da8ed03fa2816d87148e49513d0baca9","unresolved":false,"context_lines":[{"line_number":414,"context_line":"            volume_id\u003dvolume.id, attach_status\u003d\u0027attaching\u0027)"},{"line_number":415,"context_line":"        volume_attach.return_value \u003d db_attachment"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"        attachment \u003d volume.begin_attach()"},{"line_number":418,"context_line":"        self.assertEqual(volume.id, attachment.volume_id)"},{"line_number":419,"context_line":"        self.assertEqual(\u0027attaching\u0027, attachment.attach_status)"},{"line_number":420,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"bacf61ea_fac5670a","line":417,"updated":"2016-08-02 15:59:42.000000000","message":"nit: Probably a good idea to confirm that you are returning an OVO instance and not just and ORM instance.","commit_id":"11bed8d41658996fd2bef953e8906447c5554559"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"4de5ffb1ba7075a951daa54e7c36cef0a0a754f5","unresolved":false,"context_lines":[{"line_number":414,"context_line":"            volume_id\u003dvolume.id, attach_status\u003d\u0027attaching\u0027)"},{"line_number":415,"context_line":"        volume_attach.return_value \u003d db_attachment"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"        attachment \u003d volume.begin_attach()"},{"line_number":418,"context_line":"        self.assertEqual(volume.id, attachment.volume_id)"},{"line_number":419,"context_line":"        self.assertEqual(\u0027attaching\u0027, attachment.attach_status)"},{"line_number":420,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"bacf61ea_3f36b4b4","line":417,"in_reply_to":"bacf61ea_fac5670a","updated":"2016-08-03 09:20:57.000000000","message":"Done","commit_id":"11bed8d41658996fd2bef953e8906447c5554559"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"01baa71fde8b2cdda939c59ea3311c81de94a55a","unresolved":false,"context_lines":[{"line_number":415,"context_line":"        volume_attach.return_value \u003d db_attachment"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"        attachment \u003d volume.begin_attach()"},{"line_number":418,"context_line":"        self.assertTrue(isinstance(attachment, objects.VolumeAttachment))"},{"line_number":419,"context_line":"        self.assertEqual(volume.id, attachment.volume_id)"},{"line_number":420,"context_line":"        self.assertEqual(\u0027attaching\u0027, attachment.attach_status)"},{"line_number":421,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"bacf61ea_d1e3fd03","line":418,"updated":"2016-08-03 11:08:03.000000000","message":"nit:\n\n  self.assertIsInstance(attachment, objects.VolumeAttachment)","commit_id":"ea1323b992e337b2e8b228b1382cc395d11b358d"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"0a4e706dbd9302521f7f3187215b8bc549c0d364","unresolved":false,"context_lines":[{"line_number":415,"context_line":"        volume_attach.return_value \u003d db_attachment"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"        attachment \u003d volume.begin_attach()"},{"line_number":418,"context_line":"        self.assertTrue(isinstance(attachment, objects.VolumeAttachment))"},{"line_number":419,"context_line":"        self.assertEqual(volume.id, attachment.volume_id)"},{"line_number":420,"context_line":"        self.assertEqual(\u0027attaching\u0027, attachment.attach_status)"},{"line_number":421,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"bacf61ea_c0a822de","line":418,"in_reply_to":"bacf61ea_d1e3fd03","updated":"2016-08-04 01:46:45.000000000","message":"Done","commit_id":"ea1323b992e337b2e8b228b1382cc395d11b358d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ff4be9dda58d8628f83c83ccc5e2d606e32eb01a","unresolved":false,"context_lines":[{"line_number":428,"context_line":"            self.assertEqual(\u0027attaching\u0027, attachment.attach_status)"},{"line_number":429,"context_line":"            metadata_update.assert_called_once_with(self.context.elevated(),"},{"line_number":430,"context_line":"                                                    volume.id,"},{"line_number":431,"context_line":"                                                    {\u0027attached_mode\u0027: u\u0027rw\u0027},"},{"line_number":432,"context_line":"                                                    True)"},{"line_number":433,"context_line":"            self.assertEqual(\u0027rw\u0027, volume.admin_metadata[\u0027attached_mode\u0027])"},{"line_number":434,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"fa7ab95a_9444dbb7","line":431,"range":{"start_line":431,"start_character":70,"end_line":431,"end_character":71},"updated":"2016-09-01 08:51:41.000000000","message":"nit: No need to force unicode.","commit_id":"6dc20b2d8728822e637d448cb3086230a3ac460b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"3505fa7d320bf1317f3e922189e16d7c3c92e5b0","unresolved":false,"context_lines":[{"line_number":428,"context_line":"            self.assertEqual(\u0027attaching\u0027, attachment.attach_status)"},{"line_number":429,"context_line":"            metadata_update.assert_called_once_with(self.context.elevated(),"},{"line_number":430,"context_line":"                                                    volume.id,"},{"line_number":431,"context_line":"                                                    {\u0027attached_mode\u0027: u\u0027rw\u0027},"},{"line_number":432,"context_line":"                                                    True)"},{"line_number":433,"context_line":"            self.assertEqual(\u0027rw\u0027, volume.admin_metadata[\u0027attached_mode\u0027])"},{"line_number":434,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"fa7ab95a_dd31e3a3","line":431,"range":{"start_line":431,"start_character":70,"end_line":431,"end_character":71},"in_reply_to":"fa7ab95a_11941d41","updated":"2016-09-02 09:44:40.000000000","message":"I\u0027m pretty sure it doesn\u0027t, and I just tested this in my system and it runs just fine for Python 2.7 and 3.4...","commit_id":"6dc20b2d8728822e637d448cb3086230a3ac460b"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"def6cc687f572ce5d03fccf8e33119bf9801e4cb","unresolved":false,"context_lines":[{"line_number":428,"context_line":"            self.assertEqual(\u0027attaching\u0027, attachment.attach_status)"},{"line_number":429,"context_line":"            metadata_update.assert_called_once_with(self.context.elevated(),"},{"line_number":430,"context_line":"                                                    volume.id,"},{"line_number":431,"context_line":"                                                    {\u0027attached_mode\u0027: u\u0027rw\u0027},"},{"line_number":432,"context_line":"                                                    True)"},{"line_number":433,"context_line":"            self.assertEqual(\u0027rw\u0027, volume.admin_metadata[\u0027attached_mode\u0027])"},{"line_number":434,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"fa7ab95a_11941d41","line":431,"range":{"start_line":431,"start_character":70,"end_line":431,"end_character":71},"in_reply_to":"fa7ab95a_9444dbb7","updated":"2016-09-02 07:24:01.000000000","message":"If I don\u0027t, the test fails.","commit_id":"6dc20b2d8728822e637d448cb3086230a3ac460b"}],"cinder/tests/unit/objects/test_volume_attachment.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"720a6fd8160a8ba6f747b6832c32efd9cc6cb132","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        values \u003d {\u0027volume_id\u0027: fake.VOLUME_ID,"},{"line_number":72,"context_line":"                  \u0027attach_status\u0027: \u0027attaching\u0027}"},{"line_number":73,"context_line":"        db_attachment \u003d fake_volume.fake_db_volume_attachment()"},{"line_number":74,"context_line":"        db_attachment[\u0027volume_id\u0027] \u003d fake.VOLUME_ID"},{"line_number":75,"context_line":"        db_attachment[\u0027attach_mode\u0027] \u003d \u0027attaching\u0027"},{"line_number":76,"context_line":"        volume_attach.return_value \u003d db_attachment"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        attachment \u003d objects.VolumeAttachment.attach("}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_59fd8139","line":75,"range":{"start_line":74,"start_character":7,"end_line":75,"end_character":50},"updated":"2016-07-28 10:46:04.000000000","message":"nit: I think this should be passed in L73 as keyword arguments.","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"3c11bc21057b1ba8bdee426a1b2e41154d015ad9","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        values \u003d {\u0027volume_id\u0027: fake.VOLUME_ID,"},{"line_number":72,"context_line":"                  \u0027attach_status\u0027: \u0027attaching\u0027}"},{"line_number":73,"context_line":"        db_attachment \u003d fake_volume.fake_db_volume_attachment()"},{"line_number":74,"context_line":"        db_attachment[\u0027volume_id\u0027] \u003d fake.VOLUME_ID"},{"line_number":75,"context_line":"        db_attachment[\u0027attach_mode\u0027] \u003d \u0027attaching\u0027"},{"line_number":76,"context_line":"        volume_attach.return_value \u003d db_attachment"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        attachment \u003d objects.VolumeAttachment.attach("}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_21602001","line":75,"range":{"start_line":74,"start_character":7,"end_line":75,"end_character":50},"in_reply_to":"bacf61ea_59fd8139","updated":"2016-07-29 07:24:20.000000000","message":"Done","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"ad53d13d63ada5a4cc4575e854f89d5948bf37c2","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                                                   fake.ATTACHMENT_ID)])"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    @mock.patch(\u0027cinder.db.sqlalchemy.api.volume_attached\u0027)"},{"line_number":70,"context_line":"    def test_volume_attached(self, volume_attached):"},{"line_number":71,"context_line":"        attachment \u003d fake_volume.fake_volume_attachment_obj(self.context)"},{"line_number":72,"context_line":"        updated_values \u003d {\u0027mountpoint\u0027: \u0027/dev/sda\u0027,"},{"line_number":73,"context_line":"                          \u0027attach_status\u0027: \u0027attached\u0027,"}],"source_content_type":"text/x-python","patch_set":24,"id":"fa7ab95a_691d72bd","line":70,"range":{"start_line":70,"start_character":8,"end_line":70,"end_character":28},"updated":"2016-09-02 11:46:52.000000000","message":"Should be test_finish_attach.","commit_id":"dd7942702777095cd40b38a70e19655bb4fd7c06"}],"cinder/tests/unit/test_db_api.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"24e6bf93da8ed03fa2816d87148e49513d0baca9","unresolved":false,"context_lines":[{"line_number":290,"context_line":"                  \u0027instance_uuid\u0027: instance_uuid,"},{"line_number":291,"context_line":"                  \u0027attach_status\u0027: \u0027attaching\u0027, }"},{"line_number":292,"context_line":"        attachment \u003d db.volume_attach(self.ctxt, values)"},{"line_number":293,"context_line":"        (volume_db, updated_values) \u003d db.volume_attached("},{"line_number":294,"context_line":"            self.ctxt,"},{"line_number":295,"context_line":"            attachment[\u0027id\u0027],"},{"line_number":296,"context_line":"            instance_uuid, None, \u0027/tmp\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"bacf61ea_3a437f6e","line":293,"range":{"start_line":293,"start_character":8,"end_line":293,"end_character":35},"updated":"2016-08-02 15:59:42.000000000","message":"nit: Parenthesis are not necessary","commit_id":"11bed8d41658996fd2bef953e8906447c5554559"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"4de5ffb1ba7075a951daa54e7c36cef0a0a754f5","unresolved":false,"context_lines":[{"line_number":290,"context_line":"                  \u0027instance_uuid\u0027: instance_uuid,"},{"line_number":291,"context_line":"                  \u0027attach_status\u0027: \u0027attaching\u0027, }"},{"line_number":292,"context_line":"        attachment \u003d db.volume_attach(self.ctxt, values)"},{"line_number":293,"context_line":"        (volume_db, updated_values) \u003d db.volume_attached("},{"line_number":294,"context_line":"            self.ctxt,"},{"line_number":295,"context_line":"            attachment[\u0027id\u0027],"},{"line_number":296,"context_line":"            instance_uuid, None, \u0027/tmp\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"bacf61ea_9f47e01c","line":293,"range":{"start_line":293,"start_character":8,"end_line":293,"end_character":35},"in_reply_to":"bacf61ea_3a437f6e","updated":"2016-08-03 09:20:57.000000000","message":"Done","commit_id":"11bed8d41658996fd2bef953e8906447c5554559"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"24e6bf93da8ed03fa2816d87148e49513d0baca9","unresolved":false,"context_lines":[{"line_number":322,"context_line":"                  \u0027attached_host\u0027: host_name,"},{"line_number":323,"context_line":"                  \u0027attach_status\u0027: \u0027attaching\u0027, }"},{"line_number":324,"context_line":"        attachment \u003d db.volume_attach(self.ctxt, values)"},{"line_number":325,"context_line":"        (volume_db, updated_values) \u003d db.volume_attached("},{"line_number":326,"context_line":"            self.ctxt, attachment[\u0027id\u0027],"},{"line_number":327,"context_line":"            None, host_name, \u0027/tmp\u0027)"},{"line_number":328,"context_line":"        expected_updated_values \u003d {"}],"source_content_type":"text/x-python","patch_set":16,"id":"bacf61ea_da1b437f","line":325,"range":{"start_line":325,"start_character":8,"end_line":325,"end_character":35},"updated":"2016-08-02 15:59:42.000000000","message":"ditto","commit_id":"11bed8d41658996fd2bef953e8906447c5554559"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"4de5ffb1ba7075a951daa54e7c36cef0a0a754f5","unresolved":false,"context_lines":[{"line_number":322,"context_line":"                  \u0027attached_host\u0027: host_name,"},{"line_number":323,"context_line":"                  \u0027attach_status\u0027: \u0027attaching\u0027, }"},{"line_number":324,"context_line":"        attachment \u003d db.volume_attach(self.ctxt, values)"},{"line_number":325,"context_line":"        (volume_db, updated_values) \u003d db.volume_attached("},{"line_number":326,"context_line":"            self.ctxt, attachment[\u0027id\u0027],"},{"line_number":327,"context_line":"            None, host_name, \u0027/tmp\u0027)"},{"line_number":328,"context_line":"        expected_updated_values \u003d {"}],"source_content_type":"text/x-python","patch_set":16,"id":"bacf61ea_ff541ccb","line":325,"range":{"start_line":325,"start_character":8,"end_line":325,"end_character":35},"in_reply_to":"bacf61ea_da1b437f","updated":"2016-08-03 09:20:57.000000000","message":"Done","commit_id":"11bed8d41658996fd2bef953e8906447c5554559"}],"cinder/tests/unit/test_volume.py":[{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"f6d72f7051138dc97495ac6daf7d631fd4aec36d","unresolved":false,"context_lines":[{"line_number":2200,"context_line":"                                               instance_uuid, None,"},{"line_number":2201,"context_line":"                                               mountpoint, \u0027ro\u0027,"},{"line_number":2202,"context_line":"                                               volume\u003dvolume)"},{"line_number":2203,"context_line":"        vol \u003d db.volume_get(context.get_admin_context(), volume_id)"},{"line_number":2204,"context_line":"        self.assertEqual(\"in-use\", vol[\u0027status\u0027])"},{"line_number":2205,"context_line":"        self.assertEqual(\u0027attached\u0027, attachment[\u0027attach_status\u0027])"},{"line_number":2206,"context_line":"        self.assertEqual(mountpoint, attachment[\u0027mountpoint\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba63e191_56a07c9f","line":2203,"updated":"2016-01-19 17:10:43.000000000","message":"You can use volume.refresh() here instead of another db call.  Also, you could use object notation below, since you are changing this test case to user object anyways, e.g. volume.status.","commit_id":"ecfc2120c2fe7597dbce3b38ba668e5641cfb71b"},{"author":{"_account_id":14274,"name":"XinXiaohui","email":"xiaohui.xin@intel.com","username":"XinXiaohui"},"change_message_id":"1516e762f9d73f7d2d4de32b7c1eb7ffb04a04cb","unresolved":false,"context_lines":[{"line_number":2200,"context_line":"                                               instance_uuid, None,"},{"line_number":2201,"context_line":"                                               mountpoint, \u0027ro\u0027,"},{"line_number":2202,"context_line":"                                               volume\u003dvolume)"},{"line_number":2203,"context_line":"        vol \u003d db.volume_get(context.get_admin_context(), volume_id)"},{"line_number":2204,"context_line":"        self.assertEqual(\"in-use\", vol[\u0027status\u0027])"},{"line_number":2205,"context_line":"        self.assertEqual(\u0027attached\u0027, attachment[\u0027attach_status\u0027])"},{"line_number":2206,"context_line":"        self.assertEqual(mountpoint, attachment[\u0027mountpoint\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba63e191_215240de","line":2203,"in_reply_to":"ba63e191_56a07c9f","updated":"2016-01-20 09:09:40.000000000","message":"Done","commit_id":"ecfc2120c2fe7597dbce3b38ba668e5641cfb71b"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"f6d72f7051138dc97495ac6daf7d631fd4aec36d","unresolved":false,"context_lines":[{"line_number":2520,"context_line":"        attachment \u003d self.volume.attach_volume(self.context, volume_id, None,"},{"line_number":2521,"context_line":"                                               \u0027fake_host\u0027, mountpoint, \u0027rw\u0027,"},{"line_number":2522,"context_line":"                                               volume\u003dvolume)"},{"line_number":2523,"context_line":"        vol \u003d db.volume_get(context.get_admin_context(), volume_id)"},{"line_number":2524,"context_line":"        self.assertEqual(\u0027in-use\u0027, vol[\u0027status\u0027])"},{"line_number":2525,"context_line":"        self.assertEqual(\u0027attached\u0027, attachment[\u0027attach_status\u0027])"},{"line_number":2526,"context_line":"        self.assertEqual(mountpoint, attachment[\u0027mountpoint\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba63e191_d6822c00","line":2523,"updated":"2016-01-19 17:10:43.000000000","message":"Ditto","commit_id":"ecfc2120c2fe7597dbce3b38ba668e5641cfb71b"},{"author":{"_account_id":14274,"name":"XinXiaohui","email":"xiaohui.xin@intel.com","username":"XinXiaohui"},"change_message_id":"1516e762f9d73f7d2d4de32b7c1eb7ffb04a04cb","unresolved":false,"context_lines":[{"line_number":2520,"context_line":"        attachment \u003d self.volume.attach_volume(self.context, volume_id, None,"},{"line_number":2521,"context_line":"                                               \u0027fake_host\u0027, mountpoint, \u0027rw\u0027,"},{"line_number":2522,"context_line":"                                               volume\u003dvolume)"},{"line_number":2523,"context_line":"        vol \u003d db.volume_get(context.get_admin_context(), volume_id)"},{"line_number":2524,"context_line":"        self.assertEqual(\u0027in-use\u0027, vol[\u0027status\u0027])"},{"line_number":2525,"context_line":"        self.assertEqual(\u0027attached\u0027, attachment[\u0027attach_status\u0027])"},{"line_number":2526,"context_line":"        self.assertEqual(mountpoint, attachment[\u0027mountpoint\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba63e191_2117a0ee","line":2523,"in_reply_to":"ba63e191_d6822c00","updated":"2016-01-20 09:09:40.000000000","message":"Done","commit_id":"ecfc2120c2fe7597dbce3b38ba668e5641cfb71b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"1b9ee1cfd52a23c1ffbc3dc5fe745381d02b683c","unresolved":false,"context_lines":[{"line_number":2298,"context_line":"        volume_id \u003d volume[\u0027id\u0027]"},{"line_number":2299,"context_line":"        self.volume.create_volume(self.user_context,"},{"line_number":2300,"context_line":"                                  volume_id, volume\u003dvolume)"},{"line_number":2301,"context_line":"        volume_passed \u003d None"},{"line_number":2302,"context_line":"        if volume_object:"},{"line_number":2303,"context_line":"            volume_passed \u003d volume"},{"line_number":2304,"context_line":"        attachment \u003d self.volume.attach_volume(self.user_context,"},{"line_number":2305,"context_line":"                                               volume_id,"},{"line_number":2306,"context_line":"                                               instance_uuid, None,"}],"source_content_type":"text/x-python","patch_set":21,"id":"fa7ab95a_1a04bcc4","line":2303,"range":{"start_line":2301,"start_character":0,"end_line":2303,"end_character":34},"updated":"2016-08-31 10:17:38.000000000","message":"nit:\n\n volume_passed \u003d volume if volume_object else None\n\nOr if you change L2287 to\n\n @ddt.data(None, True)\n\n volume_passed \u003d volume_object and volume","commit_id":"c709cfc7c346f7ae20a92ae0acc21204f916589e"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"d32c6dc475ea8bc93f8327ccdde9a162075e25db","unresolved":false,"context_lines":[{"line_number":2298,"context_line":"        volume_id \u003d volume[\u0027id\u0027]"},{"line_number":2299,"context_line":"        self.volume.create_volume(self.user_context,"},{"line_number":2300,"context_line":"                                  volume_id, volume\u003dvolume)"},{"line_number":2301,"context_line":"        volume_passed \u003d None"},{"line_number":2302,"context_line":"        if volume_object:"},{"line_number":2303,"context_line":"            volume_passed \u003d volume"},{"line_number":2304,"context_line":"        attachment \u003d self.volume.attach_volume(self.user_context,"},{"line_number":2305,"context_line":"                                               volume_id,"},{"line_number":2306,"context_line":"                                               instance_uuid, None,"}],"source_content_type":"text/x-python","patch_set":21,"id":"fa7ab95a_21c14c2f","line":2303,"range":{"start_line":2301,"start_character":0,"end_line":2303,"end_character":34},"in_reply_to":"fa7ab95a_1a04bcc4","updated":"2016-08-31 13:09:40.000000000","message":"Done","commit_id":"c709cfc7c346f7ae20a92ae0acc21204f916589e"}],"cinder/tests/unit/utils.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"24e6bf93da8ed03fa2816d87148e49513d0baca9","unresolved":false,"context_lines":[{"line_number":98,"context_line":"    values[\u0027attach_time\u0027] \u003d now"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    attachment \u003d db.volume_attach(ctxt, values)"},{"line_number":101,"context_line":"    (volume, updated_values) \u003d db.volume_attached("},{"line_number":102,"context_line":"        ctxt, attachment[\u0027id\u0027], instance_uuid,"},{"line_number":103,"context_line":"        attached_host, mountpoint, mode)"},{"line_number":104,"context_line":"    return volume"}],"source_content_type":"text/x-python","patch_set":16,"id":"bacf61ea_bd33e9f5","line":101,"range":{"start_line":101,"start_character":4,"end_line":101,"end_character":28},"updated":"2016-08-02 15:59:42.000000000","message":"nit: Parenthesis are not necessary","commit_id":"11bed8d41658996fd2bef953e8906447c5554559"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"4de5ffb1ba7075a951daa54e7c36cef0a0a754f5","unresolved":false,"context_lines":[{"line_number":98,"context_line":"    values[\u0027attach_time\u0027] \u003d now"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    attachment \u003d db.volume_attach(ctxt, values)"},{"line_number":101,"context_line":"    (volume, updated_values) \u003d db.volume_attached("},{"line_number":102,"context_line":"        ctxt, attachment[\u0027id\u0027], instance_uuid,"},{"line_number":103,"context_line":"        attached_host, mountpoint, mode)"},{"line_number":104,"context_line":"    return volume"}],"source_content_type":"text/x-python","patch_set":16,"id":"bacf61ea_df68388a","line":101,"range":{"start_line":101,"start_character":4,"end_line":101,"end_character":28},"in_reply_to":"bacf61ea_bd33e9f5","updated":"2016-08-03 09:20:57.000000000","message":"Done","commit_id":"11bed8d41658996fd2bef953e8906447c5554559"}],"cinder/volume/manager.py":[{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"f6d72f7051138dc97495ac6daf7d631fd4aec36d","unresolved":false,"context_lines":[{"line_number":888,"context_line":"                                                          host_name_sanitized)"},{"line_number":889,"context_line":"            if attachment is not None:"},{"line_number":890,"context_line":"                updates \u003d {\u0027status\u0027: \u0027in-use\u0027}"},{"line_number":891,"context_line":"                volume.update(updates)"},{"line_number":892,"context_line":"                volume.save()"},{"line_number":893,"context_line":"                return"},{"line_number":894,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba63e191_b9644be5","line":891,"range":{"start_line":891,"start_character":16,"end_line":891,"end_character":22},"updated":"2016-01-19 17:10:43.000000000","message":"volume.status \u003d \u0027in-use\u0027","commit_id":"ecfc2120c2fe7597dbce3b38ba668e5641cfb71b"},{"author":{"_account_id":14274,"name":"XinXiaohui","email":"xiaohui.xin@intel.com","username":"XinXiaohui"},"change_message_id":"1516e762f9d73f7d2d4de32b7c1eb7ffb04a04cb","unresolved":false,"context_lines":[{"line_number":888,"context_line":"                                                          host_name_sanitized)"},{"line_number":889,"context_line":"            if attachment is not None:"},{"line_number":890,"context_line":"                updates \u003d {\u0027status\u0027: \u0027in-use\u0027}"},{"line_number":891,"context_line":"                volume.update(updates)"},{"line_number":892,"context_line":"                volume.save()"},{"line_number":893,"context_line":"                return"},{"line_number":894,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba63e191_26d8161d","line":891,"range":{"start_line":891,"start_character":16,"end_line":891,"end_character":22},"in_reply_to":"ba63e191_b9644be5","updated":"2016-01-20 09:09:40.000000000","message":"Done","commit_id":"ecfc2120c2fe7597dbce3b38ba668e5641cfb71b"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"f6d72f7051138dc97495ac6daf7d631fd4aec36d","unresolved":false,"context_lines":[{"line_number":909,"context_line":"                                                  \u0027error_attaching\u0027})"},{"line_number":910,"context_line":"                raise exception.InvalidUUID(uuid\u003dinstance_uuid)"},{"line_number":911,"context_line":""},{"line_number":912,"context_line":"            volume \u003d objects.Volume.get_by_id(context, volume_id)"},{"line_number":913,"context_line":""},{"line_number":914,"context_line":"            if volume_metadata.get(\u0027readonly\u0027) \u003d\u003d \u0027True\u0027 and mode !\u003d \u0027ro\u0027:"},{"line_number":915,"context_line":"                updates \u003d {\u0027status\u0027: \u0027error_attaching\u0027}"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba63e191_d9a57f04","line":912,"updated":"2016-01-19 17:10:43.000000000","message":"volume.refresh()","commit_id":"ecfc2120c2fe7597dbce3b38ba668e5641cfb71b"},{"author":{"_account_id":14274,"name":"XinXiaohui","email":"xiaohui.xin@intel.com","username":"XinXiaohui"},"change_message_id":"1516e762f9d73f7d2d4de32b7c1eb7ffb04a04cb","unresolved":false,"context_lines":[{"line_number":909,"context_line":"                                                  \u0027error_attaching\u0027})"},{"line_number":910,"context_line":"                raise exception.InvalidUUID(uuid\u003dinstance_uuid)"},{"line_number":911,"context_line":""},{"line_number":912,"context_line":"            volume \u003d objects.Volume.get_by_id(context, volume_id)"},{"line_number":913,"context_line":""},{"line_number":914,"context_line":"            if volume_metadata.get(\u0027readonly\u0027) \u003d\u003d \u0027True\u0027 and mode !\u003d \u0027ro\u0027:"},{"line_number":915,"context_line":"                updates \u003d {\u0027status\u0027: \u0027error_attaching\u0027}"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba63e191_86b78285","line":912,"in_reply_to":"ba63e191_d9a57f04","updated":"2016-01-20 09:09:40.000000000","message":"Done","commit_id":"ecfc2120c2fe7597dbce3b38ba668e5641cfb71b"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"f6d72f7051138dc97495ac6daf7d631fd4aec36d","unresolved":false,"context_lines":[{"line_number":913,"context_line":""},{"line_number":914,"context_line":"            if volume_metadata.get(\u0027readonly\u0027) \u003d\u003d \u0027True\u0027 and mode !\u003d \u0027ro\u0027:"},{"line_number":915,"context_line":"                updates \u003d {\u0027status\u0027: \u0027error_attaching\u0027}"},{"line_number":916,"context_line":"                volume.update(updates)"},{"line_number":917,"context_line":"                volume.save()"},{"line_number":918,"context_line":"                raise exception.InvalidVolumeAttachMode(mode\u003dmode,"},{"line_number":919,"context_line":"                                                        volume_id\u003dvolume_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba63e191_9942a73d","line":916,"updated":"2016-01-19 17:10:43.000000000","message":"You should just use:\nvolume.status \u003d \u0027error_attaching\u0027\n\nEventually, the DictCompat will be removed, so it is better to not use .update() whenever we can.","commit_id":"ecfc2120c2fe7597dbce3b38ba668e5641cfb71b"},{"author":{"_account_id":14274,"name":"XinXiaohui","email":"xiaohui.xin@intel.com","username":"XinXiaohui"},"change_message_id":"1516e762f9d73f7d2d4de32b7c1eb7ffb04a04cb","unresolved":false,"context_lines":[{"line_number":913,"context_line":""},{"line_number":914,"context_line":"            if volume_metadata.get(\u0027readonly\u0027) \u003d\u003d \u0027True\u0027 and mode !\u003d \u0027ro\u0027:"},{"line_number":915,"context_line":"                updates \u003d {\u0027status\u0027: \u0027error_attaching\u0027}"},{"line_number":916,"context_line":"                volume.update(updates)"},{"line_number":917,"context_line":"                volume.save()"},{"line_number":918,"context_line":"                raise exception.InvalidVolumeAttachMode(mode\u003dmode,"},{"line_number":919,"context_line":"                                                        volume_id\u003dvolume_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba63e191_a6bac66c","line":916,"in_reply_to":"ba63e191_9942a73d","updated":"2016-01-20 09:09:40.000000000","message":"Done","commit_id":"ecfc2120c2fe7597dbce3b38ba668e5641cfb71b"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"11e206f99470edf2410f394cefdd9f2007e079ca","unresolved":false,"context_lines":[{"line_number":886,"context_line":"    def attach_volume(self, context, volume_id, instance_uuid, host_name,"},{"line_number":887,"context_line":"                      mountpoint, mode, volume\u003dNone):"},{"line_number":888,"context_line":"        \"\"\"Updates db to show volume is attached.\"\"\""},{"line_number":889,"context_line":"        # FIXME(thangp): Remove this in v3.0 of RPC API."},{"line_number":890,"context_line":"        if volume is None:"},{"line_number":891,"context_line":"            # For older clients, mimic the old behavior and look"},{"line_number":892,"context_line":"            # up the volume by its volume_id."}],"source_content_type":"text/x-python","patch_set":10,"id":"dada55a8_f43573f7","line":889,"range":{"start_line":889,"start_character":16,"end_line":889,"end_character":22},"updated":"2016-07-18 12:41:08.000000000","message":"I think Thang is no longer involved in Cinder, so it may be better to place a name of an active Cinder developer here if you don\u0027t mind.","commit_id":"7080498bb3c19e18a2d22622df1f8b0b0226fff4"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"0c2a55b4094b81db4b7457252930e5defe0da97b","unresolved":false,"context_lines":[{"line_number":886,"context_line":"    def attach_volume(self, context, volume_id, instance_uuid, host_name,"},{"line_number":887,"context_line":"                      mountpoint, mode, volume\u003dNone):"},{"line_number":888,"context_line":"        \"\"\"Updates db to show volume is attached.\"\"\""},{"line_number":889,"context_line":"        # FIXME(thangp): Remove this in v3.0 of RPC API."},{"line_number":890,"context_line":"        if volume is None:"},{"line_number":891,"context_line":"            # For older clients, mimic the old behavior and look"},{"line_number":892,"context_line":"            # up the volume by its volume_id."}],"source_content_type":"text/x-python","patch_set":10,"id":"dada55a8_38067ece","line":889,"range":{"start_line":889,"start_character":16,"end_line":889,"end_character":22},"in_reply_to":"dada55a8_f43573f7","updated":"2016-07-20 02:20:21.000000000","message":"I put my name here.","commit_id":"7080498bb3c19e18a2d22622df1f8b0b0226fff4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"720a6fd8160a8ba6f747b6832c32efd9cc6cb132","unresolved":false,"context_lines":[{"line_number":890,"context_line":"        if volume is None:"},{"line_number":891,"context_line":"            # For older clients, mimic the old behavior and look"},{"line_number":892,"context_line":"            # up the volume by its volume_id."},{"line_number":893,"context_line":"            volume \u003d objects.Volume.get_by_id(context.elevated(), volume_id)"},{"line_number":894,"context_line":"        # check the volume status before attaching"},{"line_number":895,"context_line":"        volume_metadata \u003d volume.admin_metadata"},{"line_number":896,"context_line":"        if volume.status \u003d\u003d \u0027attaching\u0027:"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_76ee9223","line":893,"range":{"start_line":893,"start_character":46,"end_line":893,"end_character":64},"updated":"2016-07-28 10:46:04.000000000","message":"This is NOT the same thing, this is getting the whole volume as admin, and I really don\u0027t like the idea, though I understand this is necessary for updating the admin metadata.  :-(","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"3c11bc21057b1ba8bdee426a1b2e41154d015ad9","unresolved":false,"context_lines":[{"line_number":890,"context_line":"        if volume is None:"},{"line_number":891,"context_line":"            # For older clients, mimic the old behavior and look"},{"line_number":892,"context_line":"            # up the volume by its volume_id."},{"line_number":893,"context_line":"            volume \u003d objects.Volume.get_by_id(context.elevated(), volume_id)"},{"line_number":894,"context_line":"        # check the volume status before attaching"},{"line_number":895,"context_line":"        volume_metadata \u003d volume.admin_metadata"},{"line_number":896,"context_line":"        if volume.status \u003d\u003d \u0027attaching\u0027:"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_d30f572c","line":893,"range":{"start_line":893,"start_character":46,"end_line":893,"end_character":64},"in_reply_to":"bacf61ea_76ee9223","updated":"2016-07-29 07:24:20.000000000","message":"I considered this problem when I did the update. But I can\u0027t find a better way to do it.","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"270082dbe0912a8cb7e1109fc705de8702c01e1e","unresolved":false,"context_lines":[{"line_number":890,"context_line":"        if volume is None:"},{"line_number":891,"context_line":"            # For older clients, mimic the old behavior and look"},{"line_number":892,"context_line":"            # up the volume by its volume_id."},{"line_number":893,"context_line":"            volume \u003d objects.Volume.get_by_id(context.elevated(), volume_id)"},{"line_number":894,"context_line":"        # check the volume status before attaching"},{"line_number":895,"context_line":"        volume_metadata \u003d volume.admin_metadata"},{"line_number":896,"context_line":"        if volume.status \u003d\u003d \u0027attaching\u0027:"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_4d7a9bf2","line":893,"range":{"start_line":893,"start_character":46,"end_line":893,"end_character":64},"in_reply_to":"bacf61ea_d30f572c","updated":"2016-07-29 12:22:25.000000000","message":"That\u0027s why I didn\u0027t -1 for that one, because I considere it as an acceptable compromise.","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"720a6fd8160a8ba6f747b6832c32efd9cc6cb132","unresolved":false,"context_lines":[{"line_number":934,"context_line":"            attachment.save()"},{"line_number":935,"context_line":"            raise exception.InvalidUUID(uuid\u003dinstance_uuid)"},{"line_number":936,"context_line":""},{"line_number":937,"context_line":"        volume.refresh()"},{"line_number":938,"context_line":""},{"line_number":939,"context_line":"        if volume_metadata.get(\u0027readonly\u0027) \u003d\u003d \u0027True\u0027 and mode !\u003d \u0027ro\u0027:"},{"line_number":940,"context_line":"            volume.status \u003d \u0027error_attaching\u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_5621cefa","line":937,"updated":"2016-07-28 10:46:04.000000000","message":"-1: Why is this refresh necessary anymore?","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"3c11bc21057b1ba8bdee426a1b2e41154d015ad9","unresolved":false,"context_lines":[{"line_number":934,"context_line":"            attachment.save()"},{"line_number":935,"context_line":"            raise exception.InvalidUUID(uuid\u003dinstance_uuid)"},{"line_number":936,"context_line":""},{"line_number":937,"context_line":"        volume.refresh()"},{"line_number":938,"context_line":""},{"line_number":939,"context_line":"        if volume_metadata.get(\u0027readonly\u0027) \u003d\u003d \u0027True\u0027 and mode !\u003d \u0027ro\u0027:"},{"line_number":940,"context_line":"            volume.status \u003d \u0027error_attaching\u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_61317805","line":937,"in_reply_to":"bacf61ea_5621cefa","updated":"2016-07-29 07:24:20.000000000","message":"No need to refresh.","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"720a6fd8160a8ba6f747b6832c32efd9cc6cb132","unresolved":false,"context_lines":[{"line_number":955,"context_line":"            LOG.debug(\u0027Attaching volume %(volume_id)s to instance \u0027"},{"line_number":956,"context_line":"                      \u0027%(instance)s at mountpoint %(mount)s on host \u0027"},{"line_number":957,"context_line":"                      \u0027%(host)s.\u0027,"},{"line_number":958,"context_line":"                      {\u0027volume_id\u0027: volume_id, \u0027instance\u0027: instance_uuid,"},{"line_number":959,"context_line":"                       \u0027mount\u0027: mountpoint, \u0027host\u0027: host_name_sanitized},"},{"line_number":960,"context_line":"                      resource\u003dvolume)"},{"line_number":961,"context_line":"            self.driver.attach_volume(context,"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_16dc76e3","line":958,"range":{"start_line":958,"start_character":36,"end_line":958,"end_character":45},"updated":"2016-07-28 10:46:04.000000000","message":"nit: We should be using volume.id instead","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"3c11bc21057b1ba8bdee426a1b2e41154d015ad9","unresolved":false,"context_lines":[{"line_number":955,"context_line":"            LOG.debug(\u0027Attaching volume %(volume_id)s to instance \u0027"},{"line_number":956,"context_line":"                      \u0027%(instance)s at mountpoint %(mount)s on host \u0027"},{"line_number":957,"context_line":"                      \u0027%(host)s.\u0027,"},{"line_number":958,"context_line":"                      {\u0027volume_id\u0027: volume_id, \u0027instance\u0027: instance_uuid,"},{"line_number":959,"context_line":"                       \u0027mount\u0027: mountpoint, \u0027host\u0027: host_name_sanitized},"},{"line_number":960,"context_line":"                      resource\u003dvolume)"},{"line_number":961,"context_line":"            self.driver.attach_volume(context,"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_c1166454","line":958,"range":{"start_line":958,"start_character":36,"end_line":958,"end_character":45},"in_reply_to":"bacf61ea_16dc76e3","updated":"2016-07-29 07:24:20.000000000","message":"volume_id is passed in Line 886. Anyway I updated it.","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"270082dbe0912a8cb7e1109fc705de8702c01e1e","unresolved":false,"context_lines":[{"line_number":955,"context_line":"            LOG.debug(\u0027Attaching volume %(volume_id)s to instance \u0027"},{"line_number":956,"context_line":"                      \u0027%(instance)s at mountpoint %(mount)s on host \u0027"},{"line_number":957,"context_line":"                      \u0027%(host)s.\u0027,"},{"line_number":958,"context_line":"                      {\u0027volume_id\u0027: volume_id, \u0027instance\u0027: instance_uuid,"},{"line_number":959,"context_line":"                       \u0027mount\u0027: mountpoint, \u0027host\u0027: host_name_sanitized},"},{"line_number":960,"context_line":"                      resource\u003dvolume)"},{"line_number":961,"context_line":"            self.driver.attach_volume(context,"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_cd55eb50","line":958,"range":{"start_line":958,"start_character":36,"end_line":958,"end_character":45},"in_reply_to":"bacf61ea_c1166454","updated":"2016-07-29 12:22:25.000000000","message":"I know it\u0027s passed down, but at some point we\u0027ll want to remove it, so it\u0027s better to move everything to use the volume object.  It\u0027s not really changed in the next patch, but it\u0027s just a nit, so not important.","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"d16ee1e379ce0edf2e49ad3ae2a58dd85cd005e3","unresolved":false,"context_lines":[{"line_number":955,"context_line":"            LOG.debug(\u0027Attaching volume %(volume_id)s to instance \u0027"},{"line_number":956,"context_line":"                      \u0027%(instance)s at mountpoint %(mount)s on host \u0027"},{"line_number":957,"context_line":"                      \u0027%(host)s.\u0027,"},{"line_number":958,"context_line":"                      {\u0027volume_id\u0027: volume_id, \u0027instance\u0027: instance_uuid,"},{"line_number":959,"context_line":"                       \u0027mount\u0027: mountpoint, \u0027host\u0027: host_name_sanitized},"},{"line_number":960,"context_line":"                      resource\u003dvolume)"},{"line_number":961,"context_line":"            self.driver.attach_volume(context,"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_608124c3","line":958,"range":{"start_line":958,"start_character":36,"end_line":958,"end_character":45},"in_reply_to":"bacf61ea_cd55eb50","updated":"2016-08-01 07:57:13.000000000","message":"Updated.","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"720a6fd8160a8ba6f747b6832c32efd9cc6cb132","unresolved":false,"context_lines":[{"line_number":977,"context_line":"        self._notify_about_volume_usage(context, volume, \"attach.end\")"},{"line_number":978,"context_line":"        LOG.info(_LI(\"Attach volume completed successfully.\"),"},{"line_number":979,"context_line":"                 resource\u003dvolume)"},{"line_number":980,"context_line":"        attachment.refresh()"},{"line_number":981,"context_line":"        return attachment"},{"line_number":982,"context_line":""},{"line_number":983,"context_line":"    @coordination.synchronized(\u0027{volume_id}-{f_name}\u0027)"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_76369204","line":980,"updated":"2016-07-28 10:46:04.000000000","message":"-1: If update_attached actually updated the OVO this would not be necessary.","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"3c11bc21057b1ba8bdee426a1b2e41154d015ad9","unresolved":false,"context_lines":[{"line_number":977,"context_line":"        self._notify_about_volume_usage(context, volume, \"attach.end\")"},{"line_number":978,"context_line":"        LOG.info(_LI(\"Attach volume completed successfully.\"),"},{"line_number":979,"context_line":"                 resource\u003dvolume)"},{"line_number":980,"context_line":"        attachment.refresh()"},{"line_number":981,"context_line":"        return attachment"},{"line_number":982,"context_line":""},{"line_number":983,"context_line":"    @coordination.synchronized(\u0027{volume_id}-{f_name}\u0027)"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_01099c31","line":980,"in_reply_to":"bacf61ea_76369204","updated":"2016-07-29 07:24:20.000000000","message":"No need to refresh.","commit_id":"75b58b60cb0c1ed9ba8d823756e34bc5b7e4faa6"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"37841d4e21a328efb8927a6837a90aef6b238870","unresolved":false,"context_lines":[{"line_number":912,"context_line":"            # up the volume by its volume_id."},{"line_number":913,"context_line":"            volume \u003d objects.Volume.get_by_id(context, volume_id)"},{"line_number":914,"context_line":"        # check the volume status before attaching"},{"line_number":915,"context_line":"        volume_metadata \u003d self.db.volume_admin_metadata_get("},{"line_number":916,"context_line":"            context.elevated(), volume_id)"},{"line_number":917,"context_line":"        if volume.status \u003d\u003d \u0027attaching\u0027:"},{"line_number":918,"context_line":"            if (volume_metadata.get(\u0027attached_mode\u0027) and"},{"line_number":919,"context_line":"               volume_metadata.get(\u0027attached_mode\u0027) !\u003d mode):"}],"source_content_type":"text/x-python","patch_set":16,"id":"bacf61ea_02b630f8","line":916,"range":{"start_line":915,"start_character":0,"end_line":916,"end_character":42},"updated":"2016-08-02 08:32:19.000000000","message":"I use db.volume_admin_metadata_get to get volume admin_metadata instead of volume.admin_metadata as it needs admin context. \n\nWhen volume passed down has no admin context, I didn\u0027t find a better way to transfer it to volume with admin context.\n\nAny suggestions?","commit_id":"11bed8d41658996fd2bef953e8906447c5554559"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"24e6bf93da8ed03fa2816d87148e49513d0baca9","unresolved":false,"context_lines":[{"line_number":912,"context_line":"            # up the volume by its volume_id."},{"line_number":913,"context_line":"            volume \u003d objects.Volume.get_by_id(context, volume_id)"},{"line_number":914,"context_line":"        # check the volume status before attaching"},{"line_number":915,"context_line":"        volume_metadata \u003d self.db.volume_admin_metadata_get("},{"line_number":916,"context_line":"            context.elevated(), volume_id)"},{"line_number":917,"context_line":"        if volume.status \u003d\u003d \u0027attaching\u0027:"},{"line_number":918,"context_line":"            if (volume_metadata.get(\u0027attached_mode\u0027) and"},{"line_number":919,"context_line":"               volume_metadata.get(\u0027attached_mode\u0027) !\u003d mode):"}],"source_content_type":"text/x-python","patch_set":16,"id":"bacf61ea_fa99e7d1","line":916,"range":{"start_line":915,"start_character":0,"end_line":916,"end_character":42},"in_reply_to":"bacf61ea_02b630f8","updated":"2016-08-02 15:59:42.000000000","message":"-1: You can use the same pattern you used in the volume attachment OVO:\n\n with volume.obj_as_admin():\n     volume_metadata \u003d volume.admin_metadata\n\nBut I really don\u0027t understand why attach methods require the admin context since any user can do an attach/detach operation and we just keep elevating the context in the code.  But that\u0027s outside the scope of this patch.","commit_id":"11bed8d41658996fd2bef953e8906447c5554559"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"4de5ffb1ba7075a951daa54e7c36cef0a0a754f5","unresolved":false,"context_lines":[{"line_number":912,"context_line":"            # up the volume by its volume_id."},{"line_number":913,"context_line":"            volume \u003d objects.Volume.get_by_id(context, volume_id)"},{"line_number":914,"context_line":"        # check the volume status before attaching"},{"line_number":915,"context_line":"        volume_metadata \u003d self.db.volume_admin_metadata_get("},{"line_number":916,"context_line":"            context.elevated(), volume_id)"},{"line_number":917,"context_line":"        if volume.status \u003d\u003d \u0027attaching\u0027:"},{"line_number":918,"context_line":"            if (volume_metadata.get(\u0027attached_mode\u0027) and"},{"line_number":919,"context_line":"               volume_metadata.get(\u0027attached_mode\u0027) !\u003d mode):"}],"source_content_type":"text/x-python","patch_set":16,"id":"bacf61ea_ff753c32","line":916,"range":{"start_line":915,"start_character":0,"end_line":916,"end_character":42},"in_reply_to":"bacf61ea_fa99e7d1","updated":"2016-08-03 09:20:57.000000000","message":"Thank you for the suggestions.\n\nAgree with you that no need admin context for attach/detach.","commit_id":"11bed8d41658996fd2bef953e8906447c5554559"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"24e6bf93da8ed03fa2816d87148e49513d0baca9","unresolved":false,"context_lines":[{"line_number":944,"context_line":"                                        \"attach.start\")"},{"line_number":945,"context_line":""},{"line_number":946,"context_line":"        attachment \u003d volume.begin_attach()"},{"line_number":947,"context_line":"        volume_metadata \u003d self.db.volume_admin_metadata_update("},{"line_number":948,"context_line":"            context.elevated(), volume_id,"},{"line_number":949,"context_line":"            {\"attached_mode\": mode}, False)"},{"line_number":950,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"bacf61ea_9a51cbdb","line":947,"updated":"2016-08-02 15:59:42.000000000","message":"-1: Same here:\n\n with volume.obj_as_admin():\n     volume.admin_metadata[\u0027attached_mode\u0027] \u003d mode\n     volume.save()","commit_id":"11bed8d41658996fd2bef953e8906447c5554559"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"4de5ffb1ba7075a951daa54e7c36cef0a0a754f5","unresolved":false,"context_lines":[{"line_number":944,"context_line":"                                        \"attach.start\")"},{"line_number":945,"context_line":""},{"line_number":946,"context_line":"        attachment \u003d volume.begin_attach()"},{"line_number":947,"context_line":"        volume_metadata \u003d self.db.volume_admin_metadata_update("},{"line_number":948,"context_line":"            context.elevated(), volume_id,"},{"line_number":949,"context_line":"            {\"attached_mode\": mode}, False)"},{"line_number":950,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"bacf61ea_df72f848","line":947,"in_reply_to":"bacf61ea_9a51cbdb","updated":"2016-08-03 09:20:57.000000000","message":"Done","commit_id":"11bed8d41658996fd2bef953e8906447c5554559"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"1b9ee1cfd52a23c1ffbc3dc5fe745381d02b683c","unresolved":false,"context_lines":[{"line_number":982,"context_line":""},{"line_number":983,"context_line":"        attachment \u003d volume.begin_attach()"},{"line_number":984,"context_line":"        # Update admin_metadata. This needs admin context."},{"line_number":985,"context_line":"        with volume.obj_as_admin():"},{"line_number":986,"context_line":"            volume.admin_metadata[\u0027attached_mode\u0027] \u003d mode"},{"line_number":987,"context_line":"            volume.save()"},{"line_number":988,"context_line":""},{"line_number":989,"context_line":"        if instance_uuid and not uuidutils.is_uuid_like(instance_uuid):"},{"line_number":990,"context_line":"            attachment.attach_status \u003d \u0027error_attaching\u0027"}],"source_content_type":"text/x-python","patch_set":21,"id":"fa7ab95a_57437d6c","line":987,"range":{"start_line":985,"start_character":0,"end_line":987,"end_character":25},"updated":"2016-08-31 10:17:38.000000000","message":"-1: This should go in begin_attach(mode)","commit_id":"c709cfc7c346f7ae20a92ae0acc21204f916589e"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"d32c6dc475ea8bc93f8327ccdde9a162075e25db","unresolved":false,"context_lines":[{"line_number":982,"context_line":""},{"line_number":983,"context_line":"        attachment \u003d volume.begin_attach()"},{"line_number":984,"context_line":"        # Update admin_metadata. This needs admin context."},{"line_number":985,"context_line":"        with volume.obj_as_admin():"},{"line_number":986,"context_line":"            volume.admin_metadata[\u0027attached_mode\u0027] \u003d mode"},{"line_number":987,"context_line":"            volume.save()"},{"line_number":988,"context_line":""},{"line_number":989,"context_line":"        if instance_uuid and not uuidutils.is_uuid_like(instance_uuid):"},{"line_number":990,"context_line":"            attachment.attach_status \u003d \u0027error_attaching\u0027"}],"source_content_type":"text/x-python","patch_set":21,"id":"fa7ab95a_44365eb5","line":987,"range":{"start_line":985,"start_character":0,"end_line":987,"end_character":25},"in_reply_to":"fa7ab95a_57437d6c","updated":"2016-08-31 13:09:40.000000000","message":"This is reasonable.","commit_id":"c709cfc7c346f7ae20a92ae0acc21204f916589e"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"0a464bdfa8136e690f9f72d3d8a073c507e684e3","unresolved":false,"context_lines":[{"line_number":918,"context_line":""},{"line_number":919,"context_line":"    @coordination.synchronized(\u0027{volume_id}\u0027)"},{"line_number":920,"context_line":"    def attach_volume(self, context, volume_id, instance_uuid, host_name,"},{"line_number":921,"context_line":"                      mountpoint, mode, volume\u003dNone):"},{"line_number":922,"context_line":"        \"\"\"Updates db to show volume is attached.\"\"\""},{"line_number":923,"context_line":"        # FIXME(lixiaoy1): Remove this in v4.0 of RPC API."},{"line_number":924,"context_line":"        if volume is None:"}],"source_content_type":"text/x-python","patch_set":26,"id":"7a77a97e_1d48252d","line":921,"updated":"2016-11-22 07:51:02.000000000","message":"I\u0027m not suggesting removing \u0027volume_id\u0027, rather I\u0027d make it optional.  If \u0027volume\u0027 is None, then you check for \u0027volume_id\u0027. It won\u0027t hurt rolling upgrade at all because we are talking about the new interface on new service.","commit_id":"85c9fefa29be379af14e1db744546c90121fc61b"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"8cf9183af1a7db7639d4710964e874063a02aed0","unresolved":false,"context_lines":[{"line_number":918,"context_line":""},{"line_number":919,"context_line":"    @coordination.synchronized(\u0027{volume_id}\u0027)"},{"line_number":920,"context_line":"    def attach_volume(self, context, volume_id, instance_uuid, host_name,"},{"line_number":921,"context_line":"                      mountpoint, mode, volume\u003dNone):"},{"line_number":922,"context_line":"        \"\"\"Updates db to show volume is attached.\"\"\""},{"line_number":923,"context_line":"        # FIXME(lixiaoy1): Remove this in v4.0 of RPC API."},{"line_number":924,"context_line":"        if volume is None:"}],"source_content_type":"text/x-python","patch_set":26,"id":"7a77a97e_95ced7b2","line":921,"updated":"2016-11-22 01:10:40.000000000","message":"volume_id is redundant info, we may change volume_id to be an optional parameter.","commit_id":"85c9fefa29be379af14e1db744546c90121fc61b"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"40ff4966d1d3888d2c72f7ed19a7b3e4a7d65291","unresolved":false,"context_lines":[{"line_number":918,"context_line":""},{"line_number":919,"context_line":"    @coordination.synchronized(\u0027{volume_id}\u0027)"},{"line_number":920,"context_line":"    def attach_volume(self, context, volume_id, instance_uuid, host_name,"},{"line_number":921,"context_line":"                      mountpoint, mode, volume\u003dNone):"},{"line_number":922,"context_line":"        \"\"\"Updates db to show volume is attached.\"\"\""},{"line_number":923,"context_line":"        # FIXME(lixiaoy1): Remove this in v4.0 of RPC API."},{"line_number":924,"context_line":"        if volume is None:"}],"source_content_type":"text/x-python","patch_set":26,"id":"7a77a97e_95b417d2","line":921,"in_reply_to":"7a77a97e_95ced7b2","updated":"2016-11-22 01:33:50.000000000","message":"To make sure rolling upgrade succeed, volume_id should not be optional. Think about some services are in old release, some are in new release, if sending out a request without volume_id to an old service, error happens. To prevent some problems we keep volume_id. In N+2 releases, the volume_id parameter will be removed, like create_snapshot() etc.","commit_id":"85c9fefa29be379af14e1db744546c90121fc61b"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"20d88c70f3870f1367aa364261780769732f486d","unresolved":false,"context_lines":[{"line_number":971,"context_line":"        attachment \u003d volume.begin_attach(mode)"},{"line_number":972,"context_line":""},{"line_number":973,"context_line":"        if instance_uuid and not uuidutils.is_uuid_like(instance_uuid):"},{"line_number":974,"context_line":"            attachment.attach_status \u003d \u0027error_attaching\u0027"},{"line_number":975,"context_line":"            attachment.save()"},{"line_number":976,"context_line":"            raise exception.InvalidUUID(uuid\u003dinstance_uuid)"},{"line_number":977,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"7a77a97e_64417639","line":974,"range":{"start_line":974,"start_character":39,"end_line":974,"end_character":56},"updated":"2016-11-22 11:07:11.000000000","message":"We should use VolumeAttachStatus here.","commit_id":"6113a028ee70499767f8875ccc99d451488c5266"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"6bc1b8364dc14fe1e0e9fd8a3e53d2075a6a3d35","unresolved":false,"context_lines":[{"line_number":971,"context_line":"        attachment \u003d volume.begin_attach(mode)"},{"line_number":972,"context_line":""},{"line_number":973,"context_line":"        if instance_uuid and not uuidutils.is_uuid_like(instance_uuid):"},{"line_number":974,"context_line":"            attachment.attach_status \u003d \u0027error_attaching\u0027"},{"line_number":975,"context_line":"            attachment.save()"},{"line_number":976,"context_line":"            raise exception.InvalidUUID(uuid\u003dinstance_uuid)"},{"line_number":977,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"5a74a57a_78045917","line":974,"range":{"start_line":974,"start_character":39,"end_line":974,"end_character":56},"in_reply_to":"7a77a97e_64417639","updated":"2016-11-28 03:45:03.000000000","message":"Done","commit_id":"6113a028ee70499767f8875ccc99d451488c5266"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"20d88c70f3870f1367aa364261780769732f486d","unresolved":false,"context_lines":[{"line_number":975,"context_line":"            attachment.save()"},{"line_number":976,"context_line":"            raise exception.InvalidUUID(uuid\u003dinstance_uuid)"},{"line_number":977,"context_line":""},{"line_number":978,"context_line":"        if volume_metadata.get(\u0027readonly\u0027) \u003d\u003d \u0027True\u0027 and mode !\u003d \u0027ro\u0027:"},{"line_number":979,"context_line":"            volume.status \u003d \u0027error_attaching\u0027"},{"line_number":980,"context_line":"            volume.save()"},{"line_number":981,"context_line":"            self.message_api.create("},{"line_number":982,"context_line":"                context, defined_messages.ATTACH_READONLY_VOLUME,"}],"source_content_type":"text/x-python","patch_set":27,"id":"7a77a97e_843cd2c0","line":979,"range":{"start_line":978,"start_character":12,"end_line":979,"end_character":45},"updated":"2016-11-22 11:07:11.000000000","message":"And *not* use it here.\n\nBTW - shouldn\u0027t we set volume.attach_status here as well?","commit_id":"6113a028ee70499767f8875ccc99d451488c5266"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"d7eae94e3292730743c8f326f207fae2b387b304","unresolved":false,"context_lines":[{"line_number":975,"context_line":"            attachment.save()"},{"line_number":976,"context_line":"            raise exception.InvalidUUID(uuid\u003dinstance_uuid)"},{"line_number":977,"context_line":""},{"line_number":978,"context_line":"        if volume_metadata.get(\u0027readonly\u0027) \u003d\u003d \u0027True\u0027 and mode !\u003d \u0027ro\u0027:"},{"line_number":979,"context_line":"            volume.status \u003d \u0027error_attaching\u0027"},{"line_number":980,"context_line":"            volume.save()"},{"line_number":981,"context_line":"            self.message_api.create("},{"line_number":982,"context_line":"                context, defined_messages.ATTACH_READONLY_VOLUME,"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a74a57a_52abf974","line":979,"range":{"start_line":978,"start_character":12,"end_line":979,"end_character":45},"in_reply_to":"5a74a57a_c3001e04","updated":"2016-11-28 05:49:12.000000000","message":"I didn\u0027t find any places to set volume.attach_status: I mean in the attaching progress, Cinder doesn\u0027t set volume.attach_status, just set volume.attach_status as attached in db.volume_attached() in the end of attach.","commit_id":"6113a028ee70499767f8875ccc99d451488c5266"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"6bc1b8364dc14fe1e0e9fd8a3e53d2075a6a3d35","unresolved":false,"context_lines":[{"line_number":975,"context_line":"            attachment.save()"},{"line_number":976,"context_line":"            raise exception.InvalidUUID(uuid\u003dinstance_uuid)"},{"line_number":977,"context_line":""},{"line_number":978,"context_line":"        if volume_metadata.get(\u0027readonly\u0027) \u003d\u003d \u0027True\u0027 and mode !\u003d \u0027ro\u0027:"},{"line_number":979,"context_line":"            volume.status \u003d \u0027error_attaching\u0027"},{"line_number":980,"context_line":"            volume.save()"},{"line_number":981,"context_line":"            self.message_api.create("},{"line_number":982,"context_line":"                context, defined_messages.ATTACH_READONLY_VOLUME,"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a74a57a_c3001e04","line":979,"range":{"start_line":978,"start_character":12,"end_line":979,"end_character":45},"in_reply_to":"7a77a97e_843cd2c0","updated":"2016-11-28 03:45:03.000000000","message":"Done.\nI didn\u0027t find any places to set volume.attach_status, and in this function it sets volume_attachment.status as attaching in db.volume_attach().\n\nHere should attachment.status instead of volume.attach_status.","commit_id":"6113a028ee70499767f8875ccc99d451488c5266"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"20d88c70f3870f1367aa364261780769732f486d","unresolved":false,"context_lines":[{"line_number":1004,"context_line":"                                      mountpoint)"},{"line_number":1005,"context_line":"        except Exception:"},{"line_number":1006,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1007,"context_line":"                attachment.attach_status \u003d \u0027error_attaching\u0027"},{"line_number":1008,"context_line":"                attachment.save()"},{"line_number":1009,"context_line":""},{"line_number":1010,"context_line":"        volume \u003d attachment.finish_attach("}],"source_content_type":"text/x-python","patch_set":27,"id":"7a77a97e_84d85281","line":1007,"range":{"start_line":1007,"start_character":16,"end_line":1007,"end_character":60},"updated":"2016-11-22 11:07:11.000000000","message":"But use it here. :)","commit_id":"6113a028ee70499767f8875ccc99d451488c5266"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"6bc1b8364dc14fe1e0e9fd8a3e53d2075a6a3d35","unresolved":false,"context_lines":[{"line_number":1004,"context_line":"                                      mountpoint)"},{"line_number":1005,"context_line":"        except Exception:"},{"line_number":1006,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1007,"context_line":"                attachment.attach_status \u003d \u0027error_attaching\u0027"},{"line_number":1008,"context_line":"                attachment.save()"},{"line_number":1009,"context_line":""},{"line_number":1010,"context_line":"        volume \u003d attachment.finish_attach("}],"source_content_type":"text/x-python","patch_set":27,"id":"5a74a57a_630bca2a","line":1007,"range":{"start_line":1007,"start_character":16,"end_line":1007,"end_character":60},"in_reply_to":"7a77a97e_84d85281","updated":"2016-11-28 03:45:03.000000000","message":"Done","commit_id":"6113a028ee70499767f8875ccc99d451488c5266"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"20d88c70f3870f1367aa364261780769732f486d","unresolved":false,"context_lines":[{"line_number":1016,"context_line":"        self._notify_about_volume_usage(context, volume, \"attach.end\")"},{"line_number":1017,"context_line":"        LOG.info(_LI(\"Attach volume completed successfully.\"),"},{"line_number":1018,"context_line":"                 resource\u003dvolume)"},{"line_number":1019,"context_line":"        return attachment"},{"line_number":1020,"context_line":""},{"line_number":1021,"context_line":"    @coordination.synchronized(\u0027{volume_id}-{f_name}\u0027)"},{"line_number":1022,"context_line":"    def detach_volume(self, context, volume_id, attachment_id\u003dNone):"}],"source_content_type":"text/x-python","patch_set":27,"id":"7a77a97e_e42586ae","line":1019,"range":{"start_line":1019,"start_character":0,"end_line":1019,"end_character":25},"updated":"2016-11-22 11:07:11.000000000","message":"Oh, this is interesting part. So we\u0027re changing what\u0027s returned from RPC API and normally this would be dangerous. In this case this is safe, as Newton has VolumeAttachment object and will deserialize it correctly.","commit_id":"6113a028ee70499767f8875ccc99d451488c5266"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"6bc1b8364dc14fe1e0e9fd8a3e53d2075a6a3d35","unresolved":false,"context_lines":[{"line_number":1016,"context_line":"        self._notify_about_volume_usage(context, volume, \"attach.end\")"},{"line_number":1017,"context_line":"        LOG.info(_LI(\"Attach volume completed successfully.\"),"},{"line_number":1018,"context_line":"                 resource\u003dvolume)"},{"line_number":1019,"context_line":"        return attachment"},{"line_number":1020,"context_line":""},{"line_number":1021,"context_line":"    @coordination.synchronized(\u0027{volume_id}-{f_name}\u0027)"},{"line_number":1022,"context_line":"    def detach_volume(self, context, volume_id, attachment_id\u003dNone):"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a74a57a_a3016208","line":1019,"range":{"start_line":1019,"start_character":0,"end_line":1019,"end_character":25},"in_reply_to":"7a77a97e_e42586ae","updated":"2016-11-28 03:45:03.000000000","message":"Yeah, that was also what I thought.","commit_id":"6113a028ee70499767f8875ccc99d451488c5266"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"4ffb8b745f0b13fcc621f11a1372e50e8d66f3fa","unresolved":false,"context_lines":[{"line_number":971,"context_line":"        attachment \u003d volume.begin_attach(mode)"},{"line_number":972,"context_line":""},{"line_number":973,"context_line":"        if instance_uuid and not uuidutils.is_uuid_like(instance_uuid):"},{"line_number":974,"context_line":"            attachment.attach_status \u003d \\"},{"line_number":975,"context_line":"                fields.VolumeAttachStatus.ERROR_ATTACHING"},{"line_number":976,"context_line":"            attachment.save()"},{"line_number":977,"context_line":"            raise exception.InvalidUUID(uuid\u003dinstance_uuid)"}],"source_content_type":"text/x-python","patch_set":28,"id":"5a74a57a_1faf1de4","line":974,"range":{"start_line":974,"start_character":39,"end_line":974,"end_character":40},"updated":"2016-12-01 01:42:54.000000000","message":"Can you wrap this assignment in () to avoid using \\? Pretty trivial thing, but we have made an effort to get rid of using \\ for spanning multiple lines, so we should be consistent.\n\nattachment.attach_status \u003d (\n    fields.xxx)","commit_id":"32b1d0676ca7ce76e294800f5745ed9c2281bff9"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"f979e2e86ed6663bb1fe911b92333b47f830a8ec","unresolved":false,"context_lines":[{"line_number":971,"context_line":"        attachment \u003d volume.begin_attach(mode)"},{"line_number":972,"context_line":""},{"line_number":973,"context_line":"        if instance_uuid and not uuidutils.is_uuid_like(instance_uuid):"},{"line_number":974,"context_line":"            attachment.attach_status \u003d \\"},{"line_number":975,"context_line":"                fields.VolumeAttachStatus.ERROR_ATTACHING"},{"line_number":976,"context_line":"            attachment.save()"},{"line_number":977,"context_line":"            raise exception.InvalidUUID(uuid\u003dinstance_uuid)"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a71b18c_979446e9","line":974,"range":{"start_line":974,"start_character":39,"end_line":974,"end_character":40},"in_reply_to":"5a74a57a_1faf1de4","updated":"2016-12-01 08:13:13.000000000","message":"Done","commit_id":"32b1d0676ca7ce76e294800f5745ed9c2281bff9"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"4ffb8b745f0b13fcc621f11a1372e50e8d66f3fa","unresolved":false,"context_lines":[{"line_number":977,"context_line":"            raise exception.InvalidUUID(uuid\u003dinstance_uuid)"},{"line_number":978,"context_line":""},{"line_number":979,"context_line":"        if volume_metadata.get(\u0027readonly\u0027) \u003d\u003d \u0027True\u0027 and mode !\u003d \u0027ro\u0027:"},{"line_number":980,"context_line":"            attachment.attach_status \u003d \\"},{"line_number":981,"context_line":"                fields.VolumeAttachStatus.ERROR_ATTACHING"},{"line_number":982,"context_line":"            attachment.save()"},{"line_number":983,"context_line":"            self.message_api.create("}],"source_content_type":"text/x-python","patch_set":28,"id":"5a74a57a_3faa19d3","line":980,"range":{"start_line":980,"start_character":39,"end_line":980,"end_character":40},"updated":"2016-12-01 01:42:54.000000000","message":"Same.","commit_id":"32b1d0676ca7ce76e294800f5745ed9c2281bff9"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"f979e2e86ed6663bb1fe911b92333b47f830a8ec","unresolved":false,"context_lines":[{"line_number":977,"context_line":"            raise exception.InvalidUUID(uuid\u003dinstance_uuid)"},{"line_number":978,"context_line":""},{"line_number":979,"context_line":"        if volume_metadata.get(\u0027readonly\u0027) \u003d\u003d \u0027True\u0027 and mode !\u003d \u0027ro\u0027:"},{"line_number":980,"context_line":"            attachment.attach_status \u003d \\"},{"line_number":981,"context_line":"                fields.VolumeAttachStatus.ERROR_ATTACHING"},{"line_number":982,"context_line":"            attachment.save()"},{"line_number":983,"context_line":"            self.message_api.create("}],"source_content_type":"text/x-python","patch_set":28,"id":"3a71b18c_f7a3ba4d","line":980,"range":{"start_line":980,"start_character":39,"end_line":980,"end_character":40},"in_reply_to":"5a74a57a_3faa19d3","updated":"2016-12-01 08:13:13.000000000","message":"Done","commit_id":"32b1d0676ca7ce76e294800f5745ed9c2281bff9"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"4ffb8b745f0b13fcc621f11a1372e50e8d66f3fa","unresolved":false,"context_lines":[{"line_number":1006,"context_line":"                                      mountpoint)"},{"line_number":1007,"context_line":"        except Exception:"},{"line_number":1008,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1009,"context_line":"                attachment.attach_status \u003d \\"},{"line_number":1010,"context_line":"                    fields.VolumeAttachStatus.ERROR_ATTACHING"},{"line_number":1011,"context_line":"                attachment.save()"},{"line_number":1012,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"5a74a57a_5fb9952d","line":1009,"range":{"start_line":1009,"start_character":43,"end_line":1009,"end_character":44},"updated":"2016-12-01 01:42:54.000000000","message":"Same.","commit_id":"32b1d0676ca7ce76e294800f5745ed9c2281bff9"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"f979e2e86ed6663bb1fe911b92333b47f830a8ec","unresolved":false,"context_lines":[{"line_number":1006,"context_line":"                                      mountpoint)"},{"line_number":1007,"context_line":"        except Exception:"},{"line_number":1008,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1009,"context_line":"                attachment.attach_status \u003d \\"},{"line_number":1010,"context_line":"                    fields.VolumeAttachStatus.ERROR_ATTACHING"},{"line_number":1011,"context_line":"                attachment.save()"},{"line_number":1012,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"3a71b18c_378612ba","line":1009,"range":{"start_line":1009,"start_character":43,"end_line":1009,"end_character":44},"in_reply_to":"5a74a57a_5fb9952d","updated":"2016-12-01 08:13:13.000000000","message":"Done","commit_id":"32b1d0676ca7ce76e294800f5745ed9c2281bff9"}],"cinder/volume/rpcapi.py":[{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"8cf9183af1a7db7639d4710964e874063a02aed0","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                    \u0027volume\u0027: volume}"},{"line_number":195,"context_line":"        cctxt \u003d self._get_cctxt(volume.host, (\u00273.2\u0027, \u00273.0\u0027))"},{"line_number":196,"context_line":"        if not cctxt.can_send_version(\u00273.2\u0027):"},{"line_number":197,"context_line":"            msg_args.pop(\u0027volume\u0027)"},{"line_number":198,"context_line":"        return cctxt.call(ctxt, \u0027attach_volume\u0027, **msg_args)"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    def detach_volume(self, ctxt, volume, attachment_id):"}],"source_content_type":"text/x-python","patch_set":26,"id":"7a77a97e_b5c3d389","line":197,"updated":"2016-11-22 01:10:40.000000000","message":"Should we push one step forward and for clients are compatible with 3.2, we remove volumd_id from msg_args since its redundent?","commit_id":"85c9fefa29be379af14e1db744546c90121fc61b"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"0a464bdfa8136e690f9f72d3d8a073c507e684e3","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                    \u0027volume\u0027: volume}"},{"line_number":195,"context_line":"        cctxt \u003d self._get_cctxt(volume.host, (\u00273.2\u0027, \u00273.0\u0027))"},{"line_number":196,"context_line":"        if not cctxt.can_send_version(\u00273.2\u0027):"},{"line_number":197,"context_line":"            msg_args.pop(\u0027volume\u0027)"},{"line_number":198,"context_line":"        return cctxt.call(ctxt, \u0027attach_volume\u0027, **msg_args)"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    def detach_volume(self, ctxt, volume, attachment_id):"}],"source_content_type":"text/x-python","patch_set":26,"id":"7a77a97e_3d55e1c5","line":197,"updated":"2016-11-22 07:51:02.000000000","message":"What I mean is if that target service is compatible with (\u003e3.2) RPC API, why still sends out volume_id?  It will not be used by target whatsoever.","commit_id":"85c9fefa29be379af14e1db744546c90121fc61b"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"da9f3f39873598d925a7384b20fe44786b96048b","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                    \u0027volume\u0027: volume}"},{"line_number":195,"context_line":"        cctxt \u003d self._get_cctxt(volume.host, (\u00273.2\u0027, \u00273.0\u0027))"},{"line_number":196,"context_line":"        if not cctxt.can_send_version(\u00273.2\u0027):"},{"line_number":197,"context_line":"            msg_args.pop(\u0027volume\u0027)"},{"line_number":198,"context_line":"        return cctxt.call(ctxt, \u0027attach_volume\u0027, **msg_args)"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    def detach_volume(self, ctxt, volume, attachment_id):"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a74a57a_696dff31","line":197,"in_reply_to":"5a74a57a_be5f5874","updated":"2016-11-23 10:08:05.000000000","message":"I\u0027m not considering the case when c-bak is importing VolumeManager directly (I thought it cannot happen since Mitaka?).\n\nc-api shouldn\u0027t really be left behind, we\u0027re assuming the upgrade order is: c-api, c-sch, c-vol, c-bak.\n\nI\u0027ve read your comment on volume/manager.py, and if I understand you correctly you\u0027re proposing to change VolumeManager.attach_volume signature to:\n\n def attach_volume(self, context, instance_uuid, host_name, mountpoint, mode, volume\u003dNone, volume_id\u003dNone):\n\nYes, this shouldn\u0027t break us, because we\u0027re sending every argument as keyword arg, but I don\u0027t really get the benefit of that change:\n\n* We\u0027ll need to modify all the places where we\u0027re calling VolumeManager.attach_volume internally.\n* We\u0027ll still be stuck with both parameters until major RPC version bump (we could ignore the meaning of major version and break the compatibility anyway, but I don\u0027t see it as a good idea).\n* Moreover, making both volume and volume_id optional would suggest you can call attach_volume without specifying any, which isn\u0027t really a good method design.\n\nI\u0027m not sure if this is what you are proposing?","commit_id":"85c9fefa29be379af14e1db744546c90121fc61b"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"72a90abe10a8f6de3a1e76a06d87d2d0fb78ecc1","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                    \u0027volume\u0027: volume}"},{"line_number":195,"context_line":"        cctxt \u003d self._get_cctxt(volume.host, (\u00273.2\u0027, \u00273.0\u0027))"},{"line_number":196,"context_line":"        if not cctxt.can_send_version(\u00273.2\u0027):"},{"line_number":197,"context_line":"            msg_args.pop(\u0027volume\u0027)"},{"line_number":198,"context_line":"        return cctxt.call(ctxt, \u0027attach_volume\u0027, **msg_args)"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    def detach_volume(self, ctxt, volume, attachment_id):"}],"source_content_type":"text/x-python","patch_set":26,"id":"7a77a97e_7464cb48","line":197,"in_reply_to":"7a77a97e_3d55e1c5","updated":"2016-11-22 11:08:44.000000000","message":"@Huang: As volume_id is a required argument for VolumeManager.attach_volume, we cannot really remove it that way. VolumeManager needs to keep it, as it needs to be compatible with older c-bak that can call it when creating or restoring backups.\n\nWe\u0027re cleaning these up in a little different way. See [1] for examples.\n\n[1] https://review.openstack.org/#/c/370710/","commit_id":"85c9fefa29be379af14e1db744546c90121fc61b"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"a9e46ab5f16a913ea5a5ebfdb495f71370a4d2b2","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                    \u0027volume\u0027: volume}"},{"line_number":195,"context_line":"        cctxt \u003d self._get_cctxt(volume.host, (\u00273.2\u0027, \u00273.0\u0027))"},{"line_number":196,"context_line":"        if not cctxt.can_send_version(\u00273.2\u0027):"},{"line_number":197,"context_line":"            msg_args.pop(\u0027volume\u0027)"},{"line_number":198,"context_line":"        return cctxt.call(ctxt, \u0027attach_volume\u0027, **msg_args)"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    def detach_volume(self, ctxt, volume, attachment_id):"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a74a57a_be5f5874","line":197,"in_reply_to":"7a77a97e_7464cb48","updated":"2016-11-23 03:36:49.000000000","message":"Hi Michal, I don\u0027t understand your statement regarding to c-bak depends on this interface.  If I understand correctly, older c-bak (before Lisa\u0027s change to de-couple c-bak and c-vol was merged) will call volumemanger via code (same code base), it will not be affect by this change since the c-vol being called will still be old code (same as c-bak); for the case when c-bak is running independently, it\u0027s calling attach_volume via RPC API, how is this different from c-api running old version code calling new c-vol?","commit_id":"85c9fefa29be379af14e1db744546c90121fc61b"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"40ff4966d1d3888d2c72f7ed19a7b3e4a7d65291","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                    \u0027volume\u0027: volume}"},{"line_number":195,"context_line":"        cctxt \u003d self._get_cctxt(volume.host, (\u00273.2\u0027, \u00273.0\u0027))"},{"line_number":196,"context_line":"        if not cctxt.can_send_version(\u00273.2\u0027):"},{"line_number":197,"context_line":"            msg_args.pop(\u0027volume\u0027)"},{"line_number":198,"context_line":"        return cctxt.call(ctxt, \u0027attach_volume\u0027, **msg_args)"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    def detach_volume(self, ctxt, volume, attachment_id):"}],"source_content_type":"text/x-python","patch_set":26,"id":"7a77a97e_95a3374f","line":197,"in_reply_to":"7a77a97e_b5c3d389","updated":"2016-11-22 01:33:50.000000000","message":"As it is good to keep volume_id required(please see my comments in cinder/volume/manage.py), we can\u0027t pop out volume_id.","commit_id":"85c9fefa29be379af14e1db744546c90121fc61b"}]}