)]}'
{"cinder/volume/manager.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5b57403c4b673a0bfbaf1acd9dd911131d8d98c9","unresolved":false,"context_lines":[{"line_number":1840,"context_line":"        # As after detach and refresh, volume_attchments will be None."},{"line_number":1841,"context_line":"        # We keep volume_attachment for later attach."},{"line_number":1842,"context_line":"        if orig_volume_status \u003d\u003d \u0027in-use\u0027:"},{"line_number":1843,"context_line":"            attachments \u003d volume.volume_attachment"},{"line_number":1844,"context_line":"            try:"},{"line_number":1845,"context_line":"                for attachment in attachments:"},{"line_number":1846,"context_line":"                    self.detach_volume(ctxt, volume.id, attachment[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a77a97e_fbe0b5fc","line":1843,"range":{"start_line":1843,"start_character":12,"end_line":1843,"end_character":50},"updated":"2016-11-16 10:24:37.000000000","message":"nit: We don\u0027t need a temporary variable, we can use `volume.volume_attachment` directly in the for loop.","commit_id":"a8417318fa3ead0fa481ded4f9785d1fdb455c7e"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5b57403c4b673a0bfbaf1acd9dd911131d8d98c9","unresolved":false,"context_lines":[{"line_number":1841,"context_line":"        # We keep volume_attachment for later attach."},{"line_number":1842,"context_line":"        if orig_volume_status \u003d\u003d \u0027in-use\u0027:"},{"line_number":1843,"context_line":"            attachments \u003d volume.volume_attachment"},{"line_number":1844,"context_line":"            try:"},{"line_number":1845,"context_line":"                for attachment in attachments:"},{"line_number":1846,"context_line":"                    self.detach_volume(ctxt, volume.id, attachment[\u0027id\u0027])"},{"line_number":1847,"context_line":"            except Exception as ex:"},{"line_number":1848,"context_line":"                LOG.error(_LE(\"Detach migration source volume failed:\""},{"line_number":1849,"context_line":"                          \"%(err)s\"),"},{"line_number":1850,"context_line":"                          {\u0027err\u0027: ex}, resource\u003dvolume)"},{"line_number":1851,"context_line":"        else:"},{"line_number":1852,"context_line":"            attachments \u003d None"},{"line_number":1853,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7a77a97e_3bcdcd5d","line":1850,"range":{"start_line":1844,"start_character":0,"end_line":1850,"end_character":55},"updated":"2016-11-16 10:24:37.000000000","message":"-1: This doesn\u0027t seem right, the try should be in the detach so we keep trying if there are more than one attach (I know that\u0027s not the case right now):\n\n for attachment in attachments:\n     try:\n         self.detach_volume(ctxt, volume.id, attachment[\u0027id\u0027])\n      except Exception as ex:\n          LOG.error(_LE(\"Detach migration source volume from instance %(instance)s failed:\"\n                          \"%(err)s\"),\n                          {\u0027instance\u0027: attachment.instance_uuid, \n                           \u0027err\u0027: ex}, resource\u003dvolume)","commit_id":"a8417318fa3ead0fa481ded4f9785d1fdb455c7e"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5b57403c4b673a0bfbaf1acd9dd911131d8d98c9","unresolved":false,"context_lines":[{"line_number":1848,"context_line":"                LOG.error(_LE(\"Detach migration source volume failed:\""},{"line_number":1849,"context_line":"                          \"%(err)s\"),"},{"line_number":1850,"context_line":"                          {\u0027err\u0027: ex}, resource\u003dvolume)"},{"line_number":1851,"context_line":"        else:"},{"line_number":1852,"context_line":"            attachments \u003d None"},{"line_number":1853,"context_line":""},{"line_number":1854,"context_line":"        # Give driver (new_volume) a chance to update things as needed"},{"line_number":1855,"context_line":"        # after a successful migration."}],"source_content_type":"text/x-python","patch_set":1,"id":"7a77a97e_9b2c214c","line":1852,"range":{"start_line":1851,"start_character":0,"end_line":1852,"end_character":30},"updated":"2016-11-16 10:24:37.000000000","message":"-1: This can be removed","commit_id":"a8417318fa3ead0fa481ded4f9785d1fdb455c7e"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5a580d01625dd2d1cb78d1a3c1521a1bb6e7805e","unresolved":false,"context_lines":[{"line_number":1845,"context_line":"                    self.detach_volume(ctxt, volume.id, attachment[\u0027id\u0027])"},{"line_number":1846,"context_line":"                except Exception as ex:"},{"line_number":1847,"context_line":"                    LOG.error(_LE(\"Detach migration source volume failed:\""},{"line_number":1848,"context_line":"                              \"%(err)s\"),"},{"line_number":1849,"context_line":"                              {\u0027err\u0027: ex}, resource\u003dvolume)"},{"line_number":1850,"context_line":""},{"line_number":1851,"context_line":"        # Give driver (new_volume) a chance to update things as needed"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a77a97e_d363dd18","line":1848,"updated":"2016-11-16 11:23:21.000000000","message":"nit: I would prefer to log the instance that failed, but it\u0027s ok.","commit_id":"677f9848027c96855744f410aacd3a6f41384e0b"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"30792f6f26505e15d1b98d39c78370d73e8a7f8a","unresolved":false,"context_lines":[{"line_number":1845,"context_line":"                    self.detach_volume(ctxt, volume.id, attachment[\u0027id\u0027])"},{"line_number":1846,"context_line":"                except Exception as ex:"},{"line_number":1847,"context_line":"                    LOG.error(_LE(\"Detach migration source volume failed:\""},{"line_number":1848,"context_line":"                              \"%(err)s\"),"},{"line_number":1849,"context_line":"                              {\u0027err\u0027: ex}, resource\u003dvolume)"},{"line_number":1850,"context_line":""},{"line_number":1851,"context_line":"        # Give driver (new_volume) a chance to update things as needed"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a77a97e_c57add87","line":1848,"in_reply_to":"7a77a97e_0de1829a","updated":"2016-11-17 18:30:13.000000000","message":"IMO, adding instance ID to the log is very important for operators in case of any errors. Also, it will have in a future once we introduce multi-attach","commit_id":"677f9848027c96855744f410aacd3a6f41384e0b"},{"author":{"_account_id":19153,"name":"jingtao liang","email":"liang.jingtao@zte.com.cn","username":"liangjingtao"},"change_message_id":"79afa255a15bf326673ed96c5f5a92c332c5656b","unresolved":false,"context_lines":[{"line_number":1845,"context_line":"                    self.detach_volume(ctxt, volume.id, attachment[\u0027id\u0027])"},{"line_number":1846,"context_line":"                except Exception as ex:"},{"line_number":1847,"context_line":"                    LOG.error(_LE(\"Detach migration source volume failed:\""},{"line_number":1848,"context_line":"                              \"%(err)s\"),"},{"line_number":1849,"context_line":"                              {\u0027err\u0027: ex}, resource\u003dvolume)"},{"line_number":1850,"context_line":""},{"line_number":1851,"context_line":"        # Give driver (new_volume) a chance to update things as needed"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a77a97e_0de1829a","line":1848,"in_reply_to":"7a77a97e_d363dd18","updated":"2016-11-17 06:15:32.000000000","message":"Done","commit_id":"677f9848027c96855744f410aacd3a6f41384e0b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c124b0b4c7195e85db713b8724adb3a5ed6cfc19","unresolved":false,"context_lines":[{"line_number":1842,"context_line":"        if orig_volume_status \u003d\u003d \u0027in-use\u0027:"},{"line_number":1843,"context_line":"            for attachment in volume.volume_attachment:"},{"line_number":1844,"context_line":"                try:"},{"line_number":1845,"context_line":"                    self.detach_volume(ctxt, volume.id, attachment[\u0027id\u0027])"},{"line_number":1846,"context_line":"                except Exception as ex:"},{"line_number":1847,"context_line":"                    LOG.error(_LE(\u0027\u0027\u0027Detach migration source volume"},{"line_number":1848,"context_line":"                                  %(volume.id)s from instance %(instance_id)s"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a77a97e_d3632b40","line":1845,"range":{"start_line":1845,"start_character":56,"end_line":1845,"end_character":72},"updated":"2016-11-21 11:08:37.000000000","message":"nit: attachment is an OVO instance, so we should use attachment.id, but since you didn\u0027t update this line I won\u0027t downvote for it.","commit_id":"938a7b4acf47adc9af6b5d0fa7033dc573a1dc8b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c124b0b4c7195e85db713b8724adb3a5ed6cfc19","unresolved":false,"context_lines":[{"line_number":1844,"context_line":"                try:"},{"line_number":1845,"context_line":"                    self.detach_volume(ctxt, volume.id, attachment[\u0027id\u0027])"},{"line_number":1846,"context_line":"                except Exception as ex:"},{"line_number":1847,"context_line":"                    LOG.error(_LE(\u0027\u0027\u0027Detach migration source volume"},{"line_number":1848,"context_line":"                                  %(volume.id)s from instance %(instance_id)s"},{"line_number":1849,"context_line":"                                  failed: %(err)s\u0027\u0027\u0027),"},{"line_number":1850,"context_line":"                              {\u0027err\u0027: ex,"},{"line_number":1851,"context_line":"                               \u0027volume.id\u0027: volume.id,"},{"line_number":1852,"context_line":"                               \u0027instance_id\u0027: attachment[\u0027id\u0027]},"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a77a97e_13b4c3cf","line":1849,"range":{"start_line":1847,"start_character":34,"end_line":1849,"end_character":52},"updated":"2016-11-21 11:08:37.000000000","message":"-1: Please enclose each line in \", as using the multi-line string will not yield the same results since it includes the newline and leading spaces.","commit_id":"938a7b4acf47adc9af6b5d0fa7033dc573a1dc8b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c124b0b4c7195e85db713b8724adb3a5ed6cfc19","unresolved":false,"context_lines":[{"line_number":1849,"context_line":"                                  failed: %(err)s\u0027\u0027\u0027),"},{"line_number":1850,"context_line":"                              {\u0027err\u0027: ex,"},{"line_number":1851,"context_line":"                               \u0027volume.id\u0027: volume.id,"},{"line_number":1852,"context_line":"                               \u0027instance_id\u0027: attachment[\u0027id\u0027]},"},{"line_number":1853,"context_line":"                              resource\u003dvolume)"},{"line_number":1854,"context_line":""},{"line_number":1855,"context_line":"        # Give driver (new_volume) a chance to update things as needed"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a77a97e_333e1f2a","line":1852,"range":{"start_line":1852,"start_character":46,"end_line":1852,"end_character":62},"updated":"2016-11-21 11:08:37.000000000","message":"-1: Since attachment is an OVO instance: attachment.id","commit_id":"938a7b4acf47adc9af6b5d0fa7033dc573a1dc8b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"2fe6a556a20cd1c19110ad858f255a3ae5e1a1bd","unresolved":false,"context_lines":[{"line_number":1846,"context_line":"                except Exception as ex:"},{"line_number":1847,"context_line":"                    LOG.error(_LE(\"Detach migration source volume\""},{"line_number":1848,"context_line":"                                  \"%(volume.id)s from instance %(instance_id)s\""},{"line_number":1849,"context_line":"                                  \"failed: %(err)s\"),"},{"line_number":1850,"context_line":"                              {\u0027err\u0027: ex,"},{"line_number":1851,"context_line":"                               \u0027volume.id\u0027: volume.id,"},{"line_number":1852,"context_line":"                               \u0027instance_id\u0027: attachment.id},"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a77a97e_6c36f4e8","line":1849,"updated":"2016-11-21 11:59:45.000000000","message":"-1: Please add empty space at the end of the string in L1847 `volume \"` and L11848 `%(instance_id)s \"` to make the proper word separation.","commit_id":"c347abfd08f39696ea65cbddaa110e285c52b108"}]}