)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"311a3e894d25479c6a348cf1febbf567638ae5e3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"b513cd85_d6c3bfd6","updated":"2022-05-13 08:54:24.000000000","message":"LGTM, Thanks Brian.","commit_id":"6386cbb0a0b6cbf05ce625c5b1b680f4259bde2e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"259565cc6490c2f9355b0f2975b0f17832e0b5e3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"9fe5ed59_0e5cee3b","updated":"2022-04-27 13:19:48.000000000","message":"Note about the pylint error (not introduced by this patch!) inline.","commit_id":"6386cbb0a0b6cbf05ce625c5b1b680f4259bde2e"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"9d00442bea0fdbd0d82c75e21b0700fdc6c03f1a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"77dc86eb_70d140c4","updated":"2022-05-12 14:07:18.000000000","message":"Thank you for working on this! ","commit_id":"6386cbb0a0b6cbf05ce625c5b1b680f4259bde2e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"0185a61e55342007a136c0dcf7d9d342f9ae6f63","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"0ad5a1b9_521e4270","updated":"2022-04-27 10:32:42.000000000","message":"recheck cinder-tempest-plugin-lvm-lio-barbican - cinder_tempest_plugin.api.volume.test_volume_backup.VolumesBackupsTest.test_incremental_backup : backup 21b92642-67bb-4b19-be98-6c1ad7ff4194 failed to build and is in ERROR status","commit_id":"6386cbb0a0b6cbf05ce625c5b1b680f4259bde2e"}],"cinder/backup/api.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"259565cc6490c2f9355b0f2975b0f17832e0b5e3","unresolved":true,"context_lines":[{"line_number":353,"context_line":"        except Exception:"},{"line_number":354,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":355,"context_line":"                try:"},{"line_number":356,"context_line":"                    if backup and \u0027id\u0027 in backup:"},{"line_number":357,"context_line":"                        backup.destroy()"},{"line_number":358,"context_line":"                finally:"},{"line_number":359,"context_line":"                    QUOTAS.rollback(context, reservations)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ab019424_30ef550c","line":356,"range":{"start_line":356,"start_character":34,"end_line":356,"end_character":48},"updated":"2022-04-27 13:19:48.000000000","message":"pylint doesn\u0027t like this check:\n\ncinder/backup/api.py:356:42: E1135: Value \u0027backup\u0027 doesn\u0027t support membership test (unsupported-membership-test)\n\nbut removing the check, or using backup.id, or hasattr(backup, \u0027id\u0027) all break one of our unit tests:\n\ncinder.tests.unit.backup.test_backup.BackupAPITestCase.test_create_when_failed_to_create_backup_object\n\nby raising the exception:\n\noslo_versionedobjects.exception.ObjectActionError: Object action obj_load_attr failed because: attribute id not lazy-loadable","commit_id":"6386cbb0a0b6cbf05ce625c5b1b680f4259bde2e"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"9d00442bea0fdbd0d82c75e21b0700fdc6c03f1a","unresolved":true,"context_lines":[{"line_number":353,"context_line":"        except Exception:"},{"line_number":354,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":355,"context_line":"                try:"},{"line_number":356,"context_line":"                    if backup and \u0027id\u0027 in backup:"},{"line_number":357,"context_line":"                        backup.destroy()"},{"line_number":358,"context_line":"                finally:"},{"line_number":359,"context_line":"                    QUOTAS.rollback(context, reservations)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9d793919_3d5c3498","line":356,"range":{"start_line":356,"start_character":34,"end_line":356,"end_character":48},"in_reply_to":"765ea424_9b033cf3","updated":"2022-05-12 14:07:18.000000000","message":"Cool, i\u0027ll review later.","commit_id":"6386cbb0a0b6cbf05ce625c5b1b680f4259bde2e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"cd40614e0ac8e3a8538aaf74f06889a1131b4f50","unresolved":false,"context_lines":[{"line_number":353,"context_line":"        except Exception:"},{"line_number":354,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":355,"context_line":"                try:"},{"line_number":356,"context_line":"                    if backup and \u0027id\u0027 in backup:"},{"line_number":357,"context_line":"                        backup.destroy()"},{"line_number":358,"context_line":"                finally:"},{"line_number":359,"context_line":"                    QUOTAS.rollback(context, reservations)"}],"source_content_type":"text/x-python","patch_set":3,"id":"55788abf_894e3dea","line":356,"range":{"start_line":356,"start_character":34,"end_line":356,"end_character":48},"in_reply_to":"9d793919_3d5c3498","updated":"2022-05-13 12:38:18.000000000","message":"Ack","commit_id":"6386cbb0a0b6cbf05ce625c5b1b680f4259bde2e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"adac3dd3b2cfb89fd5188a6bcd58c2705457f4d4","unresolved":true,"context_lines":[{"line_number":353,"context_line":"        except Exception:"},{"line_number":354,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":355,"context_line":"                try:"},{"line_number":356,"context_line":"                    if backup and \u0027id\u0027 in backup:"},{"line_number":357,"context_line":"                        backup.destroy()"},{"line_number":358,"context_line":"                finally:"},{"line_number":359,"context_line":"                    QUOTAS.rollback(context, reservations)"}],"source_content_type":"text/x-python","patch_set":3,"id":"765ea424_9b033cf3","line":356,"range":{"start_line":356,"start_character":34,"end_line":356,"end_character":48},"in_reply_to":"ab019424_30ef550c","updated":"2022-04-27 19:31:25.000000000","message":"I try to address this in a followup patch: https://review.opendev.org/c/openstack/cinder/+/839628/","commit_id":"6386cbb0a0b6cbf05ce625c5b1b680f4259bde2e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"311a3e894d25479c6a348cf1febbf567638ae5e3","unresolved":true,"context_lines":[{"line_number":582,"context_line":"                backup.save()"},{"line_number":583,"context_line":"            else:"},{"line_number":584,"context_line":"                # create a new backup with the specified ID"},{"line_number":585,"context_line":"                backup \u003d objects.BackupImport(context\u003dcontext,"},{"line_number":586,"context_line":"                                              id\u003dbackup_record[\u0027id\u0027], **kwargs)"},{"line_number":587,"context_line":"                backup.create()"},{"line_number":588,"context_line":"            QUOTAS.commit(context, reservations)"}],"source_content_type":"text/x-python","patch_set":3,"id":"33f42188_6e9ae9fe","line":585,"range":{"start_line":585,"start_character":16,"end_line":585,"end_character":22},"updated":"2022-05-13 08:54:24.000000000","message":"the problem i see is we use the same variable name for existing/deleted/new backup.\nThe original code should\u0027ve handled it if we used existing_backup and new_backup and just check\n\n    if new_backup:\n        new_backup.destroy()\n\n\nbut i like the new code to avoid the roll back (quota) process altogether for existing backups.","commit_id":"6386cbb0a0b6cbf05ce625c5b1b680f4259bde2e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"cd40614e0ac8e3a8538aaf74f06889a1131b4f50","unresolved":true,"context_lines":[{"line_number":582,"context_line":"                backup.save()"},{"line_number":583,"context_line":"            else:"},{"line_number":584,"context_line":"                # create a new backup with the specified ID"},{"line_number":585,"context_line":"                backup \u003d objects.BackupImport(context\u003dcontext,"},{"line_number":586,"context_line":"                                              id\u003dbackup_record[\u0027id\u0027], **kwargs)"},{"line_number":587,"context_line":"                backup.create()"},{"line_number":588,"context_line":"            QUOTAS.commit(context, reservations)"}],"source_content_type":"text/x-python","patch_set":3,"id":"b1cfbd7e_80382701","line":585,"range":{"start_line":585,"start_character":16,"end_line":585,"end_character":22},"in_reply_to":"33f42188_6e9ae9fe","updated":"2022-05-13 12:38:18.000000000","message":"Agree with you on both points.","commit_id":"6386cbb0a0b6cbf05ce625c5b1b680f4259bde2e"}],"cinder/tests/unit/api/contrib/test_backups.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"311a3e894d25479c6a348cf1febbf567638ae5e3","unresolved":true,"context_lines":[{"line_number":2243,"context_line":""},{"line_number":2244,"context_line":"        # Bug #1965847: already existing backup should not be deleted"},{"line_number":2245,"context_line":"        self.assertNotEqual(fields.BackupStatus.DELETED,"},{"line_number":2246,"context_line":"                            self._get_backup_attrib(backup.id, \u0027status\u0027))"},{"line_number":2247,"context_line":"        mock_reserve.assert_not_called()"},{"line_number":2248,"context_line":"        mock_rollback.assert_not_called()"},{"line_number":2249,"context_line":"        mock_commit.assert_not_called()"}],"source_content_type":"text/x-python","patch_set":3,"id":"cb9cf419_a3741999","line":2246,"range":{"start_line":2246,"start_character":33,"end_line":2246,"end_character":51},"updated":"2022-05-13 08:54:24.000000000","message":"looks like this wasn\u0027t use before and this is the only place using this method\nhttps://github.com/openstack/cinder/search?q\u003d_get_backup_attrib\n\nnit: suggestion is to directly use the db method and remove the function\n    backup_status \u003d db.backup_get(context.get_admin_context(),\n                                  backup_id)[attrib_name]\n    self.assertNotEqual(fields.BackupStatus.DELETED, backup_status)","commit_id":"6386cbb0a0b6cbf05ce625c5b1b680f4259bde2e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"cd40614e0ac8e3a8538aaf74f06889a1131b4f50","unresolved":true,"context_lines":[{"line_number":2243,"context_line":""},{"line_number":2244,"context_line":"        # Bug #1965847: already existing backup should not be deleted"},{"line_number":2245,"context_line":"        self.assertNotEqual(fields.BackupStatus.DELETED,"},{"line_number":2246,"context_line":"                            self._get_backup_attrib(backup.id, \u0027status\u0027))"},{"line_number":2247,"context_line":"        mock_reserve.assert_not_called()"},{"line_number":2248,"context_line":"        mock_rollback.assert_not_called()"},{"line_number":2249,"context_line":"        mock_commit.assert_not_called()"}],"source_content_type":"text/x-python","patch_set":3,"id":"ceb5fabe_17a9b9a5","line":2246,"range":{"start_line":2246,"start_character":33,"end_line":2246,"end_character":51},"in_reply_to":"cb9cf419_a3741999","updated":"2022-05-13 12:38:18.000000000","message":"Yeah, I saw the function defined at the top of the file, and it did what I needed, so I just used it.  Didn\u0027t think to check if it was used anywhere else in the file!\n\nI may do a followup, because the function hides that we\u0027re getting fresh data from the DB, which is kind of essential to what we\u0027re testing here.","commit_id":"6386cbb0a0b6cbf05ce625c5b1b680f4259bde2e"}]}