)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f6bb7db545d1a991cc8f2d7959835a5ef51d77f9","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Speed up starting cinder-backup"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When start/restart cinder-backup, we will get all backups form cinder"},{"line_number":10,"context_line":"DB and then check the status in the for loop. It will take a long time"},{"line_number":11,"context_line":"when the backups in a huge amount. Now we just get all incomplete"},{"line_number":12,"context_line":"backups what we really need to do the job with."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7faddb67_0761bfa8","line":9,"range":{"start_line":9,"start_character":58,"end_line":9,"end_character":62},"updated":"2019-08-29 13:43:03.000000000","message":"nit: from","commit_id":"69cdf18318f69422a6c1cbef30d7abaf95ebf737"},{"author":{"_account_id":25837,"name":"yenai","email":"yenai_yewu@cmss.chinamobile.com","username":"yenai2008"},"change_message_id":"0eb73bc6c7aad03579b5d2d2c55186e5a56728b1","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Speed up starting cinder-backup"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When start/restart cinder-backup, we will get all backups form cinder"},{"line_number":10,"context_line":"DB and then check the status in the for loop. It will take a long time"},{"line_number":11,"context_line":"when the backups in a huge amount. Now we just get all incomplete"},{"line_number":12,"context_line":"backups what we really need to do the job with."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"5faad753_00a69c11","line":9,"range":{"start_line":9,"start_character":58,"end_line":9,"end_character":62},"in_reply_to":"7faddb67_0761bfa8","updated":"2019-09-11 10:52:24.000000000","message":"Done","commit_id":"69cdf18318f69422a6c1cbef30d7abaf95ebf737"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"2e193f0d0b8395da821d3b621fd01f37dbfb890f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":11,"id":"49229895_e89d07e7","updated":"2023-06-21 09:49:39.000000000","message":"This approach to only select the backup status we care about looks really sensible and simple to speed things up.\n\nCan we revive this change maybe?","commit_id":"302095741a9ad204228112a895f52cf6cc1b96ae"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6fbc05aa95b50270e658c52ac497436e055d029b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"e65cbdf1_8e3b1257","updated":"2023-11-17 17:02:59.000000000","message":"Mostly looks good, but a few questions inline.","commit_id":"130304b6d77ae6113d805856ed27035d8dbd1791"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"4768b251ed1622c3dcc82086738458838dc4cfa0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"51612cf0_49892cc1","updated":"2023-11-17 14:06:50.000000000","message":"recheck cinder-tempest-plugin-lvm-lio-barbican - tempest.api.compute.servers.test_device_tagging.TaggedBootDevicesTest.test_tagged_boot_devices - authentication timeout trying to connect to the instance","commit_id":"130304b6d77ae6113d805856ed27035d8dbd1791"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"b402a48c865a872b2d8a11df0cfd50f4768cde0c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"21343ac0_d4594b32","updated":"2024-01-23 10:46:57.000000000","message":"I greatly appreciate the improvement here! May I kindly also point to https://bugs.launchpad.net/cinder/+bug/2048396 to make sure any filtering DB query really fast.","commit_id":"11fd227bf441f61617deca2535f0373bc311ac1e"},{"author":{"_account_id":31779,"name":"Jean Pierre Roquesalane","display_name":"happystacker","email":"jeanpierre.roquesalane@dell.com","username":"happystacker"},"change_message_id":"d556d88d10cf5269c55811e8f50c2d8e6b4522be","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"fe741ee5_c4e90287","updated":"2024-01-22 12:55:23.000000000","message":"Nice perf improvement! Thanks for this.\nThe code looks ok to me, but I\u0027m still a bit puzzled by the tests.","commit_id":"11fd227bf441f61617deca2535f0373bc311ac1e"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"9b677358cdce8dbd752a1f7fa6f97bf9902b7094","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"4fc961cf_9ea1accb","updated":"2024-02-06 15:57:44.000000000","message":"Hold on just a second, I talked to Gorka about the right way to do this.","commit_id":"280d14e6d97298b328844e1cf61bc58d6ab9a6ef"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"83c3ae57fabd6034dd341d89c5ffd39d3dc35583","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"45582ecf_405823bb","updated":"2024-02-15 15:03:54.000000000","message":"Code is ok, I just don\u0027t love how we build the `incomplete_status` variable.\nWon\u0027t hold patch anymore.","commit_id":"52d35f93aa3acb26e86f3467840354ad99ea1711"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9a5dde51f73dddd83e33213953ec45e42b2be09b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"653b75b8_961e9a4c","updated":"2024-04-12 15:06:57.000000000","message":"A release note would be great to list this as an improvement.","commit_id":"b52eb917b4150e68e1a3d83b2374d017e7addaa9"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"71aa3bf6acf40ca217fb75d347bbb5fc942045a0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"0be2d52e_75d467de","updated":"2024-04-19 22:03:44.000000000","message":"I\u0027m going to push the same change but with the release note added. Doing so is going to clear Gorka\u0027s -1. Both comments (about the dup code in tests, and the release note) should be addressed.","commit_id":"b52eb917b4150e68e1a3d83b2374d017e7addaa9"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"dd703c10c42d89d3cc6459c51e08f19ef5ce14de","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"1fc00f64_365134b3","updated":"2025-07-31 15:59:29.000000000","message":"Code looks good to me, this will be a nice performance improvement to have.","commit_id":"7bd72aca32ff3f52c92e7f107921a6a33d64abf1"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"ce43b00e637ce5a7437f5db89f45294b2e514cc9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"1483e30d_3f2d06fb","updated":"2025-05-21 09:24:57.000000000","message":"Could this potentially be merged?","commit_id":"7bd72aca32ff3f52c92e7f107921a6a33d64abf1"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@purestorage.com","username":"sdodsley"},"change_message_id":"7a9372bcd44662142f3399692d6ab763f9065816","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"2cbe3404_b246bf13","updated":"2025-07-16 18:11:01.000000000","message":"Looks good","commit_id":"7bd72aca32ff3f52c92e7f107921a6a33d64abf1"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"3a038910f47000828975ec536c085a6acab771d9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"56b35889_ac46533e","updated":"2025-04-04 07:09:42.000000000","message":"Since this has received a +2 from Gorka already ... could anybody kindly take a another peer and maybe even merge this?","commit_id":"7bd72aca32ff3f52c92e7f107921a6a33d64abf1"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"80889423aa15c6cd29be387bf0f31e9f9b4d5599","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"004f11e6_174c100c","updated":"2025-07-15 16:54:13.000000000","message":"recheck","commit_id":"7bd72aca32ff3f52c92e7f107921a6a33d64abf1"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"b7852384f2cf6462570e21cd7181ded27ae27bc2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"90fe135b_27119c38","updated":"2025-07-24 02:20:26.000000000","message":"recheck","commit_id":"7bd72aca32ff3f52c92e7f107921a6a33d64abf1"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"9cafe16eab5558913836655c8b07f8b953c23374","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"c5c098d9_89c069b4","updated":"2025-07-24 01:49:16.000000000","message":"recheck ceph tempest encrypted volume extend failed","commit_id":"7bd72aca32ff3f52c92e7f107921a6a33d64abf1"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"cdcc79e3058e72064b839e3f30627a84f63a0408","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"a99f7b23_ed39bfd4","updated":"2025-07-24 04:26:38.000000000","message":"recheck failure in tempest-slow-py3, unrelated","commit_id":"7bd72aca32ff3f52c92e7f107921a6a33d64abf1"}],"cinder/backup/manager.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"89034fc2a69544aad08747257aec45c0c4ffd9a3","unresolved":false,"context_lines":[{"line_number":187,"context_line":"        # TODO(smulcahy) implement full resume of backup and restore"},{"line_number":188,"context_line":"        # operations on restart (rather than simply resetting)."},{"line_number":189,"context_line":"        # Note(yenai): We only need to deal with the backups that in"},{"line_number":190,"context_line":"        # the status: creating, restoring and deleting."},{"line_number":191,"context_line":"        incomplete_status \u003d [fields.BackupStatus.CREATING,"},{"line_number":192,"context_line":"                             fields.BackupStatus.RESTORING,"},{"line_number":193,"context_line":"                             fields.BackupStatus.DELETING]"}],"source_content_type":"text/x-python","patch_set":2,"id":"bfb3d3c7_f95ec310","line":190,"range":{"start_line":190,"start_character":10,"end_line":190,"end_character":55},"updated":"2019-05-21 15:17:53.000000000","message":"This is just restating what the code already says: would be better to just note that we only need backups that aren\u0027t complete.","commit_id":"5ae43587be0d5e9c28cd05ed3f4f2e1d863655f7"},{"author":{"_account_id":25837,"name":"yenai","email":"yenai_yewu@cmss.chinamobile.com","username":"yenai2008"},"change_message_id":"c5529cb492da8b0e0fb11dff7b1a926aa04107d9","unresolved":false,"context_lines":[{"line_number":187,"context_line":"        # TODO(smulcahy) implement full resume of backup and restore"},{"line_number":188,"context_line":"        # operations on restart (rather than simply resetting)."},{"line_number":189,"context_line":"        # Note(yenai): We only need to deal with the backups that in"},{"line_number":190,"context_line":"        # the status: creating, restoring and deleting."},{"line_number":191,"context_line":"        incomplete_status \u003d [fields.BackupStatus.CREATING,"},{"line_number":192,"context_line":"                             fields.BackupStatus.RESTORING,"},{"line_number":193,"context_line":"                             fields.BackupStatus.DELETING]"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_9bb478f4","line":190,"range":{"start_line":190,"start_character":10,"end_line":190,"end_character":55},"in_reply_to":"bfb3d3c7_f95ec310","updated":"2019-07-24 08:56:48.000000000","message":"Done","commit_id":"5ae43587be0d5e9c28cd05ed3f4f2e1d863655f7"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f6bb7db545d1a991cc8f2d7959835a5ef51d77f9","unresolved":false,"context_lines":[{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        # TODO(smulcahy) implement full resume of backup and restore"},{"line_number":200,"context_line":"        # operations on restart (rather than simply resetting)."},{"line_number":201,"context_line":"        # Note(yenai): We only need to deal with the backups that"},{"line_number":202,"context_line":"        # aren\u0027t complete."},{"line_number":203,"context_line":"        incomplete_status \u003d [fields.BackupStatus.CREATING,"},{"line_number":204,"context_line":"                             fields.BackupStatus.RESTORING,"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_87904f98","line":201,"range":{"start_line":201,"start_character":10,"end_line":201,"end_character":23},"updated":"2019-08-29 13:43:03.000000000","message":"-1: The Note part is not necessary","commit_id":"69cdf18318f69422a6c1cbef30d7abaf95ebf737"},{"author":{"_account_id":25837,"name":"yenai","email":"yenai_yewu@cmss.chinamobile.com","username":"yenai2008"},"change_message_id":"0eb73bc6c7aad03579b5d2d2c55186e5a56728b1","unresolved":false,"context_lines":[{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        # TODO(smulcahy) implement full resume of backup and restore"},{"line_number":200,"context_line":"        # operations on restart (rather than simply resetting)."},{"line_number":201,"context_line":"        # Note(yenai): We only need to deal with the backups that"},{"line_number":202,"context_line":"        # aren\u0027t complete."},{"line_number":203,"context_line":"        incomplete_status \u003d [fields.BackupStatus.CREATING,"},{"line_number":204,"context_line":"                             fields.BackupStatus.RESTORING,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5faad753_e0aa60d0","line":201,"range":{"start_line":201,"start_character":10,"end_line":201,"end_character":23},"in_reply_to":"7faddb67_87904f98","updated":"2019-09-11 10:52:24.000000000","message":"Done","commit_id":"69cdf18318f69422a6c1cbef30d7abaf95ebf737"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f6bb7db545d1a991cc8f2d7959835a5ef51d77f9","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        # operations on restart (rather than simply resetting)."},{"line_number":201,"context_line":"        # Note(yenai): We only need to deal with the backups that"},{"line_number":202,"context_line":"        # aren\u0027t complete."},{"line_number":203,"context_line":"        incomplete_status \u003d [fields.BackupStatus.CREATING,"},{"line_number":204,"context_line":"                             fields.BackupStatus.RESTORING,"},{"line_number":205,"context_line":"                             fields.BackupStatus.DELETING]"},{"line_number":206,"context_line":"        backups \u003d objects.BackupList.get_all("},{"line_number":207,"context_line":"            ctxt, {\u0027host\u0027: self.host, \u0027status\u0027: incomplete_status})"},{"line_number":208,"context_line":"        for backup in backups:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_87436f11","line":205,"range":{"start_line":203,"start_character":0,"end_line":205,"end_character":58},"updated":"2019-08-29 13:43:03.000000000","message":"nit: Better use a tuple than a list","commit_id":"69cdf18318f69422a6c1cbef30d7abaf95ebf737"},{"author":{"_account_id":25837,"name":"yenai","email":"yenai_yewu@cmss.chinamobile.com","username":"yenai2008"},"change_message_id":"0eb73bc6c7aad03579b5d2d2c55186e5a56728b1","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        # operations on restart (rather than simply resetting)."},{"line_number":201,"context_line":"        # Note(yenai): We only need to deal with the backups that"},{"line_number":202,"context_line":"        # aren\u0027t complete."},{"line_number":203,"context_line":"        incomplete_status \u003d [fields.BackupStatus.CREATING,"},{"line_number":204,"context_line":"                             fields.BackupStatus.RESTORING,"},{"line_number":205,"context_line":"                             fields.BackupStatus.DELETING]"},{"line_number":206,"context_line":"        backups \u003d objects.BackupList.get_all("},{"line_number":207,"context_line":"            ctxt, {\u0027host\u0027: self.host, \u0027status\u0027: incomplete_status})"},{"line_number":208,"context_line":"        for backup in backups:"}],"source_content_type":"text/x-python","patch_set":4,"id":"5faad753_80b92c2f","line":205,"range":{"start_line":203,"start_character":0,"end_line":205,"end_character":58},"in_reply_to":"7faddb67_87436f11","updated":"2019-09-11 10:52:24.000000000","message":"Done","commit_id":"69cdf18318f69422a6c1cbef30d7abaf95ebf737"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f6bb7db545d1a991cc8f2d7959835a5ef51d77f9","unresolved":false,"context_lines":[{"line_number":204,"context_line":"                             fields.BackupStatus.RESTORING,"},{"line_number":205,"context_line":"                             fields.BackupStatus.DELETING]"},{"line_number":206,"context_line":"        backups \u003d objects.BackupList.get_all("},{"line_number":207,"context_line":"            ctxt, {\u0027host\u0027: self.host, \u0027status\u0027: incomplete_status})"},{"line_number":208,"context_line":"        for backup in backups:"},{"line_number":209,"context_line":"            try:"},{"line_number":210,"context_line":"                self._cleanup_one_backup(ctxt, backup)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_47b81710","line":207,"updated":"2019-08-29 13:43:03.000000000","message":"nit: I prefer it if you specify that the data is passed as the filter parameter.\n\n            ctxt, filters\u003d{\u0027host\u0027: self.host, \u0027status\u0027: incomplete_status})","commit_id":"69cdf18318f69422a6c1cbef30d7abaf95ebf737"},{"author":{"_account_id":25837,"name":"yenai","email":"yenai_yewu@cmss.chinamobile.com","username":"yenai2008"},"change_message_id":"0eb73bc6c7aad03579b5d2d2c55186e5a56728b1","unresolved":false,"context_lines":[{"line_number":204,"context_line":"                             fields.BackupStatus.RESTORING,"},{"line_number":205,"context_line":"                             fields.BackupStatus.DELETING]"},{"line_number":206,"context_line":"        backups \u003d objects.BackupList.get_all("},{"line_number":207,"context_line":"            ctxt, {\u0027host\u0027: self.host, \u0027status\u0027: incomplete_status})"},{"line_number":208,"context_line":"        for backup in backups:"},{"line_number":209,"context_line":"            try:"},{"line_number":210,"context_line":"                self._cleanup_one_backup(ctxt, backup)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5faad753_a0b46836","line":207,"in_reply_to":"7faddb67_47b81710","updated":"2019-09-11 10:52:24.000000000","message":"Done","commit_id":"69cdf18318f69422a6c1cbef30d7abaf95ebf737"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"7f018225dd3f965b5c0044cbf26e94fa378a4f02","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        incomplete_status \u003d (fields.BackupStatus.CREATING,"},{"line_number":203,"context_line":"                             fields.BackupStatus.RESTORING,"},{"line_number":204,"context_line":"                             fields.BackupStatus.DELETING,"},{"line_number":205,"context_line":"                             fields.BackupStatus.ERROR)"},{"line_number":206,"context_line":"        backups \u003d objects.BackupList.get_all("},{"line_number":207,"context_line":"            ctxt, filters\u003d{\u0027host\u0027: self.host, \u0027status\u0027: incomplete_status})"},{"line_number":208,"context_line":"        for backup in backups:"}],"source_content_type":"text/x-python","patch_set":9,"id":"5faad753_9322e351","line":205,"range":{"start_line":205,"start_character":29,"end_line":205,"end_character":54},"updated":"2019-09-11 14:45:17.000000000","message":"we query backups in error state but do nothing with them","commit_id":"7c4f8d64a37ca764bd5e24c41efa504ff4d9d644"},{"author":{"_account_id":25837,"name":"yenai","email":"yenai_yewu@cmss.chinamobile.com","username":"yenai2008"},"change_message_id":"76cdb449dd2066166d12d82ec101e27e20084a17","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        incomplete_status \u003d (fields.BackupStatus.CREATING,"},{"line_number":203,"context_line":"                             fields.BackupStatus.RESTORING,"},{"line_number":204,"context_line":"                             fields.BackupStatus.DELETING,"},{"line_number":205,"context_line":"                             fields.BackupStatus.ERROR)"},{"line_number":206,"context_line":"        backups \u003d objects.BackupList.get_all("},{"line_number":207,"context_line":"            ctxt, filters\u003d{\u0027host\u0027: self.host, \u0027status\u0027: incomplete_status})"},{"line_number":208,"context_line":"        for backup in backups:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fa7e38b_008b1ec0","line":205,"range":{"start_line":205,"start_character":29,"end_line":205,"end_character":54},"in_reply_to":"5faad753_9322e351","updated":"2019-11-12 10:03:47.000000000","message":"No.\nYou can just see the method `_cleanup_temp_volumes_snapshots_for_one_backup`.","commit_id":"7c4f8d64a37ca764bd5e24c41efa504ff4d9d644"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"a1d229f36e7e62fe179c5ca4e4f4a7a7e03a3178","unresolved":false,"context_lines":[{"line_number":235,"context_line":"            self.db.volume_update(ctxt, volume[\u0027id\u0027],"},{"line_number":236,"context_line":"                                  {\u0027status\u0027: \u0027error_restoring\u0027})"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    def _cleanup_one_backup(self, ctxt, backup):"},{"line_number":239,"context_line":"        if backup[\u0027status\u0027] \u003d\u003d fields.BackupStatus.CREATING:"},{"line_number":240,"context_line":"            LOG.info(\u0027Resetting backup %s to error (was creating).\u0027,"},{"line_number":241,"context_line":"                     backup[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"5faad753_5008e12a","line":238,"updated":"2019-09-11 14:34:04.000000000","message":"Do we need to handle fields.BackupStatus.ERROR status here?","commit_id":"7c4f8d64a37ca764bd5e24c41efa504ff4d9d644"},{"author":{"_account_id":25837,"name":"yenai","email":"yenai_yewu@cmss.chinamobile.com","username":"yenai2008"},"change_message_id":"76cdb449dd2066166d12d82ec101e27e20084a17","unresolved":false,"context_lines":[{"line_number":235,"context_line":"            self.db.volume_update(ctxt, volume[\u0027id\u0027],"},{"line_number":236,"context_line":"                                  {\u0027status\u0027: \u0027error_restoring\u0027})"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    def _cleanup_one_backup(self, ctxt, backup):"},{"line_number":239,"context_line":"        if backup[\u0027status\u0027] \u003d\u003d fields.BackupStatus.CREATING:"},{"line_number":240,"context_line":"            LOG.info(\u0027Resetting backup %s to error (was creating).\u0027,"},{"line_number":241,"context_line":"                     backup[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fa7e38b_80b86e75","line":238,"in_reply_to":"5faad753_5008e12a","updated":"2019-11-12 10:03:47.000000000","message":"Not here. See the method `_cleanup_temp_volumes_snapshots_for_one_backup`.","commit_id":"7c4f8d64a37ca764bd5e24c41efa504ff4d9d644"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"25c2f183611a475c532d0e27b7dd45f28c06dbbd","unresolved":true,"context_lines":[{"line_number":198,"context_line":"        # TODO(smulcahy) implement full resume of backup and restore"},{"line_number":199,"context_line":"        # operations on restart (rather than simply resetting)."},{"line_number":200,"context_line":"        # We only need to deal with the backups that aren\u0027t complete."},{"line_number":201,"context_line":"        incomplete_status \u003d (fields.BackupStatus.CREATING,"},{"line_number":202,"context_line":"                             fields.BackupStatus.RESTORING,"},{"line_number":203,"context_line":"                             fields.BackupStatus.DELETING,"},{"line_number":204,"context_line":"                             fields.BackupStatus.ERROR)"}],"source_content_type":"text/x-python","patch_set":11,"id":"77c32fdb_61725d9a","line":201,"updated":"2023-06-21 09:54:39.000000000","message":"Maybe filtering out everything that is \"not in (completed, error)\" makes this easier and less prone to issues where there are more states added?","commit_id":"302095741a9ad204228112a895f52cf6cc1b96ae"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"d606a0a6d36178746e8645e3a6dee41ec19f9313","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        # TODO(smulcahy) implement full resume of backup and restore"},{"line_number":199,"context_line":"        # operations on restart (rather than simply resetting)."},{"line_number":200,"context_line":"        # We only need to deal with the backups that aren\u0027t complete."},{"line_number":201,"context_line":"        incomplete_status \u003d (fields.BackupStatus.CREATING,"},{"line_number":202,"context_line":"                             fields.BackupStatus.RESTORING,"},{"line_number":203,"context_line":"                             fields.BackupStatus.DELETING,"},{"line_number":204,"context_line":"                             fields.BackupStatus.ERROR)"}],"source_content_type":"text/x-python","patch_set":11,"id":"0868df93_a71ca2a7","line":201,"in_reply_to":"193c997d_87a41b51","updated":"2023-11-01 15:57:40.000000000","message":"I\u0027m going to go with the safe option and only exclude the completed status.","commit_id":"302095741a9ad204228112a895f52cf6cc1b96ae"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"2cefa4f125f84311fbca2ebc0b80bbfe8148a0b3","unresolved":true,"context_lines":[{"line_number":198,"context_line":"        # TODO(smulcahy) implement full resume of backup and restore"},{"line_number":199,"context_line":"        # operations on restart (rather than simply resetting)."},{"line_number":200,"context_line":"        # We only need to deal with the backups that aren\u0027t complete."},{"line_number":201,"context_line":"        incomplete_status \u003d (fields.BackupStatus.CREATING,"},{"line_number":202,"context_line":"                             fields.BackupStatus.RESTORING,"},{"line_number":203,"context_line":"                             fields.BackupStatus.DELETING,"},{"line_number":204,"context_line":"                             fields.BackupStatus.ERROR)"}],"source_content_type":"text/x-python","patch_set":11,"id":"35e4c4a1_ad816d4b","line":201,"in_reply_to":"3443fe9e_7a90fb78","updated":"2023-10-28 03:04:31.000000000","message":"So, does ERROR need a cleanup or does not need a cleanup?","commit_id":"302095741a9ad204228112a895f52cf6cc1b96ae"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"d73c99c7b6cc655f7929aef696e5ce39db666ddb","unresolved":true,"context_lines":[{"line_number":198,"context_line":"        # TODO(smulcahy) implement full resume of backup and restore"},{"line_number":199,"context_line":"        # operations on restart (rather than simply resetting)."},{"line_number":200,"context_line":"        # We only need to deal with the backups that aren\u0027t complete."},{"line_number":201,"context_line":"        incomplete_status \u003d (fields.BackupStatus.CREATING,"},{"line_number":202,"context_line":"                             fields.BackupStatus.RESTORING,"},{"line_number":203,"context_line":"                             fields.BackupStatus.DELETING,"},{"line_number":204,"context_line":"                             fields.BackupStatus.ERROR)"}],"source_content_type":"text/x-python","patch_set":11,"id":"193c997d_87a41b51","line":201,"in_reply_to":"35e4c4a1_ad816d4b","updated":"2023-10-31 09:07:47.000000000","message":"\u003e So, does ERROR need a cleanup or does not need a cleanup?\nCould you rephrase and explain your question a little more please?\n\n1) I suggested using a negative filter as in\n  \"not in (completed, error)\"\n  \nto now having to extend this list with any new status.\n  \n2) You now argue that \"error\" should not be in that list even? So only \"completed\" should be returned?\n\n\n\nApart from the particular remarks .. would you rebase and push the required changes Pete to get this change done with and merged?","commit_id":"302095741a9ad204228112a895f52cf6cc1b96ae"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"b3c4cc635d1f96741e3df54f93a752574bb81739","unresolved":true,"context_lines":[{"line_number":198,"context_line":"        # TODO(smulcahy) implement full resume of backup and restore"},{"line_number":199,"context_line":"        # operations on restart (rather than simply resetting)."},{"line_number":200,"context_line":"        # We only need to deal with the backups that aren\u0027t complete."},{"line_number":201,"context_line":"        incomplete_status \u003d (fields.BackupStatus.CREATING,"},{"line_number":202,"context_line":"                             fields.BackupStatus.RESTORING,"},{"line_number":203,"context_line":"                             fields.BackupStatus.DELETING,"},{"line_number":204,"context_line":"                             fields.BackupStatus.ERROR)"}],"source_content_type":"text/x-python","patch_set":11,"id":"b0a45b5e_64edbec5","line":201,"in_reply_to":"77c32fdb_61725d9a","updated":"2023-06-21 09:56:09.000000000","message":"\u003e Maybe filtering out everything that is \"not in (completed, error)\" \n\nI meant to say: (available, error)","commit_id":"302095741a9ad204228112a895f52cf6cc1b96ae"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"44249ca9c5b2e1d2f5deaa3280c1df583cfbd4ec","unresolved":true,"context_lines":[{"line_number":198,"context_line":"        # TODO(smulcahy) implement full resume of backup and restore"},{"line_number":199,"context_line":"        # operations on restart (rather than simply resetting)."},{"line_number":200,"context_line":"        # We only need to deal with the backups that aren\u0027t complete."},{"line_number":201,"context_line":"        incomplete_status \u003d (fields.BackupStatus.CREATING,"},{"line_number":202,"context_line":"                             fields.BackupStatus.RESTORING,"},{"line_number":203,"context_line":"                             fields.BackupStatus.DELETING,"},{"line_number":204,"context_line":"                             fields.BackupStatus.ERROR)"}],"source_content_type":"text/x-python","patch_set":11,"id":"cc266c6a_d99a5c38","line":201,"in_reply_to":"b0a45b5e_64edbec5","updated":"2023-08-23 10:47:01.000000000","message":"+1 to the negative filtering","commit_id":"302095741a9ad204228112a895f52cf6cc1b96ae"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"f523e7225f45c8dea2c77fdba3e77aad210fb483","unresolved":true,"context_lines":[{"line_number":198,"context_line":"        # TODO(smulcahy) implement full resume of backup and restore"},{"line_number":199,"context_line":"        # operations on restart (rather than simply resetting)."},{"line_number":200,"context_line":"        # We only need to deal with the backups that aren\u0027t complete."},{"line_number":201,"context_line":"        incomplete_status \u003d (fields.BackupStatus.CREATING,"},{"line_number":202,"context_line":"                             fields.BackupStatus.RESTORING,"},{"line_number":203,"context_line":"                             fields.BackupStatus.DELETING,"},{"line_number":204,"context_line":"                             fields.BackupStatus.ERROR)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3443fe9e_7a90fb78","line":201,"in_reply_to":"cc266c6a_d99a5c38","updated":"2023-08-26 17:55:26.000000000","message":"Gorka is there a way you could take this patchset over and make the required changes? I hondestly doubt yenai is still working on this one or reading these messages.","commit_id":"302095741a9ad204228112a895f52cf6cc1b96ae"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6fbc05aa95b50270e658c52ac497436e055d029b","unresolved":true,"context_lines":[{"line_number":200,"context_line":"        incomplete_status \u003d tuple(set(fields.BackupStatus.ALL) -"},{"line_number":201,"context_line":"                                  set((fields.BackupStatus.AVAILABLE,)))"},{"line_number":202,"context_line":"        backups \u003d objects.BackupList.get_all("},{"line_number":203,"context_line":"            ctxt, filters\u003d{\u0027host\u0027: self.host, \u0027status\u0027: incomplete_status})"},{"line_number":204,"context_line":"        for backup in backups:"},{"line_number":205,"context_line":"            try:"},{"line_number":206,"context_line":"                self._cleanup_one_backup(ctxt, backup)"}],"source_content_type":"text/x-python","patch_set":12,"id":"32ee9bbe_31d5fb57","line":203,"range":{"start_line":203,"start_character":46,"end_line":203,"end_character":73},"updated":"2023-11-17 17:02:59.000000000","message":"One thing about this change is that backups with no status set would be returned previously, but now they won\u0027t.  Looking at cinder/objects/backup.py, \u0027status\u0027 is a nullable field, and it doesn\u0027t look like it\u0027s set when the object is created in the database.  I don\u0027t know if it\u0027s possible for this condition to occur in real life, or whether the cleanup code would barf if there was no status.  Could you look into, it?  It seems like a backup object with no status is something that we should clean up (unless it\u0027s never going to happen).","commit_id":"130304b6d77ae6113d805856ed27035d8dbd1791"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"507d7327a220ef894dbff147dab3ea92bb3f02af","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        incomplete_status \u003d tuple(set(fields.BackupStatus.ALL) -"},{"line_number":201,"context_line":"                                  set((fields.BackupStatus.AVAILABLE,)))"},{"line_number":202,"context_line":"        backups \u003d objects.BackupList.get_all("},{"line_number":203,"context_line":"            ctxt, filters\u003d{\u0027host\u0027: self.host, \u0027status\u0027: incomplete_status})"},{"line_number":204,"context_line":"        for backup in backups:"},{"line_number":205,"context_line":"            try:"},{"line_number":206,"context_line":"                self._cleanup_one_backup(ctxt, backup)"}],"source_content_type":"text/x-python","patch_set":12,"id":"c202c359_c7d832c5","line":203,"range":{"start_line":203,"start_character":46,"end_line":203,"end_character":73},"in_reply_to":"32ee9bbe_31d5fb57","updated":"2024-01-19 05:39:33.000000000","message":"Done","commit_id":"130304b6d77ae6113d805856ed27035d8dbd1791"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f3f54a6631792d422dba5771599fa6de20d60c95","unresolved":true,"context_lines":[{"line_number":197,"context_line":"        # TODO(smulcahy) implement full resume of backup and restore"},{"line_number":198,"context_line":"        # operations on restart (rather than simply resetting)."},{"line_number":199,"context_line":"        # We only need to deal with the backups that aren\u0027t complete."},{"line_number":200,"context_line":"        incomplete_status \u003d tuple(set(fields.BackupStatus.ALL) -"},{"line_number":201,"context_line":"                                  set((fields.BackupStatus.AVAILABLE,)))"},{"line_number":202,"context_line":"        backups \u003d objects.BackupList.get_all("},{"line_number":203,"context_line":"            ctxt, filters\u003d{\u0027host\u0027: self.host, \u0027status\u0027: incomplete_status})"},{"line_number":204,"context_line":"        backups +\u003d objects.BackupList.get_all("}],"source_content_type":"text/x-python","patch_set":14,"id":"e69145b1_6c758cd6","line":201,"range":{"start_line":200,"start_character":0,"end_line":201,"end_character":72},"updated":"2024-02-01 15:43:22.000000000","message":"-1: I think this is a bit computing heavy: build 2 hash maps (sets), do the diff of 2 sets, and then convert to a tuple.\n\nWe could do something like this when we add support to None in the DB laye:\n\n```\nincomplete_status \u003d list(field.BackupStatus.ALL)\navailable_pos \u003d incomplete_status.index(field.BackupStatus.AVAILABLE)\nincomplete_status[available_pos] \u003d None\n```","commit_id":"aa224f3fda5f67bf028c1febe8418d3c4a27f1ad"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"fad47169308f9143b6911bd1819d965c61e61f71","unresolved":false,"context_lines":[{"line_number":197,"context_line":"        # TODO(smulcahy) implement full resume of backup and restore"},{"line_number":198,"context_line":"        # operations on restart (rather than simply resetting)."},{"line_number":199,"context_line":"        # We only need to deal with the backups that aren\u0027t complete."},{"line_number":200,"context_line":"        incomplete_status \u003d tuple(set(fields.BackupStatus.ALL) -"},{"line_number":201,"context_line":"                                  set((fields.BackupStatus.AVAILABLE,)))"},{"line_number":202,"context_line":"        backups \u003d objects.BackupList.get_all("},{"line_number":203,"context_line":"            ctxt, filters\u003d{\u0027host\u0027: self.host, \u0027status\u0027: incomplete_status})"},{"line_number":204,"context_line":"        backups +\u003d objects.BackupList.get_all("}],"source_content_type":"text/x-python","patch_set":14,"id":"57eb04d2_ee35fe3c","line":201,"range":{"start_line":200,"start_character":0,"end_line":201,"end_character":72},"in_reply_to":"e69145b1_6c758cd6","updated":"2024-02-16 00:21:53.000000000","message":"Although Knuth said that premature optimization is the root of all evil, I happen to hate this code too, for a different reason. All these parens and commas are super fragile. I\u0027m going to use your suggestion below. Looks more uniform and stylish too.","commit_id":"aa224f3fda5f67bf028c1febe8418d3c4a27f1ad"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f3f54a6631792d422dba5771599fa6de20d60c95","unresolved":true,"context_lines":[{"line_number":199,"context_line":"        # We only need to deal with the backups that aren\u0027t complete."},{"line_number":200,"context_line":"        incomplete_status \u003d tuple(set(fields.BackupStatus.ALL) -"},{"line_number":201,"context_line":"                                  set((fields.BackupStatus.AVAILABLE,)))"},{"line_number":202,"context_line":"        backups \u003d objects.BackupList.get_all("},{"line_number":203,"context_line":"            ctxt, filters\u003d{\u0027host\u0027: self.host, \u0027status\u0027: incomplete_status})"},{"line_number":204,"context_line":"        backups +\u003d objects.BackupList.get_all("},{"line_number":205,"context_line":"            ctxt, filters\u003d{\u0027host\u0027: self.host, \u0027status\u0027: None})"},{"line_number":206,"context_line":"        for backup in backups:"},{"line_number":207,"context_line":"            try:"},{"line_number":208,"context_line":"                self._cleanup_one_backup(ctxt, backup)"}],"source_content_type":"text/x-python","patch_set":14,"id":"9821e07b_5fe6f6fa","line":205,"range":{"start_line":202,"start_character":0,"end_line":205,"end_character":62},"updated":"2024-02-01 15:43:22.000000000","message":"-1: We should not be doing 2 requests were one was enough.\n\nPlease add the logic at the sqlalchemy low level code to make it smart enough to do the right thing when one of the items in the iterable is `None`.\n\nAs I mentioned in the mailing list, this is something that should be added in the `_process_backups_filters` method.\n\nI think this would get the job done, and you would just need to pass the tuple with the `None` value in it.\n\n ```\ndiff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py\nindex fe3e272f6a..2d8320fda2 100644\n--- a/cinder/db/sqlalchemy/api.py\n+++ b/cinder/db/sqlalchemy/api.py\n@@ -6430,6 +6430,9 @@ def _process_backups_filters(query, filters):\n                 col_attr \u003d getattr(models.Backup, \u0027backup_metadata\u0027)\n                 for k, v in value.items():\n                     query \u003d query.filter(col_attr.any(key\u003dk, value\u003dv))\n+            elif isinstance(value, abc.Iterable) and None in value:\n+                orm_field \u003d getattr(models.Backup, key)\n+                query \u003d query.filter(or_(orm_field \u003d\u003d v for v in value))\n             else:\n                 filters_dict[key] \u003d value\n```","commit_id":"aa224f3fda5f67bf028c1febe8418d3c4a27f1ad"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"f116e1a8d0363aa847189171a45b76d42a8324ff","unresolved":true,"context_lines":[{"line_number":199,"context_line":"        # We only need to deal with the backups that aren\u0027t complete."},{"line_number":200,"context_line":"        incomplete_status \u003d tuple(set(fields.BackupStatus.ALL) -"},{"line_number":201,"context_line":"                                  set((fields.BackupStatus.AVAILABLE,)))"},{"line_number":202,"context_line":"        backups \u003d objects.BackupList.get_all("},{"line_number":203,"context_line":"            ctxt, filters\u003d{\u0027host\u0027: self.host, \u0027status\u0027: incomplete_status})"},{"line_number":204,"context_line":"        backups +\u003d objects.BackupList.get_all("},{"line_number":205,"context_line":"            ctxt, filters\u003d{\u0027host\u0027: self.host, \u0027status\u0027: None})"},{"line_number":206,"context_line":"        for backup in backups:"},{"line_number":207,"context_line":"            try:"},{"line_number":208,"context_line":"                self._cleanup_one_backup(ctxt, backup)"}],"source_content_type":"text/x-python","patch_set":14,"id":"3715bff4_31976954","line":205,"range":{"start_line":202,"start_character":0,"end_line":205,"end_character":62},"in_reply_to":"9821e07b_5fe6f6fa","updated":"2024-02-03 05:34:30.000000000","message":"This works, but such special-casing really grates. I\u0027m going to try a bit of layer penetration instead. Please take a look, and we can compare it with the proposal above.","commit_id":"aa224f3fda5f67bf028c1febe8418d3c4a27f1ad"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"83c3ae57fabd6034dd341d89c5ffd39d3dc35583","unresolved":true,"context_lines":[{"line_number":200,"context_line":"        # N.B. NULL status is possible and we consider it incomplete."},{"line_number":201,"context_line":"        incomplete_status \u003d tuple(set(fields.BackupStatus.ALL) -"},{"line_number":202,"context_line":"                                  set((fields.BackupStatus.AVAILABLE,)))"},{"line_number":203,"context_line":"        incomplete_status +\u003d (None,)"},{"line_number":204,"context_line":"        backups \u003d objects.BackupList.get_all("},{"line_number":205,"context_line":"            ctxt, filters\u003d{\u0027host\u0027: self.host, \u0027status\u0027: incomplete_status})"},{"line_number":206,"context_line":"        for backup in backups:"}],"source_content_type":"text/x-python","patch_set":16,"id":"d109ed57_7d4ee06f","line":203,"updated":"2024-02-15 15:03:54.000000000","message":"It you don\u0027t like the previously proposed approach you can always do something closer to what you are doing:\n\n```\n  incomplete_status \u003d list(fields.BackupStatus.ALL)\n  incomplete_status.remove(fields.BackupStatus.AVAILABLE)\n  incomplete_status.append(None)\n```\n\nIn this case we build 1 list and remove 1 item, and add another\nBecause the current code build 2 sets (hash maps), and 4 tuples, and does a set difference...","commit_id":"52d35f93aa3acb26e86f3467840354ad99ea1711"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"fad47169308f9143b6911bd1819d965c61e61f71","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        # N.B. NULL status is possible and we consider it incomplete."},{"line_number":201,"context_line":"        incomplete_status \u003d tuple(set(fields.BackupStatus.ALL) -"},{"line_number":202,"context_line":"                                  set((fields.BackupStatus.AVAILABLE,)))"},{"line_number":203,"context_line":"        incomplete_status +\u003d (None,)"},{"line_number":204,"context_line":"        backups \u003d objects.BackupList.get_all("},{"line_number":205,"context_line":"            ctxt, filters\u003d{\u0027host\u0027: self.host, \u0027status\u0027: incomplete_status})"},{"line_number":206,"context_line":"        for backup in backups:"}],"source_content_type":"text/x-python","patch_set":16,"id":"aa322e72_ab92fb47","line":203,"in_reply_to":"d109ed57_7d4ee06f","updated":"2024-02-16 00:21:53.000000000","message":"Acknowledged","commit_id":"52d35f93aa3acb26e86f3467840354ad99ea1711"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"3af0e2fc484ed881ee48d2ea5c8d85f5e4bca745","unresolved":true,"context_lines":[{"line_number":202,"context_line":"        incomplete_status.remove(fields.BackupStatus.AVAILABLE)"},{"line_number":203,"context_line":"        incomplete_status.append(None)"},{"line_number":204,"context_line":"        backups \u003d objects.BackupList.get_all("},{"line_number":205,"context_line":"            ctxt, filters\u003d{\u0027host\u0027: self.host, \u0027status\u0027: incomplete_status})"},{"line_number":206,"context_line":"        for backup in backups:"},{"line_number":207,"context_line":"            try:"},{"line_number":208,"context_line":"                self._cleanup_one_backup(ctxt, backup)"}],"source_content_type":"text/x-python","patch_set":17,"id":"e410248f_bfa50317","line":205,"updated":"2024-04-16 10:06:45.000000000","message":"Does it make sense to also add indexes for `host`and `status` then?\nThere was some discussion around this at the PTG (see https://etherpad.opendev.org/p/dalmatian-ptg-cinder#L251).\n\n1) User API requests are usually about finding non-deleted backups for a project or even particular volume (there was the idea to add an index on volume_id).\n\n2) Cinder-backup itself still does look for backups that \n\n a) don\u0027t yet have any host assigned\n b) their own hostname assigned\n c) have a certain status","commit_id":"b52eb917b4150e68e1a3d83b2374d017e7addaa9"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"e9e1d79cc901dd9676c5309c950c0fd833fe1416","unresolved":true,"context_lines":[{"line_number":202,"context_line":"        incomplete_status.remove(fields.BackupStatus.AVAILABLE)"},{"line_number":203,"context_line":"        incomplete_status.append(None)"},{"line_number":204,"context_line":"        backups \u003d objects.BackupList.get_all("},{"line_number":205,"context_line":"            ctxt, filters\u003d{\u0027host\u0027: self.host, \u0027status\u0027: incomplete_status})"},{"line_number":206,"context_line":"        for backup in backups:"},{"line_number":207,"context_line":"            try:"},{"line_number":208,"context_line":"                self._cleanup_one_backup(ctxt, backup)"}],"source_content_type":"text/x-python","patch_set":17,"id":"bc9293c3_8d7cba4d","line":205,"in_reply_to":"e410248f_bfa50317","updated":"2024-05-17 15:47:06.000000000","message":"Regarding the host question, the old code did that, didn\u0027t it?\n\nSo yes, if you are making a backup at a host A, and crash and recover as host B, then the backup service at B will not pick up incomplete backups of A for continuation. So even if it inherited all the storage layout of A, with all the backups and volumes, and could in theory resume or restart the incomplete backups, if would not do that.\n\nHopefully I understood what the comment was about.\n\nWhat the user API does is only relevant in regards to it allowing to find and discard failed backups that the new service at B refuses to restart automatically here, I think.","commit_id":"b52eb917b4150e68e1a3d83b2374d017e7addaa9"}],"cinder/db/sqlalchemy/api.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6fbc05aa95b50270e658c52ac497436e055d029b","unresolved":true,"context_lines":[{"line_number":6442,"context_line":"                col_attr \u003d getattr(models.Backup, \u0027backup_metadata\u0027)"},{"line_number":6443,"context_line":"                for k, v in value.items():"},{"line_number":6444,"context_line":"                    query \u003d query.filter(col_attr.any(key\u003dk, value\u003dv))"},{"line_number":6445,"context_line":"            elif isinstance(value, (list, tuple, set, frozenset)):"},{"line_number":6446,"context_line":"                # Looking for values in a list; apply to query directly."},{"line_number":6447,"context_line":"                column_attr \u003d getattr(models.Backup, key)"},{"line_number":6448,"context_line":"                query \u003d query.filter(column_attr.in_(value))"},{"line_number":6449,"context_line":"            else:"},{"line_number":6450,"context_line":"                filters_dict[key] \u003d value"},{"line_number":6451,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"02e6936b_c0cc5659","line":6448,"range":{"start_line":6445,"start_character":0,"end_line":6448,"end_character":60},"updated":"2023-11-17 17:02:59.000000000","message":"this is the same thing we do in _process_volume_filters() (I\u0027m just noting that it\u0027s a standard cinder code pattern)","commit_id":"130304b6d77ae6113d805856ed27035d8dbd1791"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"83c3ae57fabd6034dd341d89c5ffd39d3dc35583","unresolved":true,"context_lines":[{"line_number":6430,"context_line":"                col_attr \u003d getattr(models.Backup, \u0027backup_metadata\u0027)"},{"line_number":6431,"context_line":"                for k, v in value.items():"},{"line_number":6432,"context_line":"                    query \u003d query.filter(col_attr.any(key\u003dk, value\u003dv))"},{"line_number":6433,"context_line":"            elif isinstance(value, (list, tuple, set, frozenset)):"},{"line_number":6434,"context_line":"                orm_field \u003d getattr(models.Backup, key)"},{"line_number":6435,"context_line":"                query \u003d query.filter(or_(orm_field \u003d\u003d v for v in value))"},{"line_number":6436,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":16,"id":"20c100fa_9e260574","line":6433,"range":{"start_line":6433,"start_character":16,"end_line":6433,"end_character":65},"updated":"2024-02-15 15:03:54.000000000","message":"nit: I usually prefer to check for iterable, like we do in other places of this file, but code looks good:\n\n```\n    elif isinstance(value, abc.Iterable) and not isinstance(value, str):\n```","commit_id":"52d35f93aa3acb26e86f3467840354ad99ea1711"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"fad47169308f9143b6911bd1819d965c61e61f71","unresolved":false,"context_lines":[{"line_number":6430,"context_line":"                col_attr \u003d getattr(models.Backup, \u0027backup_metadata\u0027)"},{"line_number":6431,"context_line":"                for k, v in value.items():"},{"line_number":6432,"context_line":"                    query \u003d query.filter(col_attr.any(key\u003dk, value\u003dv))"},{"line_number":6433,"context_line":"            elif isinstance(value, (list, tuple, set, frozenset)):"},{"line_number":6434,"context_line":"                orm_field \u003d getattr(models.Backup, key)"},{"line_number":6435,"context_line":"                query \u003d query.filter(or_(orm_field \u003d\u003d v for v in value))"},{"line_number":6436,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":16,"id":"b94795f4_6be366c5","line":6433,"range":{"start_line":6433,"start_character":16,"end_line":6433,"end_character":65},"in_reply_to":"20c100fa_9e260574","updated":"2024-02-16 00:21:53.000000000","message":"I tried abc.Iterable and unfortunately it matches unrelated cases, producing a multitude of bogus \"host \u003d ?\" condditions in the SQL statement. I had to go with the isinstance() here, else nothing works.","commit_id":"52d35f93aa3acb26e86f3467840354ad99ea1711"}],"cinder/objects/backup.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6fbc05aa95b50270e658c52ac497436e055d029b","unresolved":true,"context_lines":[{"line_number":58,"context_line":"        \u0027container\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":59,"context_line":"        \u0027parent_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":60,"context_line":"        \u0027parent\u0027: fields.ObjectField(\u0027Backup\u0027, nullable\u003dTrue),"},{"line_number":61,"context_line":"        \u0027status\u0027: c_fields.BackupStatusField(nullable\u003dTrue),"},{"line_number":62,"context_line":"        \u0027fail_reason\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":63,"context_line":"        \u0027size\u0027: fields.IntegerField(nullable\u003dTrue),"},{"line_number":64,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"5797332f_231be663","line":61,"updated":"2023-11-17 17:02:59.000000000","message":"Note that this is nullable, and it isn\u0027t set on creation.  Do we know that it will always have a value?","commit_id":"130304b6d77ae6113d805856ed27035d8dbd1791"}],"cinder/tests/unit/backup/test_backup.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f6bb7db545d1a991cc8f2d7959835a5ef51d77f9","unresolved":false,"context_lines":[{"line_number":395,"context_line":"            status\u003dfields.BackupStatus.DELETING)"},{"line_number":396,"context_line":"        bak3 \u003d self._create_backup_db_entry("},{"line_number":397,"context_line":"            status\u003dfields.BackupStatus.RESTORING)"},{"line_number":398,"context_line":"        fake_backup_list \u003d [bak1, bak2, bak3]"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        mock_backup_get_by_host \u003d self.mock_object("},{"line_number":401,"context_line":"            objects.BackupList, \u0027get_all\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_e79823cd","line":398,"updated":"2019-08-29 13:43:03.000000000","message":"-1: I know the code works, but the test needs to add some backups that will not be restored, otherwise we don\u0027t see the difference with the code we have now and it won\u0027t prevent  regressions.","commit_id":"69cdf18318f69422a6c1cbef30d7abaf95ebf737"},{"author":{"_account_id":25837,"name":"yenai","email":"yenai_yewu@cmss.chinamobile.com","username":"yenai2008"},"change_message_id":"0eb73bc6c7aad03579b5d2d2c55186e5a56728b1","unresolved":false,"context_lines":[{"line_number":395,"context_line":"            status\u003dfields.BackupStatus.DELETING)"},{"line_number":396,"context_line":"        bak3 \u003d self._create_backup_db_entry("},{"line_number":397,"context_line":"            status\u003dfields.BackupStatus.RESTORING)"},{"line_number":398,"context_line":"        fake_backup_list \u003d [bak1, bak2, bak3]"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        mock_backup_get_by_host \u003d self.mock_object("},{"line_number":401,"context_line":"            objects.BackupList, \u0027get_all\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5faad753_a0e2282e","line":398,"in_reply_to":"7faddb67_e79823cd","updated":"2019-09-11 10:52:24.000000000","message":"Done","commit_id":"69cdf18318f69422a6c1cbef30d7abaf95ebf737"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f5a100b57d19dd7726d8cc04b7c510ec837e1348","unresolved":false,"context_lines":[{"line_number":408,"context_line":"            self.backup_mgr, \u0027_cleanup_temp_volumes_snapshots_for_one_backup\u0027)"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"        self.assertIsNone("},{"line_number":411,"context_line":"            self.backup_mgr._cleanup_incomplete_backup_operations(self.ctxt))"},{"line_number":412,"context_line":""},{"line_number":413,"context_line":"        self.assertEqual(len(fake_incomplete_backup_list),"},{"line_number":414,"context_line":"                         mock_backup_cleanup.call_count)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_40c81421","line":411,"updated":"2019-09-11 11:21:53.000000000","message":"nit: The call to the method we are testing is usually better (for clarity) to not be included in an assert. ie:\n\n result \u003d self.backup_mgr._cleanup_incomplete_backup_operations(self.ctxt))\n self.assertIsNone(result)","commit_id":"015a54e4fa746599676ae24c739ad2c173531f69"},{"author":{"_account_id":25837,"name":"yenai","email":"yenai_yewu@cmss.chinamobile.com","username":"yenai2008"},"change_message_id":"28b151d9522981cbe1103568f4e371376b153706","unresolved":false,"context_lines":[{"line_number":408,"context_line":"            self.backup_mgr, \u0027_cleanup_temp_volumes_snapshots_for_one_backup\u0027)"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"        self.assertIsNone("},{"line_number":411,"context_line":"            self.backup_mgr._cleanup_incomplete_backup_operations(self.ctxt))"},{"line_number":412,"context_line":""},{"line_number":413,"context_line":"        self.assertEqual(len(fake_incomplete_backup_list),"},{"line_number":414,"context_line":"                         mock_backup_cleanup.call_count)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_462cac01","line":411,"in_reply_to":"5faad753_40c81421","updated":"2019-09-11 12:38:06.000000000","message":"Done","commit_id":"015a54e4fa746599676ae24c739ad2c173531f69"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f5a100b57d19dd7726d8cc04b7c510ec837e1348","unresolved":false,"context_lines":[{"line_number":412,"context_line":""},{"line_number":413,"context_line":"        self.assertEqual(len(fake_incomplete_backup_list),"},{"line_number":414,"context_line":"                         mock_backup_cleanup.call_count)"},{"line_number":415,"context_line":"        mock_backup_cleanup.assert_called_with(self.ctxt, bak1 or bak2 or bak3)"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"        self.assertEqual(len(fake_incomplete_backup_list),"},{"line_number":418,"context_line":"                         mock_temp_cleanup.call_count)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_a0e0e897","line":415,"updated":"2019-09-11 11:21:53.000000000","message":"-1: this is wrong, bak1 or bak2 or bak3 is evaluated as bool(bak1) or bool(bak2) or bool(bak3) which results in True, so this is not testing what we want.","commit_id":"015a54e4fa746599676ae24c739ad2c173531f69"},{"author":{"_account_id":25837,"name":"yenai","email":"yenai_yewu@cmss.chinamobile.com","username":"yenai2008"},"change_message_id":"28b151d9522981cbe1103568f4e371376b153706","unresolved":false,"context_lines":[{"line_number":412,"context_line":""},{"line_number":413,"context_line":"        self.assertEqual(len(fake_incomplete_backup_list),"},{"line_number":414,"context_line":"                         mock_backup_cleanup.call_count)"},{"line_number":415,"context_line":"        mock_backup_cleanup.assert_called_with(self.ctxt, bak1 or bak2 or bak3)"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"        self.assertEqual(len(fake_incomplete_backup_list),"},{"line_number":418,"context_line":"                         mock_temp_cleanup.call_count)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_662f68f6","line":415,"in_reply_to":"5faad753_a0e0e897","updated":"2019-09-11 12:38:06.000000000","message":"Done","commit_id":"015a54e4fa746599676ae24c739ad2c173531f69"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f5a100b57d19dd7726d8cc04b7c510ec837e1348","unresolved":false,"context_lines":[{"line_number":414,"context_line":"                         mock_backup_cleanup.call_count)"},{"line_number":415,"context_line":"        mock_backup_cleanup.assert_called_with(self.ctxt, bak1 or bak2 or bak3)"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"        self.assertEqual(len(fake_incomplete_backup_list),"},{"line_number":418,"context_line":"                         mock_temp_cleanup.call_count)"},{"line_number":419,"context_line":"        mock_temp_cleanup.assert_called_with(self.ctxt, bak1 or bak2 or bak3)"},{"line_number":420,"context_line":""},{"line_number":421,"context_line":"    @mock.patch(\u0027cinder.objects.BackupList\u0027)"},{"line_number":422,"context_line":"    @mock.patch.object(manager.BackupManager, \u0027_cleanup_one_backup\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_e0ffc031","line":419,"range":{"start_line":417,"start_character":0,"end_line":419,"end_character":77},"updated":"2019-09-11 11:21:53.000000000","message":"-1: Aren\u0027t these 2 the same as L413-L415?","commit_id":"015a54e4fa746599676ae24c739ad2c173531f69"},{"author":{"_account_id":25837,"name":"yenai","email":"yenai_yewu@cmss.chinamobile.com","username":"yenai2008"},"change_message_id":"28b151d9522981cbe1103568f4e371376b153706","unresolved":false,"context_lines":[{"line_number":414,"context_line":"                         mock_backup_cleanup.call_count)"},{"line_number":415,"context_line":"        mock_backup_cleanup.assert_called_with(self.ctxt, bak1 or bak2 or bak3)"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"        self.assertEqual(len(fake_incomplete_backup_list),"},{"line_number":418,"context_line":"                         mock_temp_cleanup.call_count)"},{"line_number":419,"context_line":"        mock_temp_cleanup.assert_called_with(self.ctxt, bak1 or bak2 or bak3)"},{"line_number":420,"context_line":""},{"line_number":421,"context_line":"    @mock.patch(\u0027cinder.objects.BackupList\u0027)"},{"line_number":422,"context_line":"    @mock.patch.object(manager.BackupManager, \u0027_cleanup_one_backup\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_2612d0b2","line":419,"range":{"start_line":417,"start_character":0,"end_line":419,"end_character":77},"in_reply_to":"5faad753_e0ffc031","updated":"2019-09-11 12:38:06.000000000","message":"Not the same.\nOne is _cleanup_one_backup, another is _cleanup_temp_volumes_snapshots_for_one_backup.","commit_id":"015a54e4fa746599676ae24c739ad2c173531f69"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"60508dfa1d695ca40f71df6782d953b5da87cca9","unresolved":false,"context_lines":[{"line_number":433,"context_line":"    def test_cleanup_incomplete_backup_operations_with_exceptions(self):"},{"line_number":434,"context_line":"        \"\"\"Test cleanup resilience in the face of exceptions.\"\"\""},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"        bak1 \u003d self._create_backup_db_entry("},{"line_number":437,"context_line":"            status\u003dfields.BackupStatus.CREATING)"},{"line_number":438,"context_line":"        bak2 \u003d self._create_backup_db_entry("},{"line_number":439,"context_line":"            status\u003dfields.BackupStatus.DELETING)"},{"line_number":440,"context_line":"        bak3 \u003d self._create_backup_db_entry("},{"line_number":441,"context_line":"            status\u003dfields.BackupStatus.RESTORING)"},{"line_number":442,"context_line":"        bak4 \u003d self._create_backup_db_entry("},{"line_number":443,"context_line":"            status\u003dfields.BackupStatus.ERROR)"},{"line_number":444,"context_line":"        fake_backup_list \u003d [bak1, bak2, bak3, bak4]"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"        self._create_backup_db_entry(status\u003dfields.BackupStatus.ERROR_DELETING)"},{"line_number":447,"context_line":"        self._create_backup_db_entry(status\u003dfields.BackupStatus.AVAILABLE)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_21af3c2c","line":444,"range":{"start_line":436,"start_character":0,"end_line":444,"end_character":51},"updated":"2019-11-12 13:59:11.000000000","message":"nit: Looks cleaner:\n\n        clean_statuses \u003d (fields.BackupStatus.CREATING, \n                          fields.BackupStatus.DELETING,\n                          fields.BackupStatus.RESTORING,\n                          fields.BackupStatus.ERROR)\n        fake_backup_list \u003d [self._create_backup_db_entry(status\u003dstatus)\n                            for status in clean_statuses]","commit_id":"8018a9810b912eaf518ad7808e12f5ac5b50af51"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"60508dfa1d695ca40f71df6782d953b5da87cca9","unresolved":false,"context_lines":[{"line_number":443,"context_line":"            status\u003dfields.BackupStatus.ERROR)"},{"line_number":444,"context_line":"        fake_backup_list \u003d [bak1, bak2, bak3, bak4]"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"        self._create_backup_db_entry(status\u003dfields.BackupStatus.ERROR_DELETING)"},{"line_number":447,"context_line":"        self._create_backup_db_entry(status\u003dfields.BackupStatus.AVAILABLE)"},{"line_number":448,"context_line":"        self._create_backup_db_entry(status\u003dfields.BackupStatus.DELETED)"},{"line_number":449,"context_line":""},{"line_number":450,"context_line":"        mock_backup_cleanup \u003d self.mock_object("},{"line_number":451,"context_line":"            self.backup_mgr, \u0027_cleanup_one_backup\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_619c540e","line":448,"range":{"start_line":446,"start_character":0,"end_line":448,"end_character":72},"updated":"2019-11-12 13:59:11.000000000","message":"nit: I think we can remove this, we\u0027ve already tested the new filtering with the previous test.","commit_id":"8018a9810b912eaf518ad7808e12f5ac5b50af51"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"60508dfa1d695ca40f71df6782d953b5da87cca9","unresolved":false,"context_lines":[{"line_number":652,"context_line":"    def test_create_backup_with_bad_volume_status(self):"},{"line_number":653,"context_line":"        \"\"\"Test creating a backup from a volume with a bad status.\"\"\""},{"line_number":654,"context_line":"        vol_id \u003d self._create_volume_db_entry("},{"line_number":655,"context_line":"            status\u003d\u0027restoring-backup\u0027, size\u003d1)"},{"line_number":656,"context_line":"        backup \u003d self._create_backup_db_entry(volume_id\u003dvol_id)"},{"line_number":657,"context_line":"        self.assertRaises(exception.InvalidVolume,"},{"line_number":658,"context_line":"                          self.backup_mgr.create_backup,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_c163c818","line":655,"updated":"2019-11-12 13:59:11.000000000","message":"I don\u0027t think this change is necessary in this patch.","commit_id":"8018a9810b912eaf518ad7808e12f5ac5b50af51"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6fbc05aa95b50270e658c52ac497436e055d029b","unresolved":true,"context_lines":[{"line_number":398,"context_line":""},{"line_number":399,"context_line":"    def test_cleanup_incomplete_backup_operations(self):"},{"line_number":400,"context_line":"        \"\"\"Test cleanup resilience in the face of exceptions.\"\"\""},{"line_number":401,"context_line":"        # Only the backups in the status creating, restoring and deleting will"},{"line_number":402,"context_line":"        # be cleaned up."},{"line_number":403,"context_line":"        bak1 \u003d self._create_backup_db_entry("},{"line_number":404,"context_line":"            status\u003dfields.BackupStatus.CREATING)"},{"line_number":405,"context_line":"        bak2 \u003d self._create_backup_db_entry("}],"source_content_type":"text/x-python","patch_set":12,"id":"b7201bdd_32812715","line":402,"range":{"start_line":401,"start_character":0,"end_line":402,"end_character":24},"updated":"2023-11-17 17:02:59.000000000","message":"This doesn\u0027t seem to be true.  The len(fake_inc_backup_list ) \u003d\u003d 6, and at line 428, you can see that the mock backup has been called 6 times.  Further, you explicitly check at line 433 to see that a backup in ERROR status has been passed to the cleanup function.","commit_id":"130304b6d77ae6113d805856ed27035d8dbd1791"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"c9b944ffab2975aae50a55b815bc131c9f7f9c64","unresolved":false,"context_lines":[{"line_number":398,"context_line":""},{"line_number":399,"context_line":"    def test_cleanup_incomplete_backup_operations(self):"},{"line_number":400,"context_line":"        \"\"\"Test cleanup resilience in the face of exceptions.\"\"\""},{"line_number":401,"context_line":"        # Only the backups in the status creating, restoring and deleting will"},{"line_number":402,"context_line":"        # be cleaned up."},{"line_number":403,"context_line":"        bak1 \u003d self._create_backup_db_entry("},{"line_number":404,"context_line":"            status\u003dfields.BackupStatus.CREATING)"},{"line_number":405,"context_line":"        bak2 \u003d self._create_backup_db_entry("}],"source_content_type":"text/x-python","patch_set":12,"id":"f94a7a93_f6adc972","line":402,"range":{"start_line":401,"start_character":0,"end_line":402,"end_character":24},"in_reply_to":"b7201bdd_32812715","updated":"2024-02-01 04:46:23.000000000","message":"It\u0027s 7 now, because we added NULL. That said, the DB contains a completed backup too. So checking the length is appropriate (learned it the hard way, of course).\n\nThe docstring comment is wrong. It appears to be copy-pasted.","commit_id":"130304b6d77ae6113d805856ed27035d8dbd1791"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6fbc05aa95b50270e658c52ac497436e055d029b","unresolved":true,"context_lines":[{"line_number":412,"context_line":"            status\u003dfields.BackupStatus.ERROR_DELETING)"},{"line_number":413,"context_line":"        bak6 \u003d self._create_backup_db_entry("},{"line_number":414,"context_line":"            status\u003dfields.BackupStatus.DELETED)"},{"line_number":415,"context_line":"        fake_incomplete_backup_list \u003d [bak1, bak2, bak3, bak4, bak5, bak6]"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"        self._create_backup_db_entry(status\u003dfields.BackupStatus.AVAILABLE)"},{"line_number":418,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"6a7a6da1_a8d931b1","line":415,"updated":"2023-11-17 17:02:59.000000000","message":"Because you use the len of this list below when checking the call count, it\u0027s possible that if for some reason we introduce a new backup status (like COMPLETE or something) that also shouldn\u0027t be included in the list to check, this test won\u0027t break.  So it might be a good idea to check for that here, something like:\n\n    # make sure that we\u0027re dealing with all the appropriate backup statuses,\n    # that is, all of them except AVAILABLE\n    self.assertEqual(len(fields.BackupStatus.ALL) - 1, fake_incomplete_backup_list)\n\nThat way, if any new status is added, this test will break, and to fix it, the author will have to decide whether or not the backup should be included in the generated list or not.","commit_id":"130304b6d77ae6113d805856ed27035d8dbd1791"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"c9b944ffab2975aae50a55b815bc131c9f7f9c64","unresolved":false,"context_lines":[{"line_number":412,"context_line":"            status\u003dfields.BackupStatus.ERROR_DELETING)"},{"line_number":413,"context_line":"        bak6 \u003d self._create_backup_db_entry("},{"line_number":414,"context_line":"            status\u003dfields.BackupStatus.DELETED)"},{"line_number":415,"context_line":"        fake_incomplete_backup_list \u003d [bak1, bak2, bak3, bak4, bak5, bak6]"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"        self._create_backup_db_entry(status\u003dfields.BackupStatus.AVAILABLE)"},{"line_number":418,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"52173fd4_6c727598","line":415,"in_reply_to":"6a7a6da1_a8d931b1","updated":"2024-02-01 04:46:23.000000000","message":"Acknowledged","commit_id":"130304b6d77ae6113d805856ed27035d8dbd1791"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6fbc05aa95b50270e658c52ac497436e055d029b","unresolved":true,"context_lines":[{"line_number":425,"context_line":"            self.ctxt)"},{"line_number":426,"context_line":"        self.assertIsNone(result)"},{"line_number":427,"context_line":""},{"line_number":428,"context_line":"        self.assertEqual(len(fake_incomplete_backup_list),"},{"line_number":429,"context_line":"                         mock_backup_cleanup.call_count)"},{"line_number":430,"context_line":"        mock_backup_cleanup.assert_any_call(self.ctxt, bak1)"},{"line_number":431,"context_line":"        mock_backup_cleanup.assert_any_call(self.ctxt, bak2)"},{"line_number":432,"context_line":"        mock_backup_cleanup.assert_any_call(self.ctxt, bak3)"},{"line_number":433,"context_line":"        mock_backup_cleanup.assert_any_call(self.ctxt, bak4)"},{"line_number":434,"context_line":""},{"line_number":435,"context_line":"        self.assertEqual(len(fake_incomplete_backup_list),"},{"line_number":436,"context_line":"                         mock_temp_cleanup.call_count)"},{"line_number":437,"context_line":"        mock_temp_cleanup.assert_any_call(self.ctxt, bak1)"}],"source_content_type":"text/x-python","patch_set":12,"id":"bc513851_e941945c","line":434,"range":{"start_line":428,"start_character":0,"end_line":434,"end_character":0},"updated":"2023-11-17 17:02:59.000000000","message":"I\u0027m not clear on what you\u0027re checking here.  Line 428 shows that the mocked function was called 6 times, but you only validate 4 of them here.  So it\u0027s possible that one of the \"missing\" 2 calls is passed the AVAILABLE backup; we don\u0027t know.\n\nInstead of creating the AVAILABLE backup as an anonymous object at line 417, maybe you should hold a reference to it and then you can check here to make sure that the mocked backup function was not called with it as an argument.  Either that or check that it was called with bak5 and bak6, and then we\u0027ll know it wasn\u0027t called with the AVAILABLE backup by process of elimination.","commit_id":"130304b6d77ae6113d805856ed27035d8dbd1791"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"c9b944ffab2975aae50a55b815bc131c9f7f9c64","unresolved":false,"context_lines":[{"line_number":425,"context_line":"            self.ctxt)"},{"line_number":426,"context_line":"        self.assertIsNone(result)"},{"line_number":427,"context_line":""},{"line_number":428,"context_line":"        self.assertEqual(len(fake_incomplete_backup_list),"},{"line_number":429,"context_line":"                         mock_backup_cleanup.call_count)"},{"line_number":430,"context_line":"        mock_backup_cleanup.assert_any_call(self.ctxt, bak1)"},{"line_number":431,"context_line":"        mock_backup_cleanup.assert_any_call(self.ctxt, bak2)"},{"line_number":432,"context_line":"        mock_backup_cleanup.assert_any_call(self.ctxt, bak3)"},{"line_number":433,"context_line":"        mock_backup_cleanup.assert_any_call(self.ctxt, bak4)"},{"line_number":434,"context_line":""},{"line_number":435,"context_line":"        self.assertEqual(len(fake_incomplete_backup_list),"},{"line_number":436,"context_line":"                         mock_temp_cleanup.call_count)"},{"line_number":437,"context_line":"        mock_temp_cleanup.assert_any_call(self.ctxt, bak1)"}],"source_content_type":"text/x-python","patch_set":12,"id":"457df243_b530d5eb","line":434,"range":{"start_line":428,"start_character":0,"end_line":434,"end_character":0},"in_reply_to":"bc513851_e941945c","updated":"2024-02-01 04:46:23.000000000","message":"I made sure to check the whole thing.","commit_id":"130304b6d77ae6113d805856ed27035d8dbd1791"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6fbc05aa95b50270e658c52ac497436e055d029b","unresolved":true,"context_lines":[{"line_number":437,"context_line":"        mock_temp_cleanup.assert_any_call(self.ctxt, bak1)"},{"line_number":438,"context_line":"        mock_temp_cleanup.assert_any_call(self.ctxt, bak2)"},{"line_number":439,"context_line":"        mock_temp_cleanup.assert_any_call(self.ctxt, bak3)"},{"line_number":440,"context_line":"        mock_temp_cleanup.assert_any_call(self.ctxt, bak4)"},{"line_number":441,"context_line":""},{"line_number":442,"context_line":"    def test_cleanup_incomplete_backup_operations_with_exceptions(self):"},{"line_number":443,"context_line":"        \"\"\"Test cleanup resilience in the face of exceptions.\"\"\""}],"source_content_type":"text/x-python","patch_set":12,"id":"f706bea0_0bdc26ef","line":440,"updated":"2023-11-17 17:02:59.000000000","message":"Comment above also applies here.","commit_id":"130304b6d77ae6113d805856ed27035d8dbd1791"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"c9b944ffab2975aae50a55b815bc131c9f7f9c64","unresolved":false,"context_lines":[{"line_number":437,"context_line":"        mock_temp_cleanup.assert_any_call(self.ctxt, bak1)"},{"line_number":438,"context_line":"        mock_temp_cleanup.assert_any_call(self.ctxt, bak2)"},{"line_number":439,"context_line":"        mock_temp_cleanup.assert_any_call(self.ctxt, bak3)"},{"line_number":440,"context_line":"        mock_temp_cleanup.assert_any_call(self.ctxt, bak4)"},{"line_number":441,"context_line":""},{"line_number":442,"context_line":"    def test_cleanup_incomplete_backup_operations_with_exceptions(self):"},{"line_number":443,"context_line":"        \"\"\"Test cleanup resilience in the face of exceptions.\"\"\""}],"source_content_type":"text/x-python","patch_set":12,"id":"7e512ca0_fcb2e823","line":440,"in_reply_to":"f706bea0_0bdc26ef","updated":"2024-02-01 04:46:23.000000000","message":"Done","commit_id":"130304b6d77ae6113d805856ed27035d8dbd1791"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"83c3ae57fabd6034dd341d89c5ffd39d3dc35583","unresolved":true,"context_lines":[{"line_number":401,"context_line":"        # For correct operation, this test relies on the DB being"},{"line_number":402,"context_line":"        # pre-populated by a properly complete backup."},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"        bak0 \u003d self._create_backup_db_entry(status\u003dNone)"},{"line_number":405,"context_line":"        bak1 \u003d self._create_backup_db_entry("},{"line_number":406,"context_line":"            status\u003dfields.BackupStatus.CREATING)"},{"line_number":407,"context_line":"        bak2 \u003d self._create_backup_db_entry("},{"line_number":408,"context_line":"            status\u003dfields.BackupStatus.DELETING)"},{"line_number":409,"context_line":"        bak3 \u003d self._create_backup_db_entry("},{"line_number":410,"context_line":"            status\u003dfields.BackupStatus.RESTORING)"},{"line_number":411,"context_line":"        bak4 \u003d self._create_backup_db_entry("},{"line_number":412,"context_line":"            status\u003dfields.BackupStatus.ERROR)"},{"line_number":413,"context_line":"        bak5 \u003d self._create_backup_db_entry("},{"line_number":414,"context_line":"            status\u003dfields.BackupStatus.ERROR_DELETING)"},{"line_number":415,"context_line":"        bak6 \u003d self._create_backup_db_entry("},{"line_number":416,"context_line":"            status\u003dfields.BackupStatus.DELETED)"},{"line_number":417,"context_line":"        fake_incomplete_backup_list \u003d ["},{"line_number":418,"context_line":"            bak0, bak1, bak2, bak3, bak4, bak5, bak6]"},{"line_number":419,"context_line":""},{"line_number":420,"context_line":"        self._create_backup_db_entry(status\u003dfields.BackupStatus.AVAILABLE)"},{"line_number":421,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"491ee2e2_9ba720e2","line":418,"range":{"start_line":404,"start_character":1,"end_line":418,"end_character":53},"updated":"2024-02-15 15:03:54.000000000","message":"nit: we could create this without the temporary variables:\n\n```\n  fake_incomplete_backup_list \u003d [\n      self._create_backup_db_entry(status\u003ds)\n      for s in (None, fields.BackupStatus.CREATING, fields.BackupStatus.DELETING, ...)\n  ]\n```","commit_id":"52d35f93aa3acb26e86f3467840354ad99ea1711"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"fad47169308f9143b6911bd1819d965c61e61f71","unresolved":false,"context_lines":[{"line_number":401,"context_line":"        # For correct operation, this test relies on the DB being"},{"line_number":402,"context_line":"        # pre-populated by a properly complete backup."},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"        bak0 \u003d self._create_backup_db_entry(status\u003dNone)"},{"line_number":405,"context_line":"        bak1 \u003d self._create_backup_db_entry("},{"line_number":406,"context_line":"            status\u003dfields.BackupStatus.CREATING)"},{"line_number":407,"context_line":"        bak2 \u003d self._create_backup_db_entry("},{"line_number":408,"context_line":"            status\u003dfields.BackupStatus.DELETING)"},{"line_number":409,"context_line":"        bak3 \u003d self._create_backup_db_entry("},{"line_number":410,"context_line":"            status\u003dfields.BackupStatus.RESTORING)"},{"line_number":411,"context_line":"        bak4 \u003d self._create_backup_db_entry("},{"line_number":412,"context_line":"            status\u003dfields.BackupStatus.ERROR)"},{"line_number":413,"context_line":"        bak5 \u003d self._create_backup_db_entry("},{"line_number":414,"context_line":"            status\u003dfields.BackupStatus.ERROR_DELETING)"},{"line_number":415,"context_line":"        bak6 \u003d self._create_backup_db_entry("},{"line_number":416,"context_line":"            status\u003dfields.BackupStatus.DELETED)"},{"line_number":417,"context_line":"        fake_incomplete_backup_list \u003d ["},{"line_number":418,"context_line":"            bak0, bak1, bak2, bak3, bak4, bak5, bak6]"},{"line_number":419,"context_line":""},{"line_number":420,"context_line":"        self._create_backup_db_entry(status\u003dfields.BackupStatus.AVAILABLE)"},{"line_number":421,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"bc02a105_a8ef1f2c","line":418,"range":{"start_line":404,"start_character":1,"end_line":418,"end_character":53},"in_reply_to":"491ee2e2_9ba720e2","updated":"2024-02-16 00:21:53.000000000","message":"Done","commit_id":"52d35f93aa3acb26e86f3467840354ad99ea1711"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"fad47169308f9143b6911bd1819d965c61e61f71","unresolved":false,"context_lines":[{"line_number":454,"context_line":"        bak6 \u003d self._create_backup_db_entry("},{"line_number":455,"context_line":"            status\u003dfields.BackupStatus.DELETED)"},{"line_number":456,"context_line":"        fake_backup_list \u003d [bak1, bak2, bak3, bak4, bak5, bak6]"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"        self._create_backup_db_entry(status\u003dfields.BackupStatus.AVAILABLE)"},{"line_number":459,"context_line":""},{"line_number":460,"context_line":"        mock_backup_cleanup \u003d self.mock_object("}],"source_content_type":"text/x-python","patch_set":16,"id":"73f578dc_d20353be","line":457,"updated":"2024-02-16 00:21:53.000000000","message":"Switched to a list comprehension here too.","commit_id":"52d35f93aa3acb26e86f3467840354ad99ea1711"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"246da877bd0adffd3a9fb37c6821c9833f7aa7cc","unresolved":true,"context_lines":[{"line_number":428,"context_line":"        for b in fake_incomplete_backup_list:"},{"line_number":429,"context_line":"            mock_backup_cleanup.assert_any_call(self.ctxt, b)"},{"line_number":430,"context_line":""},{"line_number":431,"context_line":"        self.assertEqual(len(fake_incomplete_backup_list),"},{"line_number":432,"context_line":"                         mock_temp_cleanup.call_count)"},{"line_number":433,"context_line":"        for b in fake_incomplete_backup_list:"},{"line_number":434,"context_line":"            mock_temp_cleanup.assert_any_call(self.ctxt, b)"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"    def test_cleanup_incomplete_backup_operations_with_exceptions(self):"},{"line_number":437,"context_line":"        \"\"\"Test cleanup resilience in the face of exceptions.\"\"\""}],"source_content_type":"text/x-python","patch_set":17,"id":"e7fba145_de1fa6e0","line":434,"range":{"start_line":431,"start_character":1,"end_line":434,"end_character":59},"updated":"2024-04-12 15:06:02.000000000","message":"-1: This is duplicated code from L426-L429","commit_id":"b52eb917b4150e68e1a3d83b2374d017e7addaa9"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"71aa3bf6acf40ca217fb75d347bbb5fc942045a0","unresolved":true,"context_lines":[{"line_number":428,"context_line":"        for b in fake_incomplete_backup_list:"},{"line_number":429,"context_line":"            mock_backup_cleanup.assert_any_call(self.ctxt, b)"},{"line_number":430,"context_line":""},{"line_number":431,"context_line":"        self.assertEqual(len(fake_incomplete_backup_list),"},{"line_number":432,"context_line":"                         mock_temp_cleanup.call_count)"},{"line_number":433,"context_line":"        for b in fake_incomplete_backup_list:"},{"line_number":434,"context_line":"            mock_temp_cleanup.assert_any_call(self.ctxt, b)"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"    def test_cleanup_incomplete_backup_operations_with_exceptions(self):"},{"line_number":437,"context_line":"        \"\"\"Test cleanup resilience in the face of exceptions.\"\"\""}],"source_content_type":"text/x-python","patch_set":17,"id":"9f794525_dac3ee5f","line":434,"range":{"start_line":431,"start_character":1,"end_line":434,"end_character":59},"in_reply_to":"6427f38b_7cac983c","updated":"2024-04-19 22:03:44.000000000","message":"The boilerplate looks pretty straightforward to me. I thought that factoring it out into a helper function would be a cure that\u0027s worse than the disease, so I left it like this.","commit_id":"b52eb917b4150e68e1a3d83b2374d017e7addaa9"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5a2f9c287c065b9c26f4fdca796151e8c78715f3","unresolved":false,"context_lines":[{"line_number":428,"context_line":"        for b in fake_incomplete_backup_list:"},{"line_number":429,"context_line":"            mock_backup_cleanup.assert_any_call(self.ctxt, b)"},{"line_number":430,"context_line":""},{"line_number":431,"context_line":"        self.assertEqual(len(fake_incomplete_backup_list),"},{"line_number":432,"context_line":"                         mock_temp_cleanup.call_count)"},{"line_number":433,"context_line":"        for b in fake_incomplete_backup_list:"},{"line_number":434,"context_line":"            mock_temp_cleanup.assert_any_call(self.ctxt, b)"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"    def test_cleanup_incomplete_backup_operations_with_exceptions(self):"},{"line_number":437,"context_line":"        \"\"\"Test cleanup resilience in the face of exceptions.\"\"\""}],"source_content_type":"text/x-python","patch_set":17,"id":"e095e542_5e4de56c","line":434,"range":{"start_line":431,"start_character":1,"end_line":434,"end_character":59},"in_reply_to":"9f794525_dac3ee5f","updated":"2024-05-17 15:30:33.000000000","message":"You are right, I missed that those were 2 different mocks (even though I read it a couple of times).","commit_id":"b52eb917b4150e68e1a3d83b2374d017e7addaa9"},{"author":{"_account_id":34149,"name":"Niklas Schwarz","email":"niklas.schwarz@inovex.de","username":"nschwarz"},"change_message_id":"16d11d074719802cb985d29e28ce0c6111894a45","unresolved":true,"context_lines":[{"line_number":428,"context_line":"        for b in fake_incomplete_backup_list:"},{"line_number":429,"context_line":"            mock_backup_cleanup.assert_any_call(self.ctxt, b)"},{"line_number":430,"context_line":""},{"line_number":431,"context_line":"        self.assertEqual(len(fake_incomplete_backup_list),"},{"line_number":432,"context_line":"                         mock_temp_cleanup.call_count)"},{"line_number":433,"context_line":"        for b in fake_incomplete_backup_list:"},{"line_number":434,"context_line":"            mock_temp_cleanup.assert_any_call(self.ctxt, b)"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"    def test_cleanup_incomplete_backup_operations_with_exceptions(self):"},{"line_number":437,"context_line":"        \"\"\"Test cleanup resilience in the face of exceptions.\"\"\""}],"source_content_type":"text/x-python","patch_set":17,"id":"6427f38b_7cac983c","line":434,"range":{"start_line":431,"start_character":1,"end_line":434,"end_character":59},"in_reply_to":"e7fba145_de1fa6e0","updated":"2024-04-16 10:47:01.000000000","message":"It is not quiet duplicate code. The first for-loop asserts on \u0027mock_backup_cleanup\u0027, the second loop asserts on the \u0027mock_temp_cleanup\u0027. Looks similar.\n\nBut the code can be shorten indeed. First both asserEquals and then the for loop with both checks on the call","commit_id":"b52eb917b4150e68e1a3d83b2374d017e7addaa9"}]}