)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"30dd30e10b3239385edd7854ea9a1a2e84c4cf75","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Added filters while fetching backups from DB for incremental"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In create method of backup, while doing an incremental backup all"},{"line_number":10,"context_line":"existing backup entries gets fetched from DB to search for the one"},{"line_number":11,"context_line":"to use in incremental."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Now, only required one would be fetched after adding sort_keys\u003d\u0027created_at\u0027"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"1f1a1f67_3b76b671","line":10,"range":{"start_line":10,"start_character":24,"end_line":10,"end_character":28},"updated":"2017-07-19 13:49:44.000000000","message":"nit: get","commit_id":"6e110722ac4c60ee8fe8041f5e399c2a8c5df2d5"},{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"20861febe41c154f09faeb2ab266c6564d510c56","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Added filters while fetching backups from DB for incremental"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In create method of backup, while doing an incremental backup all"},{"line_number":10,"context_line":"existing backup entries gets fetched from DB to search for the one"},{"line_number":11,"context_line":"to use in incremental."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Now, only required one would be fetched after adding sort_keys\u003d\u0027created_at\u0027"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ff346bd7_d57d07e8","line":10,"range":{"start_line":10,"start_character":24,"end_line":10,"end_character":28},"in_reply_to":"1f1a1f67_3b76b671","updated":"2017-07-24 11:51:16.000000000","message":"Done","commit_id":"6e110722ac4c60ee8fe8041f5e399c2a8c5df2d5"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5792f024873a6bdc34163d9f9264b15b1e8e81ac","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Optimize getting parent backup for new incremental"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When creating an incremental backup all existing backup entries where fetched"},{"line_number":10,"context_line":"from the DB to then search this potentially large list for the one to use as"},{"line_number":11,"context_line":"parent."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":16,"id":"214cfc56_c44cfe63","line":9,"range":{"start_line":9,"start_character":64,"end_line":9,"end_character":69},"updated":"2024-04-12 14:58:04.000000000","message":"were","commit_id":"b4e46ba5745ebefd3d2092686069e412eac222fa"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"a0a1321ac82e4c33624687340a0b6be397777d75","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Optimize getting parent backup for new incremental"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When creating an incremental backup all existing backup entries where fetched"},{"line_number":10,"context_line":"from the DB to then search this potentially large list for the one to use as"},{"line_number":11,"context_line":"parent."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":16,"id":"e9fcde9b_d6a0adad","line":9,"range":{"start_line":9,"start_character":64,"end_line":9,"end_character":69},"in_reply_to":"214cfc56_c44cfe63","updated":"2026-04-07 14:19:50.000000000","message":"Done","commit_id":"b4e46ba5745ebefd3d2092686069e412eac222fa"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"092122d1194fc0968c78bdc6b447b9997a86e106","unresolved":true,"context_lines":[{"line_number":10,"context_line":"from the DB to then search this potentially large list for the one to use as"},{"line_number":11,"context_line":"parent. This change introduces a new method utilizing a single DB query to"},{"line_number":12,"context_line":"find the parent (if one exists). To improve performance even more, two"},{"line_number":13,"context_line":"database indices where added."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Alongside, a previous bug with finding parents for snapshot backups"},{"line_number":16,"context_line":"was fixed. This caused to sometimes return an actually ineligible parent."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":36,"id":"54aac1a4_bf1fada5","line":13,"range":{"start_line":13,"start_character":17,"end_line":13,"end_character":23},"updated":"2026-08-18 15:27:51.000000000","message":"nit: were\n\nAlso, as per the inline, only 1 index appears so one is wrong...","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"7351beb8b1a9afb1f47f7dbbc6a3892f326c06e3","unresolved":false,"context_lines":[{"line_number":10,"context_line":"from the DB to then search this potentially large list for the one to use as"},{"line_number":11,"context_line":"parent. This change introduces a new method utilizing a single DB query to"},{"line_number":12,"context_line":"find the parent (if one exists). To improve performance even more, two"},{"line_number":13,"context_line":"database indices where added."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Alongside, a previous bug with finding parents for snapshot backups"},{"line_number":16,"context_line":"was fixed. This caused to sometimes return an actually ineligible parent."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":36,"id":"58fd3f7c_3db4d7ba","line":13,"range":{"start_line":13,"start_character":17,"end_line":13,"end_character":23},"in_reply_to":"54aac1a4_bf1fada5","updated":"2026-09-02 07:49:40.000000000","message":"Done","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"092122d1194fc0968c78bdc6b447b9997a86e106","unresolved":true,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Alongside, a previous bug with finding parents for snapshot backups"},{"line_number":16,"context_line":"was fixed. This caused to sometimes return an actually ineligible parent."},{"line_number":17,"context_line":"This case properly trigger an error now."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Additionally the backup status of `RESTORING` was added to the list of"},{"line_number":20,"context_line":"valid status, as a restoring backup is indeed a valid, read-only source for a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":36,"id":"1956bd76_881f5bd3","line":17,"range":{"start_line":17,"start_character":0,"end_line":17,"end_character":40},"updated":"2026-08-18 15:27:51.000000000","message":"grammar: This case now raises an error.","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"7351beb8b1a9afb1f47f7dbbc6a3892f326c06e3","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Alongside, a previous bug with finding parents for snapshot backups"},{"line_number":16,"context_line":"was fixed. This caused to sometimes return an actually ineligible parent."},{"line_number":17,"context_line":"This case properly trigger an error now."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Additionally the backup status of `RESTORING` was added to the list of"},{"line_number":20,"context_line":"valid status, as a restoring backup is indeed a valid, read-only source for a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":36,"id":"e47ca6fa_c2df0f60","line":17,"range":{"start_line":17,"start_character":0,"end_line":17,"end_character":40},"in_reply_to":"1956bd76_881f5bd3","updated":"2026-09-02 07:49:40.000000000","message":"Done","commit_id":"5e843ea8282137390ac7947c933da256419353b2"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"64cca988739e2a7baabdb14c940b49463e227424","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":7,"id":"baffa285_ea6a10d8","updated":"2024-01-29 13:17:09.000000000","message":"I now looked at this one in a little more detail when rebasing\n... and there are multiple caveats and pitfalls:\n\n1) Only fetching the \"most recent\" (and maybe filtered here with \"status\u003dfields.BackupStatus.AVAILABLE\") backups for a volume would indeed be a great improvement as it moves the heavy lifting to the DB.\n\n2) But to also support incremental backups of volume snapshots (!) the response would be in correct / NOT sufficient. As Eric commented [1], the max function seems a little redundant at first, but it\u0027s not! If an incremental backup of a snapshot is taken, finding the parent is a little more complicated, than just using the last successful volume backup:\n\nThe list of backups needs to be searched for\n\n  * Case1: if it\u0027s not backing up a snapshot, simply the most recent (successful) backup. Easy, see 1).\n\n  * Case2: if it\u0027s incrementally backing up a snapshot, most recent backup that, is older than the snapshot itself (if it was younger, than changed blocks would be missing)\n\n\nI rebased the code and reworked the filters to determine the proper parent backup_id all within the DB query. I know and see the tests are still broken, but please kindly take a look and give me some feedback on this idea to clean and speed things up.\n\n\n\n\n[1] https://review.opendev.org/c/openstack/cinder/+/484729/comment/ffb9cba7_6d6526e1/","commit_id":"6048217d70342ca5c129535a355e7f0effe36e80"},{"author":{"_account_id":31779,"name":"Jean Pierre Roquesalane","display_name":"happystacker","email":"jeanpierre.roquesalane@dell.com","username":"happystacker"},"change_message_id":"b6c0670365a02a44c5b049f007239cbc60e8bf59","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"356d369b_1a2c0af4","updated":"2024-01-26 11:14:19.000000000","message":"Please resolve conflict and resubmit a new patchset","commit_id":"6048217d70342ca5c129535a355e7f0effe36e80"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"0ce73476d895f7735d7fc062d6d8fefae400d8e7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"fe685dda_7fff7abf","in_reply_to":"baffa285_ea6a10d8","updated":"2026-02-16 11:28:44.000000000","message":"Done","commit_id":"6048217d70342ca5c129535a355e7f0effe36e80"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"0c7f516925a5a6591199b19d4cfec839b7a4f356","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":11,"id":"b0db7205_bb379a2f","updated":"2024-02-07 15:07:16.000000000","message":"I am looking for guidance on how to best add the DB query to find the parent backup and then \"reach\" it or call within the backup.\n\n\nMy current approach is to add \"backup_get_parent_for_incremental\" to db (SQLAlchemy) to do the actual query.\n\n\n\nNow where I\u0027d like some input:\n\n\n1) Should I directly call the backup_get_parent_for_incremental from the \"db\" (and then convert it into a backup object cinder.backup.api.create()? \n\n2) Should I add a method to objects.BackupList or objects.Backup to abstract the DB access away and get a Backup object (which is required as \"parent\" parameter for the to be created backup anyways?","commit_id":"56ebbfb5fabde2e330f11d16770efd2b6ed5aea7"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"054fcd1e4d349fa25f716f1a265b86a4afd52e47","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"708a97d4_2d107e2a","updated":"2024-02-07 14:39:28.000000000","message":"It\u0027s unclear how openstack-tox-pep8 passes on this patch -- \"tox -e pep8\" fails with a dozen errors when run locally.","commit_id":"56ebbfb5fabde2e330f11d16770efd2b6ed5aea7"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"1a0b437028a0d26dc0f5ef1d7893c89ae7299ec3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"d12e7b30_771ddec1","in_reply_to":"5e59972d_d7b236db","updated":"2026-07-20 07:47:05.000000000","message":"Done","commit_id":"56ebbfb5fabde2e330f11d16770efd2b6ed5aea7"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"3c53e7a5b19af3ecee507a49d85e1ddab903344b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":11,"id":"5e59972d_d7b236db","in_reply_to":"b0db7205_bb379a2f","updated":"2024-02-07 17:00:24.000000000","message":"I pushed a somewhat complete version now with 2) implemented and (hopefully) no more pep8 or failing tests.\n\nPlease let me know what you think.","commit_id":"56ebbfb5fabde2e330f11d16770efd2b6ed5aea7"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"35e53c2f6a4d11c29df487b78c294615e43e9e2e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"10823397_630952b6","updated":"2024-02-08 08:56:17.000000000","message":"@Michal I added you since you seems to be a heavy user of incremental backups as you replied to the change about keeping only a few snapshots (https://review.opendev.org/c/openstack/cinder/+/810457/comments/19fa14ba_e49102ad).\n\nMaybe you find the time to look into this change here as well?","commit_id":"f7d39a1f495b36d83c7d4b8bddb6bfba569ab5f4"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"cb75610086c36c949e9cefa8c8cabc2827e8fbf9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"f51ee4b6_090a921f","updated":"2024-02-13 06:13:40.000000000","message":"Love it.\n\nI wish I could create my own method all_non_avail() or something. But that one only runs at startup, but this one runs all the time.\n\nThe way comparison with timestamp is programmed down to SQL looks good to me.","commit_id":"440bade7a3912a62d89173f35d4098cba3bc7f43"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"b86114c4544fb2104ca6ae562cac1b5e445d6979","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"12cb3f60_b8aacf5f","updated":"2024-02-13 09:04:41.000000000","message":"recheck","commit_id":"440bade7a3912a62d89173f35d4098cba3bc7f43"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"ae0e127a8477a5489dda54af335bbc754ea22bb3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"73593648_a53e338c","updated":"2024-02-14 13:46:27.000000000","message":"There were some functional tests failing. I found that I did not consider the fix from https://review.opendev.org/c/openstack/cinder/+/720833, which I how (hopefully) did with the latest patchset.\n\nPTAL.","commit_id":"b501817376f000b814d5d4dea38b47d48ea18fba"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"6d662b0840cda9c4a2856ba2010687b32a4e7d3e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"0abefa2d_40cda333","updated":"2024-03-04 12:15:05.000000000","message":"Could you kindly take another look at this one?","commit_id":"b4e46ba5745ebefd3d2092686069e412eac222fa"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"33f014faa01c9a4790736c55aa0cbab9077889f3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"ecb1f16a_ad6fee0a","updated":"2024-02-19 15:14:55.000000000","message":"I had to do one more fix as \u0027project_only\u003dTrue\u0027 for _backups_get_query still let\u0027s the admin see it all :-)\n\n\nPlease kindly take another look at this one.","commit_id":"b4e46ba5745ebefd3d2092686069e412eac222fa"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"7311e2ddfb75c3da02744c558d70ee976c114b52","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"2a5235ca_0d2a58bc","updated":"2024-04-09 07:28:04.000000000","message":"May I kindly ask for your review again?","commit_id":"b4e46ba5745ebefd3d2092686069e412eac222fa"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"6f3439db4b3e9e7ac9e3dd152fc59ea004b0b0f3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"da12c600_428fc582","updated":"2024-03-20 13:56:23.000000000","message":"Pete, could you kindly check this one out again?","commit_id":"b4e46ba5745ebefd3d2092686069e412eac222fa"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"1aecd6bd6100065be3fea37fac5052eeff0f525c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"ea549478_c0864cd7","updated":"2024-03-04 16:08:27.000000000","message":"Peter please see me inline comments and kindly tell me if I got anything wrong here.","commit_id":"b4e46ba5745ebefd3d2092686069e412eac222fa"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"d8399b3d2296a4d6acef7656ac5cdae9589ca9e4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"691ca168_0da083f6","updated":"2024-03-04 15:03:23.000000000","message":"See inline, but basically please explain how this came about.","commit_id":"b4e46ba5745ebefd3d2092686069e412eac222fa"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5792f024873a6bdc34163d9f9264b15b1e8e81ac","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"388eb730_705dec11","updated":"2024-04-12 14:58:04.000000000","message":"requires UT for sqlalchemy method","commit_id":"3d9b6bf0846531e190c0f4f8758cdf62686e27db"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a59530b986328cd86eb633d57536c7a5fc08f09f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"1a9459c4_779fa9a6","updated":"2024-04-12 15:00:39.000000000","message":"-1: For the missing UTs, here are some samples of how they look like: cinder/tests/unit/test_db_api.py","commit_id":"ff403a1b9119dce47bf4dfcfa6882be2d7d8db80"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"9dfdd4fd568d9d22f7faa7dcf4ddcdf8294aabfe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"7aaf2410_0264051e","updated":"2024-04-16 08:20:55.000000000","message":"As discussed at the PTG on Friday, I pushed a new Patchset including.UTs. PTAL\n\nI also fixed \n\"cinder.tests.unit.api.contrib.test_backups.BackupsAPITestCase.test_create_backup_delta_2_True\".\nThat should actually also have failed with the old code (wrong order to create the full backup and the snapshot, resulting in the data_timestamp of the full to be too recent new to base an incremental snapshot backup on.","commit_id":"f34bcabd4e6e6155e06e3d6e357263269fed6fbc"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"4d9fd9d57fb6c1324c50dda1392db6614600c9e4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"33b3d39a_18a742ed","updated":"2024-05-17 15:37:11.000000000","message":"Regarding the failure of openstacksdk-functional-devstack. I\u0027m pretty sure Christian narrowed it down correctly to the timestamps. However there may be a mismatch between what the large block comment by xyang says and what the code does. The existing code only sorts the backups and finds the least worst one with max(). Therefore, a backup that fails the timestamp checks still gets selected in the end.\n\nThis behavior needs to be preserved. What the test is doing should continue to work. We constructed even more absurd scenarios during the upstream patch review meeting today, which should still work.\n\nThe consensus suggestion, then, is to run the optimized query as proposed in this patch. But if that fails, run a wider one, without the timestamps in WHERE statement (in filters), and pick up least worst backup from the result, to be the parent of the incremental.","commit_id":"f34bcabd4e6e6155e06e3d6e357263269fed6fbc"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"7a121c3b4050d613894b1b2caa5914d96bf354b1","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":19,"id":"b6da8f16_5c9481e3","updated":"2024-04-18 13:55:29.000000000","message":"There currently is test \"openstack.tests.functional.cloud.test_project_cleanup.TestProjectCleanup.test_block_storage_cleanup\" failing, see https://zuul.opendev.org/t/openstack/build/4f9ea8e583fa4b8495298367091f9f71.\n\nIf you look at what happens at https://github.com/openstack/openstacksdk/blob/8c6a129b8cea97ae8534c921576cac4b347a6194/openstack/tests/functional/cloud/test_project_cleanup.py#L132 onwards you see that:\n\n1. A volume is created\n2. A snapshot of that volume is created\n3. A full backup of the volume is created\n4. A incremental backup of the snapshot (created in step 2) is created, which fails.\n\n\nComparing the former and this new implementation I believe the code was buggy:\n\nOne cannot create an incremental snapshot backup, if there is no full backup (of the volume OR the snapshot) that has a \u0027data_timestamp\u0027 before the one of the snapshot.\n\n\nCould anybody kindly try and understand the possible cases here so I know if\n\na) My code is borken or where\nb) OpenstackSDK needs to create some different resources to \"project cleanup\"","commit_id":"f34bcabd4e6e6155e06e3d6e357263269fed6fbc"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"8abddc1bbdda6a980aae045a6819e287ec271bff","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"79cbe663_1a605a0d","updated":"2024-04-19 15:38:47.000000000","message":"This looks not worse than before to me. I also found the part that I was missing about the context\u0027s project_id.","commit_id":"f34bcabd4e6e6155e06e3d6e357263269fed6fbc"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"da1123dc2a2a7ee863414f345e46d06b213d3475","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":19,"id":"b7020ba1_14700c21","updated":"2024-04-29 06:05:08.000000000","message":"Tobias, since you worked on this part of the code as well and seems to be a cinder-backup user, do you mind diving into my question at:\n\nhttps://review.opendev.org/c/openstack/cinder/+/484729/comments/b6da8f16_5c9481e3\n\n\nI\u0027d really love for this to be resolved quickly so we can get this change here merged.","commit_id":"f34bcabd4e6e6155e06e3d6e357263269fed6fbc"},{"author":{"_account_id":16137,"name":"Tobias Urdin","email":"tobias.urdin@binero.com","username":"tobasco"},"change_message_id":"dff23c38ac0573b2a345e3fd5be34073ffb8597b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"f10edbf8_cda01d0a","updated":"2024-04-24 11:50:39.000000000","message":"gah wish i saw this before refactoring the same code in my series https://review.opendev.org/c/openstack/cinder/+/916683 – either way I\u0027ll change that to depend on this one","commit_id":"f34bcabd4e6e6155e06e3d6e357263269fed6fbc"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"1570f9fc55f7a52590765d92b3ed115b5df1c5ed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"04a6a0d1_048d2915","updated":"2024-04-16 11:41:40.000000000","message":"recheck tempest-integrated-storage openstacksdk-functional-devstack","commit_id":"f34bcabd4e6e6155e06e3d6e357263269fed6fbc"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"6eda3226813ada3550e8c1c1e32cde5759752488","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"e92b8e28_d3ec11cf","updated":"2024-04-17 05:06:13.000000000","message":"recheck tempest-integrated-storage openstacksdk-functional-devstack temptest-slow-py3","commit_id":"f34bcabd4e6e6155e06e3d6e357263269fed6fbc"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"1a0b437028a0d26dc0f5ef1d7893c89ae7299ec3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"343ac355_ff385966","in_reply_to":"95096300_6702df65","updated":"2026-07-20 07:47:05.000000000","message":"Done","commit_id":"f34bcabd4e6e6155e06e3d6e357263269fed6fbc"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"cd048050524fab36161160b5d1e9e8ec76ce91ab","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":19,"id":"95096300_6702df65","in_reply_to":"b6da8f16_5c9481e3","updated":"2024-05-17 14:48:25.000000000","message":"It may not seem logical to do that operation, but the code worked just fine and the restored data would be consistent with what was backed up.\n\nThe only thing that would happen is that you would be using more data.\n\nSo if that specific operation stops working then it would be a regression (because something that used to work now doesn\u0027t) even if it was a \"weird feature/behavior\".","commit_id":"f34bcabd4e6e6155e06e3d6e357263269fed6fbc"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"1a0b437028a0d26dc0f5ef1d7893c89ae7299ec3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"c295e136_1437375c","in_reply_to":"b7020ba1_14700c21","updated":"2026-07-20 07:47:05.000000000","message":"Done","commit_id":"f34bcabd4e6e6155e06e3d6e357263269fed6fbc"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"6fb827bd12ce33662bf65928e7e999c8cea72ddc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"fee2c923_908c0b81","updated":"2025-07-11 09:37:45.000000000","message":"Could this have its final review and merge?","commit_id":"12afc25347fdbb5358e1d764cf5a9c8f14d0dca8"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"b72903f44a48cbde19f2ece6e34ffd62c658f255","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"c8020077_7db4a868","updated":"2025-07-16 18:13:49.000000000","message":"Need to check why Zuul is failing due to missing pytz module","commit_id":"12afc25347fdbb5358e1d764cf5a9c8f14d0dca8"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"9c2470f7fdb140c5b5ef62e30f47970b8c702ff3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"28d0fb82_516ed537","updated":"2025-08-21 07:31:42.000000000","message":"Is there anything more I can do to help getting this across the finish line?","commit_id":"b9c991aca2f37326335ea1be8e39d0b8e806c81c"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"7c29824da347dda9d6881d0e3d9024c7556051bd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"eef48a41_bc542403","updated":"2025-08-07 18:56:42.000000000","message":"Patch rebased on latest master and only pytz reference updated to builtin zoneinfo.","commit_id":"b9c991aca2f37326335ea1be8e39d0b8e806c81c"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"9cb9259e0ea8be2673d1b1d4375566b00d2cc9f9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"6c7b3e6f_d3a4e50e","updated":"2025-08-07 18:55:47.000000000","message":"pytz was deprecated in favor of builtin zoneinfo, see https://review.opendev.org/c/openstack/requirements/+/875854 for more information.","commit_id":"b9c991aca2f37326335ea1be8e39d0b8e806c81c"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"d104d4ea313715cc4a18e031368b1cd5cb883156","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"e639a96d_3469482c","updated":"2025-08-08 07:42:00.000000000","message":"recheck cinder-plugin-ceph-tempest","commit_id":"b9c991aca2f37326335ea1be8e39d0b8e806c81c"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"23b973da49d12ebc3a1e5157555fe73f38c6833a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"6966e4b0_e534c657","updated":"2026-02-16 21:00:22.000000000","message":"recheck","commit_id":"ed665250e60d8ca870317e2181c2a26617564414"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"4325d676421f2010ad111c82b8ed4c365d19322b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":27,"id":"20baaf24_809593d1","updated":"2026-02-20 15:14:58.000000000","message":"I continue to think this patch is valuable. LGTM.","commit_id":"48ee92caf36874dc4f9c414ce824e62158026769"},{"author":{"_account_id":35075,"name":"Alexander Deiter","email":"adeiter@infinidat.com","username":"adeiter"},"change_message_id":"2d3d1960fde02a83add62e89033b78292d43a53a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":27,"id":"b76df9a9_77193085","updated":"2026-02-20 15:40:30.000000000","message":"Looks good to me - thank you!","commit_id":"48ee92caf36874dc4f9c414ce824e62158026769"},{"author":{"_account_id":36171,"name":"jayaanand borra","display_name":"jayaanand borra","email":"jayaanand.borra@netapp.com","username":"jayaanan","status":"netapp"},"change_message_id":"3ad95e0ffff9ee10b345d119132cb845ec8f7cbc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":29,"id":"b88162c8_15081af9","updated":"2026-04-07 08:41:30.000000000","message":"Also, now backup scope is increased to RESTORING.I agree with other comments.","commit_id":"29594932bee2ef312d9151770bc615221ca41196"},{"author":{"_account_id":36171,"name":"jayaanand borra","display_name":"jayaanand borra","email":"jayaanand.borra@netapp.com","username":"jayaanan","status":"netapp"},"change_message_id":"aade4af9eb6e49742f7924c7e77044c0316aeaf8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":29,"id":"d3ae5c38_7983db36","updated":"2026-04-07 08:40:04.000000000","message":"from __future__ import annotations changes all annotations in the file to be lazily evaluated strings, which can break code that inspects annotations at runtime?","commit_id":"29594932bee2ef312d9151770bc615221ca41196"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"cedec1880ee65bf927776e9bb504c0824b964674","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":29,"id":"b617793e_bdeb2bec","updated":"2026-04-02 06:57:22.000000000","message":"recheck openstacksdk-functional-devstack","commit_id":"29594932bee2ef312d9151770bc615221ca41196"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"a0a1321ac82e4c33624687340a0b6be397777d75","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":29,"id":"64e637b8_358a3ee4","in_reply_to":"b88162c8_15081af9","updated":"2026-04-07 14:19:50.000000000","message":"Yes. A backup in RESTORING state is absolutely still a valid (read: \"THE\") valid parent. Not including this status would cause the parent of new backup to move one up which is still working, but not really correct.","commit_id":"29594932bee2ef312d9151770bc615221ca41196"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"7f1c4ac8f96bf73d44dad97df01ca29fe57da850","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":30,"id":"206ce926_5f0a3fbf","updated":"2026-04-28 14:48:51.000000000","message":"I believe the failing CI for openstacksdk might actually a bug.\nKindly have a look at https://bugs.launchpad.net/openstacksdk/+bug/2148715","commit_id":"959af97e1ca40126c7bb7b64c3c79b872f9cb3f1"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"1f922bbf6373e6c84c1860c16528fb6a510609ad","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"c9c9d954_8d8ccf8a","updated":"2026-07-13 18:38:45.000000000","message":"Nice optimization — replacing load-all-backups + in-memory max() with a single ordered DB query is the right fix for bug 1696715, and the object/db/api layering is clean. A few items below before +1: one undocumented behavior change (RESTORING now an eligible parent) and some test gaps.","commit_id":"7b89aba74d5f17a0470cc3847bd366201c618533"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"448f499bb292ccf30aaad8522491d249a7019b2d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":32,"id":"ede8b9d3_60d88fd4","updated":"2026-07-16 13:27:21.000000000","message":"Thanks for your time and thorough review Simon!\nPTAL at the new patchset containing the requested changes.\n\nKindly also please have a look at https://bugs.launchpad.net/openstacksdk/+bug/2148715 about the currently failing integration tests with OpenStackSDK.\nThis needs to be resolved somehow, otherwise the test still fails for this change here.","commit_id":"7b89aba74d5f17a0470cc3847bd366201c618533"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"3d2253b41ec235ce2189ae52733526925e29f671","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"0f5bc2ea_2c83a4c7","updated":"2026-07-13 18:29:45.000000000","message":"recheck","commit_id":"7b89aba74d5f17a0470cc3847bd366201c618533"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"0348a1bc6465fb81080c765d8fd44fabcc861e0b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"d62a76fd_80530261","in_reply_to":"ede8b9d3_60d88fd4","updated":"2026-07-20 07:46:25.000000000","message":"https://review.opendev.org/c/openstack/openstacksdk/+/997562","commit_id":"7b89aba74d5f17a0470cc3847bd366201c618533"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"551cef1c4e43e33925ae87f055541cf2af7380d8","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":33,"id":"47744519_070edb18","updated":"2026-08-07 08:45:25.000000000","message":"Simon, it seems like openstacksdk-functional-devstack is still failing:\n\n--------\n\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\nFailed 1 tests - output below:\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n\nopenstack.tests.functional.cloud.test_project_cleanup.TestProjectCleanup.test_block_storage_cleanup\n---------------------------------------------------------------------------------------------------\n\nCaptured traceback:\n~~~~~~~~~~~~~~~~~~~\n    Traceback (most recent call last):\n\n      File \"/home/zuul/src/opendev.org/openstack/openstacksdk/openstack/tests/functional/cloud/test_project_cleanup.py\", line 218, in test_block_storage_cleanup\n    self.assertEqual(\n\n      File \"/home/zuul/src/opendev.org/openstack/openstacksdk/openstack/tests/base.py\", line 102, in assertEqual\n    return super().assertEqual(expected, observed, message)\n           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n      File \"/home/zuul/src/opendev.org/openstack/openstacksdk/.tox/functional/lib/python3.12/site-packages/testtools/testcase.py\", line 513, in assertEqual\n    self.assertThat(observed, matcher, message)\n\n      File \"/home/zuul/src/opendev.org/openstack/openstacksdk/.tox/functional/lib/python3.12/site-packages/testtools/testcase.py\", line 704, in assertThat\n    raise mismatch_error\n\n    testtools.matchers._impl.MismatchError: 0 !\u003d 1\n\n--------\n\n\n\nSo was your fix https://review.opendev.org/c/openstack/openstacksdk/+/997562 for bug https://bugs.launchpad.net/openstacksdk/+bug/2148715 not sufficient or do we hit another ?","commit_id":"97d48fd8b1970585cf6ada25926d5e7295731ebb"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"69d1e1de9884270e3d93aafacbceb69b3f029ac1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":33,"id":"2c822b3b_76f2537b","updated":"2026-08-06 12:27:09.000000000","message":"recheck","commit_id":"97d48fd8b1970585cf6ada25926d5e7295731ebb"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"95f69695c968df5e5938f8a694d4c5d9933fc902","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":33,"id":"57b8da1d_1690172c","updated":"2026-08-06 09:08:36.000000000","message":"recheck","commit_id":"97d48fd8b1970585cf6ada25926d5e7295731ebb"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"8d4646d9469dbd45e804ae1a9116a0e29a0498ae","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":33,"id":"420a8c53_62feee23","updated":"2026-07-16 17:17:51.000000000","message":"run Everpure CI","commit_id":"97d48fd8b1970585cf6ada25926d5e7295731ebb"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"4c4d2fa9f4d4144872e260a36eb300cce65c6782","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":33,"id":"7ec14ec7_27773955","updated":"2026-07-21 13:25:16.000000000","message":"run Everpure CI","commit_id":"97d48fd8b1970585cf6ada25926d5e7295731ebb"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"aa71698bff1b6b9c99e6dba1a2431a43397ac4c2","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":33,"id":"f9ba729d_5a252f90","in_reply_to":"47744519_070edb18","updated":"2026-08-07 13:48:47.000000000","message":"My patch is in and working - this is a different failure... it\u0027s a pre-existing Cinder race conditon. I\u0027ll work on a patch.","commit_id":"97d48fd8b1970585cf6ada25926d5e7295731ebb"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"2c4bd71021703f17f56bf54e1301647e8956edba","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":33,"id":"b238c917_72cf82c0","in_reply_to":"f9ba729d_5a252f90","updated":"2026-08-18 14:13:49.000000000","message":"awesome thanks! Could you like to the patch once it\u0027s pushed?\nI suppose we need to add another depends-on then to have this one here turn green at some point?","commit_id":"97d48fd8b1970585cf6ada25926d5e7295731ebb"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"2c4bd71021703f17f56bf54e1301647e8956edba","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":35,"id":"1f39dd80_5d236c31","updated":"2026-08-18 14:13:49.000000000","message":"Thanks Simon again for your time an devotion to this issue.\nI hope I just addressed all your remarks and findings in my recent PS.\n\n\nPTAL again.","commit_id":"5b0e05c6116516fd26dcecab3c39c53ff787786b"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"24e6442df670dd76c1b613d577297ca249768d09","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":35,"id":"67c7d2e7_43dbd2d6","updated":"2026-08-14 00:05:10.000000000","message":"The Depnds-On cleared the issue and that patch has now merged.\nStill outstanding comments from an earlier PS and some new ones here.","commit_id":"5b0e05c6116516fd26dcecab3c39c53ff787786b"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"25313a6b71a657c324e1aef9a089386c142fd07b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":35,"id":"6472ab0a_c43f5cc7","updated":"2026-08-07 15:45:08.000000000","message":"recheck","commit_id":"5b0e05c6116516fd26dcecab3c39c53ff787786b"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"7351beb8b1a9afb1f47f7dbbc6a3892f326c06e3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":36,"id":"3b0e044f_6f410a1c","updated":"2026-09-02 07:49:40.000000000","message":"sorry about the delay, I was on vacation.\nPTAL and see my questions.","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"6b35acc600a7eac0a22c378c4dccca5e50355b25","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":42,"id":"3dca147a_a5ae3f33","updated":"2026-09-04 15:29:46.000000000","message":"I just pushed a new PS, PTAL.","commit_id":"72fec9c71d666ed471ee9a0ae16a036e55f6cd0b"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"c34db7071d3d020715b1c3bc7387aa245a493189","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":42,"id":"1d9f6510_c0daff41","updated":"2026-09-03 12:52:51.000000000","message":"Unfortunately PS37 introduced a functional regression that PS36 did not have,\nand it\u0027s the reason the Pure iSCSI CI has been red on every patchset since.\nFull diagnosis in the comment on `cinder/db/api.py`. It\u0027s a one-line fix.\n\nTwo tests fail in `cinder_tempest_plugin.api.volume.admin.test_volume_backup`:\n\n  `test_backup_crossproject_admin_negative`       (expected BadRequest, got 202)\n  `test_incremental_backup_respective_parents`    (backup went to ERROR)\n\nwhile `test_backup_crossproject_user_negative` still passes — which is exactly\nthe fingerprint of the problem.\n\nWorth knowing: cinder\u0027s own check pipeline has no cinder-tempest-plugin job\n(it runs tempest-integrated-storage, which doesn\u0027t include these tests), so\nupstream Zuul cannot catch this. Only third-party CIs that run the plugin\nsuite with backup enabled will. StorPool has also been failing on PS39/41/42\nand I\u0027d expect it\u0027s the same two tests.","commit_id":"72fec9c71d666ed471ee9a0ae16a036e55f6cd0b"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"6b35acc600a7eac0a22c378c4dccca5e50355b25","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":42,"id":"dafd82d0_e0db8696","in_reply_to":"1d9f6510_c0daff41","updated":"2026-09-04 15:29:46.000000000","message":"Acknowledged","commit_id":"72fec9c71d666ed471ee9a0ae16a036e55f6cd0b"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"9d23875e53ddb80cf87cb7428539de6db0862dba","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":43,"id":"64545969_8d39fc57","updated":"2026-09-04 18:30:04.000000000","message":"Regression is fixed and Pure CI with c-bak tests pass.\nJust 2 nits, but they don\u0027t stop my +1","commit_id":"ed4a49cc5ca31c9c4045b184ce1825ab42e2d1b9"}],"cinder/backup/api.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"30dd30e10b3239385edd7854ea9a1a2e84c4cf75","unresolved":false,"context_lines":[{"line_number":249,"context_line":"        # incremental backup."},{"line_number":250,"context_line":"        latest_backup \u003d None"},{"line_number":251,"context_line":"        if incremental:"},{"line_number":252,"context_line":"            backups \u003d \\"},{"line_number":253,"context_line":"                objects.BackupList.get_all_by_volume(context.elevated(),"},{"line_number":254,"context_line":"                                                     volume_id, limit\u003d1,"},{"line_number":255,"context_line":"                                                     sort_keys\u003d[\u0027created_at\u0027],"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_1b1f7a08","line":252,"range":{"start_line":252,"start_character":22,"end_line":252,"end_character":23},"updated":"2017-07-19 13:49:44.000000000","message":"-1: we should be using a different way to split the line or parenthesis, \\ is not recommended\n\n            backups \u003d objects.BackupList.get_all_by_volume(\n                context.elevated(),\n                volume_id, limit\u003d1,\n                sort_keys\u003d[\u0027created_at\u0027],\n                sort_dirs\u003d[\u0027desc\u0027])\n\nor\n\n            backups \u003d (\n                objects.BackupList.get_all_by_volume(context.elevated(),\n                                                     volume_id, limit\u003d1,\n                                                     sort_keys\u003d[\u0027created_at\u0027],\n                                                     sort_dirs\u003d[\u0027desc\u0027]))","commit_id":"6e110722ac4c60ee8fe8041f5e399c2a8c5df2d5"},{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"20861febe41c154f09faeb2ab266c6564d510c56","unresolved":false,"context_lines":[{"line_number":249,"context_line":"        # incremental backup."},{"line_number":250,"context_line":"        latest_backup \u003d None"},{"line_number":251,"context_line":"        if incremental:"},{"line_number":252,"context_line":"            backups \u003d \\"},{"line_number":253,"context_line":"                objects.BackupList.get_all_by_volume(context.elevated(),"},{"line_number":254,"context_line":"                                                     volume_id, limit\u003d1,"},{"line_number":255,"context_line":"                                                     sort_keys\u003d[\u0027created_at\u0027],"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_756c1b36","line":252,"range":{"start_line":252,"start_character":22,"end_line":252,"end_character":23},"in_reply_to":"1f1a1f67_1b1f7a08","updated":"2017-07-24 11:51:16.000000000","message":"Done","commit_id":"6e110722ac4c60ee8fe8041f5e399c2a8c5df2d5"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"30dd30e10b3239385edd7854ea9a1a2e84c4cf75","unresolved":false,"context_lines":[{"line_number":253,"context_line":"                objects.BackupList.get_all_by_volume(context.elevated(),"},{"line_number":254,"context_line":"                                                     volume_id, limit\u003d1,"},{"line_number":255,"context_line":"                                                     sort_keys\u003d[\u0027created_at\u0027],"},{"line_number":256,"context_line":"                                                     sort_dirs\u003d[\u0027desc\u0027])"},{"line_number":257,"context_line":"            if backups.objects:"},{"line_number":258,"context_line":"                # NOTE(xyang): The \u0027data_timestamp\u0027 field records the time"},{"line_number":259,"context_line":"                # when the data on the volume was first saved. If it is"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_db70e242","line":256,"updated":"2017-07-19 13:49:44.000000000","message":"-1: Instead of adding new optional parameters to the method we should be using `get_all` method instead since we can filter by the volume_id there and already supports the parameters we want.","commit_id":"6e110722ac4c60ee8fe8041f5e399c2a8c5df2d5"},{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"20861febe41c154f09faeb2ab266c6564d510c56","unresolved":false,"context_lines":[{"line_number":253,"context_line":"                objects.BackupList.get_all_by_volume(context.elevated(),"},{"line_number":254,"context_line":"                                                     volume_id, limit\u003d1,"},{"line_number":255,"context_line":"                                                     sort_keys\u003d[\u0027created_at\u0027],"},{"line_number":256,"context_line":"                                                     sort_dirs\u003d[\u0027desc\u0027])"},{"line_number":257,"context_line":"            if backups.objects:"},{"line_number":258,"context_line":"                # NOTE(xyang): The \u0027data_timestamp\u0027 field records the time"},{"line_number":259,"context_line":"                # when the data on the volume was first saved. If it is"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_55691723","line":256,"in_reply_to":"1f1a1f67_db70e242","updated":"2017-07-24 11:51:16.000000000","message":"Done","commit_id":"6e110722ac4c60ee8fe8041f5e399c2a8c5df2d5"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"87f9b24227d44d470817e872735e9cb70bd03321","unresolved":false,"context_lines":[{"line_number":268,"context_line":"                # When taking an incremental backup of the snapshot, the"},{"line_number":269,"context_line":"                # parent should be the backup at 8:00, not 8:20, and the"},{"line_number":270,"context_line":"                # \u0027data_timestamp\u0027 of this new backup will be 8:10."},{"line_number":271,"context_line":"                latest_backup \u003d max("},{"line_number":272,"context_line":"                    backups.objects,"},{"line_number":273,"context_line":"                    key\u003dlambda x: x[\u0027data_timestamp\u0027]"},{"line_number":274,"context_line":"                    if (not snapshot or (snapshot and x[\u0027data_timestamp\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"ffb9cba7_6d6526e1","line":271,"range":{"start_line":271,"start_character":32,"end_line":271,"end_character":36},"updated":"2019-04-24 14:22:07.000000000","message":"If the backups query now only returns one item, this should be re-worked, right?\n\n(I think this max logic is being moved into the object/db query above.)","commit_id":"6048217d70342ca5c129535a355e7f0effe36e80"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"4b43ff0efb1f641a046a99a8181a9a3fd6d0b8c4","unresolved":false,"context_lines":[{"line_number":268,"context_line":"                # When taking an incremental backup of the snapshot, the"},{"line_number":269,"context_line":"                # parent should be the backup at 8:00, not 8:20, and the"},{"line_number":270,"context_line":"                # \u0027data_timestamp\u0027 of this new backup will be 8:10."},{"line_number":271,"context_line":"                latest_backup \u003d max("},{"line_number":272,"context_line":"                    backups.objects,"},{"line_number":273,"context_line":"                    key\u003dlambda x: x[\u0027data_timestamp\u0027]"},{"line_number":274,"context_line":"                    if (not snapshot or (snapshot and x[\u0027data_timestamp\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"ffb9cba7_ad81de4c","line":271,"range":{"start_line":271,"start_character":32,"end_line":271,"end_character":36},"in_reply_to":"ffb9cba7_6d6526e1","updated":"2019-04-24 14:23:22.000000000","message":"Also, doesn\u0027t there need to be filtering to ensure we pick a successful backup here?  (In both the old and new code.)","commit_id":"6048217d70342ca5c129535a355e7f0effe36e80"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"054fcd1e4d349fa25f716f1a265b86a4afd52e47","unresolved":true,"context_lines":[{"line_number":43,"context_line":"from cinder import quota_utils"},{"line_number":44,"context_line":"from cinder.scheduler import rpcapi as scheduler_rpcapi"},{"line_number":45,"context_line":"import cinder.volume"},{"line_number":46,"context_line":"import datetime"},{"line_number":47,"context_line":"import traceback"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"backup_opts \u003d ["}],"source_content_type":"text/x-python","patch_set":11,"id":"da863ed2_25c848c5","line":46,"updated":"2024-02-07 14:39:28.000000000","message":"This is strange -- this import should generate an I100 error for being in the wrong group, but the pep8 check job passes here.","commit_id":"56ebbfb5fabde2e330f11d16770efd2b6ed5aea7"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"c83d1ef056e4e0584d25c382f4c7c1970b8b48bf","unresolved":false,"context_lines":[{"line_number":43,"context_line":"from cinder import quota_utils"},{"line_number":44,"context_line":"from cinder.scheduler import rpcapi as scheduler_rpcapi"},{"line_number":45,"context_line":"import cinder.volume"},{"line_number":46,"context_line":"import datetime"},{"line_number":47,"context_line":"import traceback"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"backup_opts \u003d ["}],"source_content_type":"text/x-python","patch_set":11,"id":"ab99ee66_dc76e53e","line":46,"in_reply_to":"da863ed2_25c848c5","updated":"2026-06-23 15:48:39.000000000","message":"Acknowledged","commit_id":"56ebbfb5fabde2e330f11d16770efd2b6ed5aea7"},{"author":{"_account_id":13915,"name":"Silvan Kaiser","email":"silvan@quobyte.com","username":"kaisers"},"change_message_id":"34cf64d1a9c873f22e2f7d55b571a43533aa4977","unresolved":true,"context_lines":[{"line_number":308,"context_line":"            if not parent_backup:"},{"line_number":309,"context_line":"                # NOTE (nschwarz) Fetching the parent backup via a database"},{"line_number":310,"context_line":"                # query is the optimized way but due to the previous filtering"},{"line_number":311,"context_line":"                # (including backups in the evaluation because a datetime was"},{"line_number":312,"context_line":"                # set for them which are not in scope for valid parent backups"},{"line_number":313,"context_line":"                # which cannot be implemented in the query it is required to"},{"line_number":314,"context_line":"                # preserve the old behaviour to find a suitable backup."}],"source_content_type":"text/x-python","patch_set":25,"id":"cae7d177_06cefe2e","line":311,"range":{"start_line":311,"start_character":18,"end_line":311,"end_character":19},"updated":"2026-02-06 15:21:08.000000000","message":"Not sure where this parenthesis should close?","commit_id":"616e382b4b16784de34ebc1393f847d3786f24d2"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"0ce73476d895f7735d7fc062d6d8fefae400d8e7","unresolved":false,"context_lines":[{"line_number":308,"context_line":"            if not parent_backup:"},{"line_number":309,"context_line":"                # NOTE (nschwarz) Fetching the parent backup via a database"},{"line_number":310,"context_line":"                # query is the optimized way but due to the previous filtering"},{"line_number":311,"context_line":"                # (including backups in the evaluation because a datetime was"},{"line_number":312,"context_line":"                # set for them which are not in scope for valid parent backups"},{"line_number":313,"context_line":"                # which cannot be implemented in the query it is required to"},{"line_number":314,"context_line":"                # preserve the old behaviour to find a suitable backup."}],"source_content_type":"text/x-python","patch_set":25,"id":"a34847db_b7ea80e0","line":311,"range":{"start_line":311,"start_character":18,"end_line":311,"end_character":19},"in_reply_to":"cae7d177_06cefe2e","updated":"2026-02-16 11:28:44.000000000","message":"Reworded with the next patchset.","commit_id":"616e382b4b16784de34ebc1393f847d3786f24d2"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"acb21f427f107837c4a4fde4a06e046a1c324d45","unresolved":true,"context_lines":[{"line_number":306,"context_line":"            )"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"            if not parent_backup:"},{"line_number":309,"context_line":"                # NOTE (nschwarz) Fetching the parent backup via a database"},{"line_number":310,"context_line":"                # query is the optimized way but due to the previous filtering"},{"line_number":311,"context_line":"                # (including backups in the evaluation because a datetime was"},{"line_number":312,"context_line":"                # set for them which are not in scope for valid parent backups"},{"line_number":313,"context_line":"                # which cannot be implemented in the query it is required to"},{"line_number":314,"context_line":"                # preserve the old behaviour to find a suitable backup."},{"line_number":315,"context_line":"                # This should be removed in further patches and replaced with"},{"line_number":316,"context_line":"                # an api error"},{"line_number":317,"context_line":"                LOG.warning(\u0027Switching to unoptimized behaviour to find \u0027"},{"line_number":318,"context_line":"                            \u0027possible parent backup\u0027)"},{"line_number":319,"context_line":"                backups \u003d objects.BackupList.get_all_by_volume("}],"source_content_type":"text/x-python","patch_set":25,"id":"64c36e4d_29d49c52","line":316,"range":{"start_line":309,"start_character":0,"end_line":316,"end_character":30},"updated":"2026-02-06 15:11:12.000000000","message":"Can you better explain this? I\u0027m failing to understand why it would needed to query the database twice (in this case, we made the performance even worse), since the data should be available in the database at the time the first query was run. If that is failing to find it, it seems to me that the query should be fixed in the first place.","commit_id":"616e382b4b16784de34ebc1393f847d3786f24d2"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"c83d1ef056e4e0584d25c382f4c7c1970b8b48bf","unresolved":false,"context_lines":[{"line_number":306,"context_line":"            )"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"            if not parent_backup:"},{"line_number":309,"context_line":"                # NOTE (nschwarz) Fetching the parent backup via a database"},{"line_number":310,"context_line":"                # query is the optimized way but due to the previous filtering"},{"line_number":311,"context_line":"                # (including backups in the evaluation because a datetime was"},{"line_number":312,"context_line":"                # set for them which are not in scope for valid parent backups"},{"line_number":313,"context_line":"                # which cannot be implemented in the query it is required to"},{"line_number":314,"context_line":"                # preserve the old behaviour to find a suitable backup."},{"line_number":315,"context_line":"                # This should be removed in further patches and replaced with"},{"line_number":316,"context_line":"                # an api error"},{"line_number":317,"context_line":"                LOG.warning(\u0027Switching to unoptimized behaviour to find \u0027"},{"line_number":318,"context_line":"                            \u0027possible parent backup\u0027)"},{"line_number":319,"context_line":"                backups \u003d objects.BackupList.get_all_by_volume("}],"source_content_type":"text/x-python","patch_set":25,"id":"3fab84ae_5db59892","line":316,"range":{"start_line":309,"start_character":0,"end_line":316,"end_character":30},"in_reply_to":"6046c80d_ba1fc90b","updated":"2026-06-23 15:48:39.000000000","message":"Done","commit_id":"616e382b4b16784de34ebc1393f847d3786f24d2"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"0ce73476d895f7735d7fc062d6d8fefae400d8e7","unresolved":true,"context_lines":[{"line_number":306,"context_line":"            )"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"            if not parent_backup:"},{"line_number":309,"context_line":"                # NOTE (nschwarz) Fetching the parent backup via a database"},{"line_number":310,"context_line":"                # query is the optimized way but due to the previous filtering"},{"line_number":311,"context_line":"                # (including backups in the evaluation because a datetime was"},{"line_number":312,"context_line":"                # set for them which are not in scope for valid parent backups"},{"line_number":313,"context_line":"                # which cannot be implemented in the query it is required to"},{"line_number":314,"context_line":"                # preserve the old behaviour to find a suitable backup."},{"line_number":315,"context_line":"                # This should be removed in further patches and replaced with"},{"line_number":316,"context_line":"                # an api error"},{"line_number":317,"context_line":"                LOG.warning(\u0027Switching to unoptimized behaviour to find \u0027"},{"line_number":318,"context_line":"                            \u0027possible parent backup\u0027)"},{"line_number":319,"context_line":"                backups \u003d objects.BackupList.get_all_by_volume("}],"source_content_type":"text/x-python","patch_set":25,"id":"ef3b264e_22561a49","line":316,"range":{"start_line":309,"start_character":0,"end_line":316,"end_character":30},"in_reply_to":"64c36e4d_29d49c52","updated":"2026-02-16 11:28:44.000000000","message":"First of all thanks Erlon for reviewing this patch! I must admit that after two YEARS I did not expect any movement on this one anymore. But let me be blunt, this is also a high risk of not getting a hold of a contributor anymore, at the very least it takes a while for the reviewer to dive back into his own code ;-)\n\nTo answer your question please kindly see the conversation at \nhttps://review.opendev.org/c/openstack/cinder/+/484729/comments/b6da8f16_5c9481e3\n\nIn essence ... this does NOT query the database twice, but actually uses the new DB-query method to determine a parent. If, and only IF, no parent is found, the old code is used to attempt to find a parent. Sole to maintain the old behavior.\n\nThis was also discussed at the PTG in Apr 2024 (https://etherpad.opendev.org/p/dalmatian-ptg-cinder#L485) I believe.\n\nSince to me the alternative path only manifests use-cases which should not actually work looking at the volume and backups lifecycle, I would have not issue to remove this alternative and throw an error.\n\nBut, back then it was about allowing the massive speed improvement while not changing the behavior at all.","commit_id":"616e382b4b16784de34ebc1393f847d3786f24d2"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"da25b066bf5cc2cae92e07f18c0ead7183ea7960","unresolved":true,"context_lines":[{"line_number":306,"context_line":"            )"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"            if not parent_backup:"},{"line_number":309,"context_line":"                # NOTE (nschwarz) Fetching the parent backup via a database"},{"line_number":310,"context_line":"                # query is the optimized way but due to the previous filtering"},{"line_number":311,"context_line":"                # (including backups in the evaluation because a datetime was"},{"line_number":312,"context_line":"                # set for them which are not in scope for valid parent backups"},{"line_number":313,"context_line":"                # which cannot be implemented in the query it is required to"},{"line_number":314,"context_line":"                # preserve the old behaviour to find a suitable backup."},{"line_number":315,"context_line":"                # This should be removed in further patches and replaced with"},{"line_number":316,"context_line":"                # an api error"},{"line_number":317,"context_line":"                LOG.warning(\u0027Switching to unoptimized behaviour to find \u0027"},{"line_number":318,"context_line":"                            \u0027possible parent backup\u0027)"},{"line_number":319,"context_line":"                backups \u003d objects.BackupList.get_all_by_volume("}],"source_content_type":"text/x-python","patch_set":25,"id":"6046c80d_ba1fc90b","line":316,"range":{"start_line":309,"start_character":0,"end_line":316,"end_character":30},"in_reply_to":"8a54eacd_5a101e87","updated":"2026-03-26 13:33:32.000000000","message":"Hey Nilkas, so, doing another pass on this with more attention. The new code added on backup_get_parent_for_incremental() already does what the old code do in all practical scenarios I can think of. If no timestap is passed, it will not use the filter, select all entries, sort and return the first. **It always find a suitable backup**.\n\nThe only case where the DB query returns None is when no AVAILABLE backup exists for that volume/project/timestamp combination. In that exact same scenario, max() returns a non-AVAILABLE backup, and the fallback code then sets parent_backup \u003d None anyway. So the fallback doesn\u0027t rescue anything — it just takes a slower path to the same None.\n\nThis means the fallback is dead code by construction, not just in theory. Keeping it doesn\u0027t preserve any behavior — it only adds confusion, a misleading log warning, and an unnecessary second DB round-trip.\n\nThe old code is confusing and that\u0027s what\u0027s making us to agonize on whether to keep it or not, so we should not defer the action recommended even by the author, \"TODO Should be removed in a further patch and cause api error\", and remove that code in this change.\n\nAddtionally to that, can you please also change the beforeDataTimestamp camel casing, to before_data_timestamp to comply with the coding standard?\n\nps. a way to put a stone on the matter, would be to change the test_create_backup_delta test, creating the edge scenarios you are visualizing, and making asserts showing that the code, can pass in both branches, new and legacy. With that I would be happy to let the code as it is.","commit_id":"616e382b4b16784de34ebc1393f847d3786f24d2"},{"author":{"_account_id":34149,"name":"Niklas Schwarz","email":"niklas.schwarz@inovex.de","username":"nschwarz"},"change_message_id":"514812223f3a714b9fba8e8da20301372fd0d539","unresolved":true,"context_lines":[{"line_number":306,"context_line":"            )"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"            if not parent_backup:"},{"line_number":309,"context_line":"                # NOTE (nschwarz) Fetching the parent backup via a database"},{"line_number":310,"context_line":"                # query is the optimized way but due to the previous filtering"},{"line_number":311,"context_line":"                # (including backups in the evaluation because a datetime was"},{"line_number":312,"context_line":"                # set for them which are not in scope for valid parent backups"},{"line_number":313,"context_line":"                # which cannot be implemented in the query it is required to"},{"line_number":314,"context_line":"                # preserve the old behaviour to find a suitable backup."},{"line_number":315,"context_line":"                # This should be removed in further patches and replaced with"},{"line_number":316,"context_line":"                # an api error"},{"line_number":317,"context_line":"                LOG.warning(\u0027Switching to unoptimized behaviour to find \u0027"},{"line_number":318,"context_line":"                            \u0027possible parent backup\u0027)"},{"line_number":319,"context_line":"                backups \u003d objects.BackupList.get_all_by_volume("}],"source_content_type":"text/x-python","patch_set":25,"id":"8a54eacd_5a101e87","line":316,"range":{"start_line":309,"start_character":0,"end_line":316,"end_character":30},"in_reply_to":"cfc2dcef_2778b96f","updated":"2026-02-20 16:32:15.000000000","message":"The new code uses an effective database query to filter for the latest AVAILABLE backup based on a timestamp which is the correct way. It finds the latest backup and uses this for an incremental backup.\n\nThe old code just finds the latest AVAILABLE backup before the timestamp 1.1.1, so it should always find a suitable backup even if it is not the latest. This will result in bigger incremental backup. This situation can happen if, e.g. are currently restoring the latest backup. That\u0027s why it would be a good change to also include additional states in the database query. But as discussed above this is out of scope for this change.\n\nExactly this beforeDataTimestamp is the performance booster for the search for a suitable backup.\n\nThe reason the old code is still included is because, as already mentioned, of backwards capability. What the correct way of handling the edge case of not finding the latest backup is should be another discussion.","commit_id":"616e382b4b16784de34ebc1393f847d3786f24d2"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"92bc192a80972b2f4e197fb67e43f8886246462e","unresolved":true,"context_lines":[{"line_number":306,"context_line":"            )"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"            if not parent_backup:"},{"line_number":309,"context_line":"                # NOTE (nschwarz) Fetching the parent backup via a database"},{"line_number":310,"context_line":"                # query is the optimized way but due to the previous filtering"},{"line_number":311,"context_line":"                # (including backups in the evaluation because a datetime was"},{"line_number":312,"context_line":"                # set for them which are not in scope for valid parent backups"},{"line_number":313,"context_line":"                # which cannot be implemented in the query it is required to"},{"line_number":314,"context_line":"                # preserve the old behaviour to find a suitable backup."},{"line_number":315,"context_line":"                # This should be removed in further patches and replaced with"},{"line_number":316,"context_line":"                # an api error"},{"line_number":317,"context_line":"                LOG.warning(\u0027Switching to unoptimized behaviour to find \u0027"},{"line_number":318,"context_line":"                            \u0027possible parent backup\u0027)"},{"line_number":319,"context_line":"                backups \u003d objects.BackupList.get_all_by_volume("}],"source_content_type":"text/x-python","patch_set":25,"id":"cfc2dcef_2778b96f","line":316,"range":{"start_line":309,"start_character":0,"end_line":316,"end_character":30},"in_reply_to":"ef3b264e_22561a49","updated":"2026-02-20 16:10:52.000000000","message":"So my point is, both queries filter the same thing. The fallback is effectively dead code: the DB query already applies the same filters, so the only case where it returns None is when no AVAILABLE backup exists. This parent vs latest thing is a bit confusing , so I might be missing something.\n\nAlso, the beforeDataTimestamp filter is out of the contex of this change (performace) and shloud be added in a separate change.","commit_id":"616e382b4b16784de34ebc1393f847d3786f24d2"},{"author":{"_account_id":36171,"name":"jayaanand borra","display_name":"jayaanand borra","email":"jayaanand.borra@netapp.com","username":"jayaanan","status":"netapp"},"change_message_id":"aade4af9eb6e49742f7924c7e77044c0316aeaf8","unresolved":true,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"\"\"\"Handles all requests relating to the volume backups service.\"\"\""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from __future__ import annotations"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"import random"},{"line_number":23,"context_line":"from typing import Optional"}],"source_content_type":"text/x-python","patch_set":29,"id":"349c02da_9f292601","line":20,"updated":"2026-04-07 08:40:04.000000000","message":"from __future__ import annotations changes all annotations in the file to be lazily evaluated strings. do you see any impact on runtime code?","commit_id":"29594932bee2ef312d9151770bc615221ca41196"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"a0a1321ac82e4c33624687340a0b6be397777d75","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"\"\"\"Handles all requests relating to the volume backups service.\"\"\""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from __future__ import annotations"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"import random"},{"line_number":23,"context_line":"from typing import Optional"}],"source_content_type":"text/x-python","patch_set":29,"id":"c2c30e1b_47c02ae4","line":20,"in_reply_to":"349c02da_9f292601","updated":"2026-04-07 14:19:50.000000000","message":"Wondering how / when that was added. Removed the import now.","commit_id":"29594932bee2ef312d9151770bc615221ca41196"}],"cinder/db/api.py":[{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"1f922bbf6373e6c84c1860c16528fb6a510609ad","unresolved":true,"context_lines":[{"line_number":6579,"context_line":"                           project_only\u003dTrue)"},{"line_number":6580,"context_line":"        .filter_by(project_id\u003dcontext.project_id)"},{"line_number":6581,"context_line":"        .filter_by(volume_id\u003dvolume_id)"},{"line_number":6582,"context_line":"        .filter(models.Backup.status.in_("},{"line_number":6583,"context_line":"            ("},{"line_number":6584,"context_line":"                fields.BackupStatus.AVAILABLE,"},{"line_number":6585,"context_line":"                fields.BackupStatus.RESTORING"},{"line_number":6586,"context_line":"            )"},{"line_number":6587,"context_line":"        ))"},{"line_number":6588,"context_line":"        .order_by(desc(models.Backup.data_timestamp))"},{"line_number":6589,"context_line":"    )"},{"line_number":6590,"context_line":"    # NOTE(crohmann): In case a backup with a data_timestamp"}],"source_content_type":"text/x-python","patch_set":32,"id":"097e3017_206688ed","line":6587,"range":{"start_line":6582,"start_character":8,"end_line":6587,"end_character":10},"updated":"2026-07-13 18:38:45.000000000","message":"Behavior change: the code this replaces accepted a parent only in AVAILABLE state (the old max() key required status \u003d\u003d AVAILABLE, plus an explicit !\u003d AVAILABLE → raise). Including RESTORING is defensible — a backup being restored is a read-only, intact source — but it\u0027s not mentioned in the commit message or release note and isn\u0027t tested. Please either keep it with (a) a code comment here explaining why RESTORING is a valid base, (b) a note in the release note, and (c) a positive test that a RESTORING backup is selected; or drop RESTORING to preserve prior behavior.","commit_id":"7b89aba74d5f17a0470cc3847bd366201c618533"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"448f499bb292ccf30aaad8522491d249a7019b2d","unresolved":false,"context_lines":[{"line_number":6579,"context_line":"                           project_only\u003dTrue)"},{"line_number":6580,"context_line":"        .filter_by(project_id\u003dcontext.project_id)"},{"line_number":6581,"context_line":"        .filter_by(volume_id\u003dvolume_id)"},{"line_number":6582,"context_line":"        .filter(models.Backup.status.in_("},{"line_number":6583,"context_line":"            ("},{"line_number":6584,"context_line":"                fields.BackupStatus.AVAILABLE,"},{"line_number":6585,"context_line":"                fields.BackupStatus.RESTORING"},{"line_number":6586,"context_line":"            )"},{"line_number":6587,"context_line":"        ))"},{"line_number":6588,"context_line":"        .order_by(desc(models.Backup.data_timestamp))"},{"line_number":6589,"context_line":"    )"},{"line_number":6590,"context_line":"    # NOTE(crohmann): In case a backup with a data_timestamp"}],"source_content_type":"text/x-python","patch_set":32,"id":"d1ba81f2_f19468b2","line":6587,"range":{"start_line":6582,"start_character":8,"end_line":6587,"end_character":10},"in_reply_to":"097e3017_206688ed","updated":"2026-07-16 13:27:21.000000000","message":"Acknowledged","commit_id":"7b89aba74d5f17a0470cc3847bd366201c618533"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"1f922bbf6373e6c84c1860c16528fb6a510609ad","unresolved":true,"context_lines":[{"line_number":6585,"context_line":"                fields.BackupStatus.RESTORING"},{"line_number":6586,"context_line":"            )"},{"line_number":6587,"context_line":"        ))"},{"line_number":6588,"context_line":"        .order_by(desc(models.Backup.data_timestamp))"},{"line_number":6589,"context_line":"    )"},{"line_number":6590,"context_line":"    # NOTE(crohmann): In case a backup with a data_timestamp"},{"line_number":6591,"context_line":"    # before a certain time is required add this condition."}],"source_content_type":"text/x-python","patch_set":32,"id":"79c6fdf2_8649a98e","line":6588,"range":{"start_line":6588,"start_character":9,"end_line":6588,"end_character":22},"updated":"2026-07-13 18:38:45.000000000","message":"Nit: no secondary sort, so ties on data_timestamp resolve in DB-defined order. Rarely matters, but a tiebreaker like .order_by(desc(data_timestamp), desc(created_at)) (or id) would make selection deterministic.","commit_id":"7b89aba74d5f17a0470cc3847bd366201c618533"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"448f499bb292ccf30aaad8522491d249a7019b2d","unresolved":false,"context_lines":[{"line_number":6585,"context_line":"                fields.BackupStatus.RESTORING"},{"line_number":6586,"context_line":"            )"},{"line_number":6587,"context_line":"        ))"},{"line_number":6588,"context_line":"        .order_by(desc(models.Backup.data_timestamp))"},{"line_number":6589,"context_line":"    )"},{"line_number":6590,"context_line":"    # NOTE(crohmann): In case a backup with a data_timestamp"},{"line_number":6591,"context_line":"    # before a certain time is required add this condition."}],"source_content_type":"text/x-python","patch_set":32,"id":"f60bf1bf_7f3c0c91","line":6588,"range":{"start_line":6588,"start_character":9,"end_line":6588,"end_character":22},"in_reply_to":"79c6fdf2_8649a98e","updated":"2026-07-16 13:27:21.000000000","message":"Acknowledged","commit_id":"7b89aba74d5f17a0470cc3847bd366201c618533"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"24e6442df670dd76c1b613d577297ca249768d09","unresolved":true,"context_lines":[{"line_number":6576,"context_line":"    # data_timestamp"},{"line_number":6577,"context_line":"    # Apart from an AVAILABLE backup, any backup that is currently RESTORING"},{"line_number":6578,"context_line":"    # to a new volume is also valid, as it\u0027s a stable, read-only source."},{"line_number":6579,"context_line":"    query \u003d ("},{"line_number":6580,"context_line":"        _backups_get_query(context\u003dcontext, joined_load\u003dFalse,"},{"line_number":6581,"context_line":"                           project_only\u003dTrue)"},{"line_number":6582,"context_line":"        .filter_by(project_id\u003dcontext.project_id)"}],"source_content_type":"text/x-python","patch_set":35,"id":"8050337e_94997861","line":6579,"updated":"2026-08-14 00:05:10.000000000","message":"Since optimisation is the whole point of the change:\nno index supports this query. `cinder/db/models.py` declares only\n`backups_deleted_project_id_idx on (deleted, project_id)` — nothing on `volume_id`\nor `data_timestamp`.","commit_id":"5b0e05c6116516fd26dcecab3c39c53ff787786b"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"2c4bd71021703f17f56bf54e1301647e8956edba","unresolved":false,"context_lines":[{"line_number":6576,"context_line":"    # data_timestamp"},{"line_number":6577,"context_line":"    # Apart from an AVAILABLE backup, any backup that is currently RESTORING"},{"line_number":6578,"context_line":"    # to a new volume is also valid, as it\u0027s a stable, read-only source."},{"line_number":6579,"context_line":"    query \u003d ("},{"line_number":6580,"context_line":"        _backups_get_query(context\u003dcontext, joined_load\u003dFalse,"},{"line_number":6581,"context_line":"                           project_only\u003dTrue)"},{"line_number":6582,"context_line":"        .filter_by(project_id\u003dcontext.project_id)"}],"source_content_type":"text/x-python","patch_set":35,"id":"c5e4dd2f_ebdded03","line":6579,"in_reply_to":"8050337e_94997861","updated":"2026-08-18 14:13:49.000000000","message":"Done.\n\nI created a new idx with volume_id first and created_at as second component.\nThat should help with all operations by volume_id and then also when filtering on created_at values.","commit_id":"5b0e05c6116516fd26dcecab3c39c53ff787786b"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"24e6442df670dd76c1b613d577297ca249768d09","unresolved":true,"context_lines":[{"line_number":6589,"context_line":"        ))"},{"line_number":6590,"context_line":"        .order_by("},{"line_number":6591,"context_line":"            desc(models.Backup.data_timestamp),"},{"line_number":6592,"context_line":"            desc(models.Backup.id)"},{"line_number":6593,"context_line":"        )"},{"line_number":6594,"context_line":"    )"},{"line_number":6595,"context_line":"    # NOTE(crohmann): In case a backup with a data_timestamp"}],"source_content_type":"text/x-python","patch_set":35,"id":"8444de22_c2ba056c","line":6592,"updated":"2026-08-14 00:05:10.000000000","message":"`id` is a UUID so ordering bears no lelation to how recent the backup is. `data_timestamp` could work, but two backups from the same snapshot would have the same value, so maybe `created_at` is the correct approach","commit_id":"5b0e05c6116516fd26dcecab3c39c53ff787786b"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"2c4bd71021703f17f56bf54e1301647e8956edba","unresolved":false,"context_lines":[{"line_number":6589,"context_line":"        ))"},{"line_number":6590,"context_line":"        .order_by("},{"line_number":6591,"context_line":"            desc(models.Backup.data_timestamp),"},{"line_number":6592,"context_line":"            desc(models.Backup.id)"},{"line_number":6593,"context_line":"        )"},{"line_number":6594,"context_line":"    )"},{"line_number":6595,"context_line":"    # NOTE(crohmann): In case a backup with a data_timestamp"}],"source_content_type":"text/x-python","patch_set":35,"id":"5fd68414_fa48421b","line":6592,"in_reply_to":"8444de22_c2ba056c","updated":"2026-08-18 14:13:49.000000000","message":"Done\n\nChanged to created_at - I likely configued this with the snapshot id which Ceph does and that is (I doubt this is guaranteed though) an ever increasing value.","commit_id":"5b0e05c6116516fd26dcecab3c39c53ff787786b"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"c34db7071d3d020715b1c3bc7387aa245a493189","unresolved":true,"context_lines":[{"line_number":6579,"context_line":"    query \u003d ("},{"line_number":6580,"context_line":"        _backups_get_query(context\u003dcontext, joined_load\u003dFalse,"},{"line_number":6581,"context_line":"                           project_only\u003dTrue)"},{"line_number":6582,"context_line":"        .filter_by(volume_id\u003dvolume_id)"},{"line_number":6583,"context_line":"        .filter(models.Backup.status.in_("},{"line_number":6584,"context_line":"            ("},{"line_number":6585,"context_line":"                fields.BackupStatus.AVAILABLE,"}],"source_content_type":"text/x-python","patch_set":42,"id":"038c4fec_ead3c761","line":6582,"updated":"2026-09-03 12:52:51.000000000","message":"PS36 had\n\n    .filter_by(project_id\u003dcontext.project_id)\n\non this line and PS37 removed it. Pure CI was green on PS36 and has been red\non every patchset since.\n\nThe filter is not redundant with project_only\u003dTrue. model_query() applies the\nproject_only filter only \"if is_user_context(context)\", so:\n\n  - user context  -\u003e still scoped to their own project\n  - admin context -\u003e is_user_context() is False, so NO project filter at all,\n                     and the query sees every project\u0027s backups\n\nauthorize_project_context(context, volume_project_id) doesn\u0027t cover it either\n— that\u0027s also a no-op for admin contexts. And the pre-patch code passed\nfilters\u003d{\u0027project_id\u0027: context.project_id} explicitly, which applied\nregardless of admin-ness. So this is an admin-context-only behaviour change.\n\nWhat it does in practice, from the PS42 iSCSI run:\n\n  child  f51a3967  project_id \u003d 50552f2d6d4f4812936c3e126be8d25a  (admin)\n  parent f6c60868  project_id \u003d 95e9288bf1b44583a4141cef95f44db3  (user)\n\nThe admin\u0027s incremental selected the *user\u0027s* backup as its parent. The Swift\ndriver then looked for the parent\u0027s manifest in the requesting project\u0027s\naccount and got a 404:\n\n  swiftclient.exceptions.ClientException: Object GET failed:\n  .../v1/AUTH_50552f2d6d4f4812936c3e126be8d25a/volumebackups/\n      volume_f0a4b030-.../az_nova_backup_f6c60868-..._sha256file 404 Not Found\n\nwhich propagates out of continue_backup and puts the backup in ERROR. Any\ndriver that namespaces storage per project breaks the same way — Swift just\nsurfaces it loudly. Independently of the ERROR, an admin silently basing an\nincremental on another project\u0027s backup is a cross-tenant issue in its own\nright, which is what test_backup_crossproject_admin_negative exists to\nprevent.\n\nPlease restore the filter:\n\n    _backups_get_query(context\u003dcontext, joined_load\u003dFalse,\n                       project_only\u003dTrue)\n    .filter_by(project_id\u003dcontext.project_id)\n    .filter_by(volume_id\u003dvolume_id)\n\nWorth a short NOTE too, since this looks removable but isn\u0027t — something like\n\"project_only only scopes user contexts; admins need this explicit filter so\nthey can\u0027t base an incremental on another project\u0027s backup.\"","commit_id":"72fec9c71d666ed471ee9a0ae16a036e55f6cd0b"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"6b35acc600a7eac0a22c378c4dccca5e50355b25","unresolved":false,"context_lines":[{"line_number":6579,"context_line":"    query \u003d ("},{"line_number":6580,"context_line":"        _backups_get_query(context\u003dcontext, joined_load\u003dFalse,"},{"line_number":6581,"context_line":"                           project_only\u003dTrue)"},{"line_number":6582,"context_line":"        .filter_by(volume_id\u003dvolume_id)"},{"line_number":6583,"context_line":"        .filter(models.Backup.status.in_("},{"line_number":6584,"context_line":"            ("},{"line_number":6585,"context_line":"                fields.BackupStatus.AVAILABLE,"}],"source_content_type":"text/x-python","patch_set":42,"id":"cc71a9eb_b3aff04b","line":6582,"in_reply_to":"038c4fec_ead3c761","updated":"2026-09-04 15:29:46.000000000","message":"Acknowledged.\n\nArgh, sorry. I drive-by removed this and did not mention it in my update.\nThanks for explaining the consequences in all detail!","commit_id":"72fec9c71d666ed471ee9a0ae16a036e55f6cd0b"}],"cinder/db/models.py":[{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"092122d1194fc0968c78bdc6b447b9997a86e106","unresolved":true,"context_lines":[{"line_number":1012,"context_line":"        # Speed up normal listings"},{"line_number":1013,"context_line":"        sa.Index(\u0027backups_deleted_project_id_idx\u0027, \u0027deleted\u0027, \u0027project_id\u0027),"},{"line_number":1014,"context_line":"        # Speed up searches by volume_id and creation date"},{"line_number":1015,"context_line":"        sa.Index(\u0027backups_volume_id_created_at_idx\u0027,"},{"line_number":1016,"context_line":"                 \u0027volume_id\u0027,"},{"line_number":1017,"context_line":"                 \u0027created_at\u0027),"},{"line_number":1018,"context_line":"        CinderBase.__table_args__,"}],"source_content_type":"text/x-python","patch_set":36,"id":"6d47785d_f998441d","line":1015,"updated":"2026-08-18 15:27:51.000000000","message":"this needs an Alebmic migration, which will need a mention in the release note","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"6b35acc600a7eac0a22c378c4dccca5e50355b25","unresolved":false,"context_lines":[{"line_number":1012,"context_line":"        # Speed up normal listings"},{"line_number":1013,"context_line":"        sa.Index(\u0027backups_deleted_project_id_idx\u0027, \u0027deleted\u0027, \u0027project_id\u0027),"},{"line_number":1014,"context_line":"        # Speed up searches by volume_id and creation date"},{"line_number":1015,"context_line":"        sa.Index(\u0027backups_volume_id_created_at_idx\u0027,"},{"line_number":1016,"context_line":"                 \u0027volume_id\u0027,"},{"line_number":1017,"context_line":"                 \u0027created_at\u0027),"},{"line_number":1018,"context_line":"        CinderBase.__table_args__,"}],"source_content_type":"text/x-python","patch_set":36,"id":"157d0487_2957270b","line":1015,"in_reply_to":"1aeb76e0_ad5897e1","updated":"2026-09-04 15:29:46.000000000","message":"Acknowledged","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"c34db7071d3d020715b1c3bc7387aa245a493189","unresolved":true,"context_lines":[{"line_number":1012,"context_line":"        # Speed up normal listings"},{"line_number":1013,"context_line":"        sa.Index(\u0027backups_deleted_project_id_idx\u0027, \u0027deleted\u0027, \u0027project_id\u0027),"},{"line_number":1014,"context_line":"        # Speed up searches by volume_id and creation date"},{"line_number":1015,"context_line":"        sa.Index(\u0027backups_volume_id_created_at_idx\u0027,"},{"line_number":1016,"context_line":"                 \u0027volume_id\u0027,"},{"line_number":1017,"context_line":"                 \u0027created_at\u0027),"},{"line_number":1018,"context_line":"        CinderBase.__table_args__,"}],"source_content_type":"text/x-python","patch_set":36,"id":"1aeb76e0_ad5897e1","line":1015,"in_reply_to":"61a59d8b_48986ce3","updated":"2026-09-03 12:52:51.000000000","message":"No — that drift is deliberate, and you were right to leave it out. Those\nelements are whitelisted in CinderModelsMigrationsSync.filter_metadata_diff\n(the ignore_leftover_nested_quota helper) with a \"TODO: (D Release)\" marker.\nUnder the rolling-upgrade policy the ORM stops using a column one release\nbefore the DB drops it, so models.py and the schema are intentionally out of\nsync for that window. Autogenerate doesn\u0027t know about the filter, so it\nproposes the drops; the sync test then ignores them.\n\nSo: keep your migration to just the create_index, which is what you\u0027ve done.","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"7351beb8b1a9afb1f47f7dbbc6a3892f326c06e3","unresolved":true,"context_lines":[{"line_number":1012,"context_line":"        # Speed up normal listings"},{"line_number":1013,"context_line":"        sa.Index(\u0027backups_deleted_project_id_idx\u0027, \u0027deleted\u0027, \u0027project_id\u0027),"},{"line_number":1014,"context_line":"        # Speed up searches by volume_id and creation date"},{"line_number":1015,"context_line":"        sa.Index(\u0027backups_volume_id_created_at_idx\u0027,"},{"line_number":1016,"context_line":"                 \u0027volume_id\u0027,"},{"line_number":1017,"context_line":"                 \u0027created_at\u0027),"},{"line_number":1018,"context_line":"        CinderBase.__table_args__,"}],"source_content_type":"text/x-python","patch_set":36,"id":"61a59d8b_48986ce3","line":1015,"in_reply_to":"6d47785d_f998441d","updated":"2026-09-02 07:49:40.000000000","message":"I added the alembic migrations. But on autogenerate these upgrades where also added ...\n\n    op.drop_column(\u0027quotas\u0027, \u0027allocated\u0027)\n    op.drop_index(op.f(\u0027ix_reservations_allocated_id\u0027), table_name\u003d\u0027reservations\u0027)\n    op.drop_constraint(None, \u0027reservations\u0027, type_\u003d\u0027foreignkey\u0027)\n    op.drop_column(\u0027reservations\u0027, \u0027allocated_id\u0027)\n\nWhere there other changes missing their alembic migrations?\n\nAs for the release notes ... how would that comment look like then? Are database migrations not something that has to be done (or at least invoked to be certain) for every release and is written in the upgrade guide at https://docs.openstack.org/cinder/latest/admin/upgrades.html#database-upgrades?","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"092122d1194fc0968c78bdc6b447b9997a86e106","unresolved":true,"context_lines":[{"line_number":1014,"context_line":"        # Speed up searches by volume_id and creation date"},{"line_number":1015,"context_line":"        sa.Index(\u0027backups_volume_id_created_at_idx\u0027,"},{"line_number":1016,"context_line":"                 \u0027volume_id\u0027,"},{"line_number":1017,"context_line":"                 \u0027created_at\u0027),"},{"line_number":1018,"context_line":"        CinderBase.__table_args__,"},{"line_number":1019,"context_line":"    )"},{"line_number":1020,"context_line":""}],"source_content_type":"text/x-python","patch_set":36,"id":"dadb5c58_d048ceab","line":1017,"updated":"2026-08-18 15:27:51.000000000","message":"The column order doesn\u0027t match the query this is meant to accelerate.\n\nbackup_get_parent_for_incremental issues, in effect:\n```\n  WHERE deleted \u003d ? AND project_id \u003d ? AND volume_id \u003d ? AND status IN (...)\n  ORDER BY data_timestamp DESC, created_at DESC\n  LIMIT 1\n```\ncreated_at is only the *tiebreak*; data_timestamp is the primary sort key and\nisn\u0027t in the index at all. So (volume_id, created_at) buys the volume_id\nequality seek — which is the big win over scanning the whole project, so this\nisn\u0027t useless — but the engine still has to filter and filesort on\ndata_timestamp afterwards.\n\nRe your note that it \"should help ... when filtering on created_at values\":\nthis query never filters on created_at, it only orders by it after\ndata_timestamp, so the second column can\u0027t be used here.\n\n(volume_id, data_timestamp) is the minimum useful shape. Ideally\n(deleted, project_id, volume_id, data_timestamp), which covers the entire\npredicate plus the sort and makes this a one-row backwards range scan.\n\nAlso: the commit message says two indices were added, but I only see one here —\nis a second one missing, or should the message say one?","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"7351beb8b1a9afb1f47f7dbbc6a3892f326c06e3","unresolved":true,"context_lines":[{"line_number":1014,"context_line":"        # Speed up searches by volume_id and creation date"},{"line_number":1015,"context_line":"        sa.Index(\u0027backups_volume_id_created_at_idx\u0027,"},{"line_number":1016,"context_line":"                 \u0027volume_id\u0027,"},{"line_number":1017,"context_line":"                 \u0027created_at\u0027),"},{"line_number":1018,"context_line":"        CinderBase.__table_args__,"},{"line_number":1019,"context_line":"    )"},{"line_number":1020,"context_line":""}],"source_content_type":"text/x-python","patch_set":36,"id":"f1975857_8215f154","line":1017,"in_reply_to":"dadb5c58_d048ceab","updated":"2026-09-02 07:49:40.000000000","message":"Oh man ... that was not really done properly, sorry.\nI\u0027d go for the minimum useful shape. MySQL / MariaDB might even do an index merge if I am not mistaken, but in any case maintaining a large index for a rather seldomly issued query might also not be a good use of resources.","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"c34db7071d3d020715b1c3bc7387aa245a493189","unresolved":false,"context_lines":[{"line_number":1014,"context_line":"        # Speed up searches by volume_id and creation date"},{"line_number":1015,"context_line":"        sa.Index(\u0027backups_volume_id_created_at_idx\u0027,"},{"line_number":1016,"context_line":"                 \u0027volume_id\u0027,"},{"line_number":1017,"context_line":"                 \u0027created_at\u0027),"},{"line_number":1018,"context_line":"        CinderBase.__table_args__,"},{"line_number":1019,"context_line":"    )"},{"line_number":1020,"context_line":""}],"source_content_type":"text/x-python","patch_set":36,"id":"c58b9e4a_5865a5d8","line":1017,"in_reply_to":"f1975857_8215f154","updated":"2026-09-03 12:52:51.000000000","message":"Acknowledged","commit_id":"5e843ea8282137390ac7947c933da256419353b2"}],"cinder/db/sqlalchemy/api.py":[{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"d8399b3d2296a4d6acef7656ac5cdae9589ca9e4","unresolved":true,"context_lines":[{"line_number":6471,"context_line":"    query \u003d ("},{"line_number":6472,"context_line":"        _backups_get_query(context\u003dcontext, joined_load\u003dFalse,"},{"line_number":6473,"context_line":"                           project_only\u003dTrue)"},{"line_number":6474,"context_line":"        .filter_by(project_id\u003dcontext.project_id)"},{"line_number":6475,"context_line":"        .filter_by(volume_id\u003dvolume_id)"},{"line_number":6476,"context_line":"        .filter_by(status\u003dfields.BackupStatus.AVAILABLE)"},{"line_number":6477,"context_line":"        .order_by(desc(models.Backup.data_timestamp))"}],"source_content_type":"text/x-python","patch_set":16,"id":"668ced84_315f5895","line":6474,"updated":"2024-03-04 15:03:23.000000000","message":"As much as I can tell, filtering by project_id wasn\u0027t done before. What brought this on? Did you run some tests that indicated that it was needed?","commit_id":"b4e46ba5745ebefd3d2092686069e412eac222fa"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"7a121c3b4050d613894b1b2caa5914d96bf354b1","unresolved":false,"context_lines":[{"line_number":6471,"context_line":"    query \u003d ("},{"line_number":6472,"context_line":"        _backups_get_query(context\u003dcontext, joined_load\u003dFalse,"},{"line_number":6473,"context_line":"                           project_only\u003dTrue)"},{"line_number":6474,"context_line":"        .filter_by(project_id\u003dcontext.project_id)"},{"line_number":6475,"context_line":"        .filter_by(volume_id\u003dvolume_id)"},{"line_number":6476,"context_line":"        .filter_by(status\u003dfields.BackupStatus.AVAILABLE)"},{"line_number":6477,"context_line":"        .order_by(desc(models.Backup.data_timestamp))"}],"source_content_type":"text/x-python","patch_set":16,"id":"b23790d8_ecd20dcc","line":6474,"in_reply_to":"617a98df_4597c12b","updated":"2024-04-18 13:55:29.000000000","message":"Done","commit_id":"b4e46ba5745ebefd3d2092686069e412eac222fa"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"1aecd6bd6100065be3fea37fac5052eeff0f525c","unresolved":true,"context_lines":[{"line_number":6471,"context_line":"    query \u003d ("},{"line_number":6472,"context_line":"        _backups_get_query(context\u003dcontext, joined_load\u003dFalse,"},{"line_number":6473,"context_line":"                           project_only\u003dTrue)"},{"line_number":6474,"context_line":"        .filter_by(project_id\u003dcontext.project_id)"},{"line_number":6475,"context_line":"        .filter_by(volume_id\u003dvolume_id)"},{"line_number":6476,"context_line":"        .filter_by(status\u003dfields.BackupStatus.AVAILABLE)"},{"line_number":6477,"context_line":"        .order_by(desc(models.Backup.data_timestamp))"}],"source_content_type":"text/x-python","patch_set":16,"id":"617a98df_4597c12b","line":6474,"in_reply_to":"668ced84_315f5895","updated":"2024-03-04 16:08:27.000000000","message":"It actually was, and there were tests failing.\nPlease see my comments about the last patchset uploads:\n\n* https://review.opendev.org/c/openstack/cinder/+/484729/comments/73593648_a53e338c\n* https://review.opendev.org/c/openstack/cinder/+/484729/comments/ecb1f16a_ad6fee0a\n\n\nIn short, even though I used \"project_only\u003dTrue\" on _backups_get_query, this does not apply when the query is done is admin context, see https://opendev.org/openstack/cinder/src/commit/20fe5b51f6aad2651b460dc43a48d743573ebd02/cinder/db/sqlalchemy/api.py#L315.\n\nAnd this is what https://review.opendev.org/c/openstack/cinder/+/720833 actually fixed.\n\n\nYou may argue filtering on project_id is now done twice. But I thought not setting \"project_only\u003dTrue\" would be even more confusing.","commit_id":"b4e46ba5745ebefd3d2092686069e412eac222fa"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"bfcfe9ccc39d15fe9d7b2a6e0ad275dedfb334b3","unresolved":true,"context_lines":[{"line_number":6473,"context_line":"                           project_only\u003dTrue)"},{"line_number":6474,"context_line":"        .filter_by(project_id\u003dcontext.project_id)"},{"line_number":6475,"context_line":"        .filter_by(volume_id\u003dvolume_id)"},{"line_number":6476,"context_line":"        .filter_by(status\u003dfields.BackupStatus.AVAILABLE)"},{"line_number":6477,"context_line":"        .order_by(desc(models.Backup.data_timestamp))"},{"line_number":6478,"context_line":"    )"},{"line_number":6479,"context_line":"    # NOTE(crohmann): In case a backup with a data_timestamp"}],"source_content_type":"text/x-python","patch_set":16,"id":"4dcdfdae_833c6173","line":6476,"updated":"2024-04-12 14:46:40.000000000","message":"FYI: If the last backup we should be using is being restored, then we would select the wrong backup as parent.","commit_id":"b4e46ba5745ebefd3d2092686069e412eac222fa"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"02596b14584f6976981bbeba864f641f277f1880","unresolved":false,"context_lines":[{"line_number":6473,"context_line":"                           project_only\u003dTrue)"},{"line_number":6474,"context_line":"        .filter_by(project_id\u003dcontext.project_id)"},{"line_number":6475,"context_line":"        .filter_by(volume_id\u003dvolume_id)"},{"line_number":6476,"context_line":"        .filter_by(status\u003dfields.BackupStatus.AVAILABLE)"},{"line_number":6477,"context_line":"        .order_by(desc(models.Backup.data_timestamp))"},{"line_number":6478,"context_line":"    )"},{"line_number":6479,"context_line":"    # NOTE(crohmann): In case a backup with a data_timestamp"}],"source_content_type":"text/x-python","patch_set":16,"id":"64802bd4_4ae1c60c","line":6476,"in_reply_to":"2b77d8e9_5ff594d7","updated":"2025-10-26 09:18:49.000000000","message":"Done","commit_id":"b4e46ba5745ebefd3d2092686069e412eac222fa"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"a376f36f5e95f0ab074230b264a110672c34a095","unresolved":true,"context_lines":[{"line_number":6473,"context_line":"                           project_only\u003dTrue)"},{"line_number":6474,"context_line":"        .filter_by(project_id\u003dcontext.project_id)"},{"line_number":6475,"context_line":"        .filter_by(volume_id\u003dvolume_id)"},{"line_number":6476,"context_line":"        .filter_by(status\u003dfields.BackupStatus.AVAILABLE)"},{"line_number":6477,"context_line":"        .order_by(desc(models.Backup.data_timestamp))"},{"line_number":6478,"context_line":"    )"},{"line_number":6479,"context_line":"    # NOTE(crohmann): In case a backup with a data_timestamp"}],"source_content_type":"text/x-python","patch_set":16,"id":"89a1927b_6fccc90e","line":6476,"in_reply_to":"4dcdfdae_833c6173","updated":"2024-04-18 14:44:17.000000000","message":"Should I add the RESTORING status to the list then then?","commit_id":"b4e46ba5745ebefd3d2092686069e412eac222fa"},{"author":{"_account_id":16137,"name":"Tobias Urdin","email":"tobias.urdin@binero.com","username":"tobasco"},"change_message_id":"f338550bbd6c935f4831cb3ab6f96c36b622e6ff","unresolved":true,"context_lines":[{"line_number":6473,"context_line":"                           project_only\u003dTrue)"},{"line_number":6474,"context_line":"        .filter_by(project_id\u003dcontext.project_id)"},{"line_number":6475,"context_line":"        .filter_by(volume_id\u003dvolume_id)"},{"line_number":6476,"context_line":"        .filter_by(status\u003dfields.BackupStatus.AVAILABLE)"},{"line_number":6477,"context_line":"        .order_by(desc(models.Backup.data_timestamp))"},{"line_number":6478,"context_line":"    )"},{"line_number":6479,"context_line":"    # NOTE(crohmann): In case a backup with a data_timestamp"}],"source_content_type":"text/x-python","patch_set":16,"id":"2b77d8e9_5ff594d7","line":6476,"in_reply_to":"8753d3c3_9b1622c2","updated":"2024-08-28 09:17:40.000000000","message":"Perhaps add a TODO in the code about that for the future?","commit_id":"b4e46ba5745ebefd3d2092686069e412eac222fa"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"8abddc1bbdda6a980aae045a6819e287ec271bff","unresolved":true,"context_lines":[{"line_number":6473,"context_line":"                           project_only\u003dTrue)"},{"line_number":6474,"context_line":"        .filter_by(project_id\u003dcontext.project_id)"},{"line_number":6475,"context_line":"        .filter_by(volume_id\u003dvolume_id)"},{"line_number":6476,"context_line":"        .filter_by(status\u003dfields.BackupStatus.AVAILABLE)"},{"line_number":6477,"context_line":"        .order_by(desc(models.Backup.data_timestamp))"},{"line_number":6478,"context_line":"    )"},{"line_number":6479,"context_line":"    # NOTE(crohmann): In case a backup with a data_timestamp"}],"source_content_type":"text/x-python","patch_set":16,"id":"8753d3c3_9b1622c2","line":6476,"in_reply_to":"89a1927b_6fccc90e","updated":"2024-04-19 15:38:47.000000000","message":"Gorka\u0027s comment makes sense to me but the old code only filtered by AVAILABLE. So this patch at least does not make it worse.","commit_id":"b4e46ba5745ebefd3d2092686069e412eac222fa"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"bfcfe9ccc39d15fe9d7b2a6e0ad275dedfb334b3","unresolved":true,"context_lines":[{"line_number":6479,"context_line":"    # NOTE(crohmann): In case a backup with a data_timestamp"},{"line_number":6480,"context_line":"    # before a certain time is required add this condition."},{"line_number":6481,"context_line":"    if beforeDataTimestamp:"},{"line_number":6482,"context_line":"        query.filter("},{"line_number":6483,"context_line":"            models.Backup.data_timestamp \u003c beforeDataTimestamp"},{"line_number":6484,"context_line":"        )"},{"line_number":6485,"context_line":"    return query.first()"},{"line_number":6486,"context_line":""},{"line_number":6487,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"556115dc_8ccdc0ad","line":6484,"range":{"start_line":6482,"start_character":1,"end_line":6484,"end_character":9},"updated":"2024-04-12 14:46:40.000000000","message":"-1: Needs to be assigned to the query.\n\n```\nquery \u003d query.filter\n```","commit_id":"b4e46ba5745ebefd3d2092686069e412eac222fa"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"c593252de9d89dfee4c150eee5544bb7ad15191c","unresolved":false,"context_lines":[{"line_number":6479,"context_line":"    # NOTE(crohmann): In case a backup with a data_timestamp"},{"line_number":6480,"context_line":"    # before a certain time is required add this condition."},{"line_number":6481,"context_line":"    if beforeDataTimestamp:"},{"line_number":6482,"context_line":"        query.filter("},{"line_number":6483,"context_line":"            models.Backup.data_timestamp \u003c beforeDataTimestamp"},{"line_number":6484,"context_line":"        )"},{"line_number":6485,"context_line":"    return query.first()"},{"line_number":6486,"context_line":""},{"line_number":6487,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"520c815b_0d1969a9","line":6484,"range":{"start_line":6482,"start_character":1,"end_line":6484,"end_character":9},"in_reply_to":"556115dc_8ccdc0ad","updated":"2024-04-12 14:53:38.000000000","message":"Acknowledged","commit_id":"b4e46ba5745ebefd3d2092686069e412eac222fa"}],"cinder/objects/backup.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a59530b986328cd86eb633d57536c7a5fc08f09f","unresolved":true,"context_lines":[{"line_number":229,"context_line":"                                                         beforeDataTimestamp)"},{"line_number":230,"context_line":"        if db_backup:"},{"line_number":231,"context_line":"            return cls._from_db_object(context, objects.Backup(), db_backup)"},{"line_number":232,"context_line":"        else:"},{"line_number":233,"context_line":"            return None"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"6e626a5a_298ee032","line":232,"range":{"start_line":232,"start_character":8,"end_line":232,"end_character":12},"updated":"2024-04-12 15:00:39.000000000","message":"nit: This else is not necessary, because on L231 we are returning","commit_id":"3d9b6bf0846531e190c0f4f8758cdf62686e27db"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"9dfdd4fd568d9d22f7faa7dcf4ddcdf8294aabfe","unresolved":false,"context_lines":[{"line_number":229,"context_line":"                                                         beforeDataTimestamp)"},{"line_number":230,"context_line":"        if db_backup:"},{"line_number":231,"context_line":"            return cls._from_db_object(context, objects.Backup(), db_backup)"},{"line_number":232,"context_line":"        else:"},{"line_number":233,"context_line":"            return None"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"30bfab55_79e6ca9a","line":232,"range":{"start_line":232,"start_character":8,"end_line":232,"end_character":12},"in_reply_to":"6e626a5a_298ee032","updated":"2024-04-16 08:20:55.000000000","message":"Acknowledged","commit_id":"3d9b6bf0846531e190c0f4f8758cdf62686e27db"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"aa71698bff1b6b9c99e6dba1a2431a43397ac4c2","unresolved":true,"context_lines":[{"line_number":224,"context_line":"        return base64.encode_as_text(retval)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"    @classmethod"},{"line_number":227,"context_line":"    def get_parent_for_incremental(cls,"},{"line_number":228,"context_line":"                                   context,"},{"line_number":229,"context_line":"                                   volume_id,"},{"line_number":230,"context_line":"                                   volume_project_id,"}],"source_content_type":"text/x-python","patch_set":33,"id":"9d52ed79_06a38c9a","line":227,"updated":"2026-08-07 13:48:47.000000000","message":"Minor, latent trap: this calls _from_db_object without expected_attrs, and the\nDB query uses joined_load\u003dFalse, so \u0027metadata\u0027 is never set on the returned\nobject. Backup.obj_load_attr only knows how to lazy-load \u0027parent\u0027 — for\n\u0027metadata\u0027 it falls through and resets changes without setting anything, so\nany future parent.metadata access raises rather than loading.\n\nNothing reads it today (manager.py only uses backup.parent_id), so this isn\u0027t\na bug now, but it is a behaviour change from the old path via\nBackupList.get_all_by_volume, which did load metadata. Worth a one-line\ncomment saying metadata is intentionally not loaded.","commit_id":"97d48fd8b1970585cf6ada25926d5e7295731ebb"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"2c4bd71021703f17f56bf54e1301647e8956edba","unresolved":false,"context_lines":[{"line_number":224,"context_line":"        return base64.encode_as_text(retval)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"    @classmethod"},{"line_number":227,"context_line":"    def get_parent_for_incremental(cls,"},{"line_number":228,"context_line":"                                   context,"},{"line_number":229,"context_line":"                                   volume_id,"},{"line_number":230,"context_line":"                                   volume_project_id,"}],"source_content_type":"text/x-python","patch_set":33,"id":"4d7cee8b_4d357153","line":227,"in_reply_to":"9d52ed79_06a38c9a","updated":"2026-08-18 14:13:49.000000000","message":"Done\n\nI added a comment","commit_id":"97d48fd8b1970585cf6ada25926d5e7295731ebb"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"092122d1194fc0968c78bdc6b447b9997a86e106","unresolved":true,"context_lines":[{"line_number":223,"context_line":"        retval \u003d jsonutils.dump_as_bytes(kwargs)"},{"line_number":224,"context_line":"        return base64.encode_as_text(retval)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"    @classmethod"},{"line_number":227,"context_line":"    # The returned backup object is intentially without metadata"},{"line_number":228,"context_line":"    # (joined_load\u003dFalse). The backup manager only needs the id of parent"},{"line_number":229,"context_line":"    def get_parent_for_incremental(cls,"}],"source_content_type":"text/x-python","patch_set":36,"id":"c94b31e6_c59eed49","line":226,"updated":"2026-08-18 15:27:51.000000000","message":"I would put the classmethod below the comment, so the comment reads as belonging to the whole section","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"7351beb8b1a9afb1f47f7dbbc6a3892f326c06e3","unresolved":false,"context_lines":[{"line_number":223,"context_line":"        retval \u003d jsonutils.dump_as_bytes(kwargs)"},{"line_number":224,"context_line":"        return base64.encode_as_text(retval)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"    @classmethod"},{"line_number":227,"context_line":"    # The returned backup object is intentially without metadata"},{"line_number":228,"context_line":"    # (joined_load\u003dFalse). The backup manager only needs the id of parent"},{"line_number":229,"context_line":"    def get_parent_for_incremental(cls,"}],"source_content_type":"text/x-python","patch_set":36,"id":"3d6a7cb7_387b6a1a","line":226,"in_reply_to":"c94b31e6_c59eed49","updated":"2026-09-02 07:49:40.000000000","message":"Done","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"092122d1194fc0968c78bdc6b447b9997a86e106","unresolved":true,"context_lines":[{"line_number":224,"context_line":"        return base64.encode_as_text(retval)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"    @classmethod"},{"line_number":227,"context_line":"    # The returned backup object is intentially without metadata"},{"line_number":228,"context_line":"    # (joined_load\u003dFalse). The backup manager only needs the id of parent"},{"line_number":229,"context_line":"    def get_parent_for_incremental(cls,"},{"line_number":230,"context_line":"                                   context,"}],"source_content_type":"text/x-python","patch_set":36,"id":"56a32c0d_2090bb92","line":227,"range":{"start_line":227,"start_character":36,"end_line":227,"end_character":48},"updated":"2026-08-18 15:27:51.000000000","message":"nit: typo - intentionally","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"7351beb8b1a9afb1f47f7dbbc6a3892f326c06e3","unresolved":false,"context_lines":[{"line_number":224,"context_line":"        return base64.encode_as_text(retval)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"    @classmethod"},{"line_number":227,"context_line":"    # The returned backup object is intentially without metadata"},{"line_number":228,"context_line":"    # (joined_load\u003dFalse). The backup manager only needs the id of parent"},{"line_number":229,"context_line":"    def get_parent_for_incremental(cls,"},{"line_number":230,"context_line":"                                   context,"}],"source_content_type":"text/x-python","patch_set":36,"id":"f5dea54e_90b861d6","line":227,"range":{"start_line":227,"start_character":36,"end_line":227,"end_character":48},"in_reply_to":"56a32c0d_2090bb92","updated":"2026-09-02 07:49:40.000000000","message":"Done","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"092122d1194fc0968c78bdc6b447b9997a86e106","unresolved":true,"context_lines":[{"line_number":225,"context_line":""},{"line_number":226,"context_line":"    @classmethod"},{"line_number":227,"context_line":"    # The returned backup object is intentially without metadata"},{"line_number":228,"context_line":"    # (joined_load\u003dFalse). The backup manager only needs the id of parent"},{"line_number":229,"context_line":"    def get_parent_for_incremental(cls,"},{"line_number":230,"context_line":"                                   context,"},{"line_number":231,"context_line":"                                   volume_id,"}],"source_content_type":"text/x-python","patch_set":36,"id":"731f9133_de98b836","line":228,"range":{"start_line":228,"start_character":67,"end_line":228,"end_character":73},"updated":"2026-08-18 15:27:51.000000000","message":"this sentence feels incomplete","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"7351beb8b1a9afb1f47f7dbbc6a3892f326c06e3","unresolved":false,"context_lines":[{"line_number":225,"context_line":""},{"line_number":226,"context_line":"    @classmethod"},{"line_number":227,"context_line":"    # The returned backup object is intentially without metadata"},{"line_number":228,"context_line":"    # (joined_load\u003dFalse). The backup manager only needs the id of parent"},{"line_number":229,"context_line":"    def get_parent_for_incremental(cls,"},{"line_number":230,"context_line":"                                   context,"},{"line_number":231,"context_line":"                                   volume_id,"}],"source_content_type":"text/x-python","patch_set":36,"id":"fec6f15f_f51228a0","line":228,"range":{"start_line":228,"start_character":67,"end_line":228,"end_character":73},"in_reply_to":"731f9133_de98b836","updated":"2026-09-02 07:49:40.000000000","message":"Done","commit_id":"5e843ea8282137390ac7947c933da256419353b2"}],"cinder/tests/unit/db/test_migrations.py":[{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"e97276d28ad1c6d737993c4e1b76539512e32c43","unresolved":false,"context_lines":[{"line_number":443,"context_line":"    def _check_385d1313da9a(self, connection):"},{"line_number":444,"context_line":"        \"\"\"Test backups as index on volume_id and data_timestamp.\"\"\""},{"line_number":445,"context_line":"        db_utils.index_exists(connection,"},{"line_number":446,"context_line":"                              \u0027backups\u0027, \u0027backups_volume_id_data_timestamp_idx\u0027)"},{"line_number":447,"context_line":""},{"line_number":448,"context_line":""},{"line_number":449,"context_line":"    # TODO: (D Release) Uncomment method _check_afd7494d43b7 and create a"}],"source_content_type":"text/x-python","patch_set":40,"id":"c7943cae_d56e5989","line":446,"updated":"2026-09-02 12:28:40.000000000","message":"pep8: E501 line too long (80 \u003e 79 characters)","commit_id":"0030b7b79082022a71a2c43cf6383991e63f9003"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"e97276d28ad1c6d737993c4e1b76539512e32c43","unresolved":false,"context_lines":[{"line_number":446,"context_line":"                              \u0027backups\u0027, \u0027backups_volume_id_data_timestamp_idx\u0027)"},{"line_number":447,"context_line":""},{"line_number":448,"context_line":""},{"line_number":449,"context_line":"    # TODO: (D Release) Uncomment method _check_afd7494d43b7 and create a"},{"line_number":450,"context_line":"    # migration with hash afd7494d43b7 using the following command:"},{"line_number":451,"context_line":"    #   $ tox -e venv -- alembic -c cinder/db/alembic.ini revision \\"},{"line_number":452,"context_line":"    #     --rev-id afd7494d43b7  -m \u0027drop quota leftovers\u0027"}],"source_content_type":"text/x-python","patch_set":40,"id":"b34bf51f_42af0471","line":449,"updated":"2026-09-02 12:28:40.000000000","message":"pep8: E303 too many blank lines (2)","commit_id":"0030b7b79082022a71a2c43cf6383991e63f9003"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"c34db7071d3d020715b1c3bc7387aa245a493189","unresolved":true,"context_lines":[{"line_number":442,"context_line":""},{"line_number":443,"context_line":"    def _check_385d1313da9a(self, connection):"},{"line_number":444,"context_line":"        \"\"\"Test backups as index on volume_id and data_timestamp.\"\"\""},{"line_number":445,"context_line":"        db_utils.index_exists(connection,"},{"line_number":446,"context_line":"                              \u0027backups\u0027,"},{"line_number":447,"context_line":"                              \u0027backups_volume_id_data_timestamp_idx\u0027)"},{"line_number":448,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"21aa1241_51deda61","line":445,"updated":"2026-09-03 12:52:51.000000000","message":"nit: `db_utils.index_exists()` returns a bool and the result is discarded here, so\nthis check asserts nothing and would pass even if the migration hadn\u0027t created\nthe index. `self.assertTrue(db_utils.index_exists(...))` would make it real.\n\nIn fairness, the existing `_check_daa98075b90d` just above does the same thing,\nso you\u0027re following local precedent rather than breaking it — feel free to\nleave it and let someone fix both, or fix yours and leave the older one alone.","commit_id":"72fec9c71d666ed471ee9a0ae16a036e55f6cd0b"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"6b35acc600a7eac0a22c378c4dccca5e50355b25","unresolved":false,"context_lines":[{"line_number":442,"context_line":""},{"line_number":443,"context_line":"    def _check_385d1313da9a(self, connection):"},{"line_number":444,"context_line":"        \"\"\"Test backups as index on volume_id and data_timestamp.\"\"\""},{"line_number":445,"context_line":"        db_utils.index_exists(connection,"},{"line_number":446,"context_line":"                              \u0027backups\u0027,"},{"line_number":447,"context_line":"                              \u0027backups_volume_id_data_timestamp_idx\u0027)"},{"line_number":448,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"18860b09_d5f0d5a3","line":445,"in_reply_to":"21aa1241_51deda61","updated":"2026-09-04 15:29:46.000000000","message":"Done","commit_id":"72fec9c71d666ed471ee9a0ae16a036e55f6cd0b"}],"cinder/tests/unit/test_db_api.py":[{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"1f922bbf6373e6c84c1860c16528fb6a510609ad","unresolved":true,"context_lines":[{"line_number":3358,"context_line":"                \"volume_id\": fake.VOLUME_ID,"},{"line_number":3359,"context_line":"                \"size\": 1,"},{"line_number":3360,"context_line":"                \"object_count\": 1,"},{"line_number":3361,"context_line":"                \"status\": \"error\","},{"line_number":3362,"context_line":"                \"data_timestamp\": UTC_NOW + datetime.timedelta(seconds\u003d1),"},{"line_number":3363,"context_line":"            },"},{"line_number":3364,"context_line":"            \"new_backup_params\": {"}],"source_content_type":"text/x-python","patch_set":32,"id":"ca43c287_6a57592a","line":3361,"range":{"start_line":3361,"start_character":16,"end_line":3361,"end_character":34},"updated":"2026-07-13 18:38:45.000000000","message":"This case sets status\u003d\"error\" and a too-young data_timestamp, so the row is already excluded by status — the data_timestamp \u003c before_data_timestamp filter is never actually exercised (the assertion would still pass if that filter were removed). Change this to \"status\": \"available\" so the age filter is the sole exclusion reason it\u0027s meant to verify.","commit_id":"7b89aba74d5f17a0470cc3847bd366201c618533"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"448f499bb292ccf30aaad8522491d249a7019b2d","unresolved":false,"context_lines":[{"line_number":3358,"context_line":"                \"volume_id\": fake.VOLUME_ID,"},{"line_number":3359,"context_line":"                \"size\": 1,"},{"line_number":3360,"context_line":"                \"object_count\": 1,"},{"line_number":3361,"context_line":"                \"status\": \"error\","},{"line_number":3362,"context_line":"                \"data_timestamp\": UTC_NOW + datetime.timedelta(seconds\u003d1),"},{"line_number":3363,"context_line":"            },"},{"line_number":3364,"context_line":"            \"new_backup_params\": {"}],"source_content_type":"text/x-python","patch_set":32,"id":"ad147fd9_7dc35a40","line":3361,"range":{"start_line":3361,"start_character":16,"end_line":3361,"end_character":34},"in_reply_to":"ca43c287_6a57592a","updated":"2026-07-16 13:27:21.000000000","message":"Acknowledged","commit_id":"7b89aba74d5f17a0470cc3847bd366201c618533"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"1f922bbf6373e6c84c1860c16528fb6a510609ad","unresolved":true,"context_lines":[{"line_number":3359,"context_line":"                \"size\": 1,"},{"line_number":3360,"context_line":"                \"object_count\": 1,"},{"line_number":3361,"context_line":"                \"status\": \"error\","},{"line_number":3362,"context_line":"                \"data_timestamp\": UTC_NOW + datetime.timedelta(seconds\u003d1),"},{"line_number":3363,"context_line":"            },"},{"line_number":3364,"context_line":"            \"new_backup_params\": {"},{"line_number":3365,"context_line":"                \"project_id\": fake.PROJECT_ID,"}],"source_content_type":"text/x-python","patch_set":32,"id":"aab8bfaf_a6253f0b","line":3362,"range":{"start_line":3362,"start_character":16,"end_line":3362,"end_character":74},"updated":"2026-07-13 18:38:45.000000000","message":"UTC_NOW (line 45) is naive, while the positive test uses timeutils.utcnow(with_timezone\u003dTrue) (line 3401). Once the case above uses available, the age comparison will actually run and a naive-vs-aware data_timestamp comparison may raise. Please make these timestamps consistently tz-aware.","commit_id":"7b89aba74d5f17a0470cc3847bd366201c618533"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"448f499bb292ccf30aaad8522491d249a7019b2d","unresolved":false,"context_lines":[{"line_number":3359,"context_line":"                \"size\": 1,"},{"line_number":3360,"context_line":"                \"object_count\": 1,"},{"line_number":3361,"context_line":"                \"status\": \"error\","},{"line_number":3362,"context_line":"                \"data_timestamp\": UTC_NOW + datetime.timedelta(seconds\u003d1),"},{"line_number":3363,"context_line":"            },"},{"line_number":3364,"context_line":"            \"new_backup_params\": {"},{"line_number":3365,"context_line":"                \"project_id\": fake.PROJECT_ID,"}],"source_content_type":"text/x-python","patch_set":32,"id":"502da601_cd86cf4d","line":3362,"range":{"start_line":3362,"start_character":16,"end_line":3362,"end_character":74},"in_reply_to":"aab8bfaf_a6253f0b","updated":"2026-07-16 13:27:21.000000000","message":"Acknowledged","commit_id":"7b89aba74d5f17a0470cc3847bd366201c618533"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"1f922bbf6373e6c84c1860c16528fb6a510609ad","unresolved":true,"context_lines":[{"line_number":3396,"context_line":"        )"},{"line_number":3397,"context_line":""},{"line_number":3398,"context_line":"    @mock.patch.object(sqlalchemy_api, \"authorize_project_context\")"},{"line_number":3399,"context_line":"    def test_backup_get_parent_for_incremental_positive(self, mock_authorize):"},{"line_number":3400,"context_line":""},{"line_number":3401,"context_line":"        UTC_NOW_TZ \u003d timeutils.utcnow(with_timezone\u003dTrue)"},{"line_number":3402,"context_line":"        user_context \u003d context.RequestContext("}],"source_content_type":"text/x-python","patch_set":32,"id":"8e82277f_debb4f72","line":3399,"range":{"start_line":3399,"start_character":4,"end_line":3399,"end_character":78},"updated":"2026-07-13 18:38:45.000000000","message":"The positive test only walks a chain where the newest backup is always available, which doesn\u0027t cover what actually distinguishes this query from the old code. Please add:\n\n1. newest backup is creating/error → query must fall back to the previous available one (the core correctness property);\n2. before_data_timestamp/snapshot path selecting an older backup that is also available (current assert checks only the timestamp bound, not status);\n3. a RESTORING positive case if that status is retained (ties to the db/api.py comment).","commit_id":"7b89aba74d5f17a0470cc3847bd366201c618533"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"448f499bb292ccf30aaad8522491d249a7019b2d","unresolved":false,"context_lines":[{"line_number":3396,"context_line":"        )"},{"line_number":3397,"context_line":""},{"line_number":3398,"context_line":"    @mock.patch.object(sqlalchemy_api, \"authorize_project_context\")"},{"line_number":3399,"context_line":"    def test_backup_get_parent_for_incremental_positive(self, mock_authorize):"},{"line_number":3400,"context_line":""},{"line_number":3401,"context_line":"        UTC_NOW_TZ \u003d timeutils.utcnow(with_timezone\u003dTrue)"},{"line_number":3402,"context_line":"        user_context \u003d context.RequestContext("}],"source_content_type":"text/x-python","patch_set":32,"id":"e1170d98_07b3b0ed","line":3399,"range":{"start_line":3399,"start_character":4,"end_line":3399,"end_character":78},"in_reply_to":"8e82277f_debb4f72","updated":"2026-07-16 13:27:21.000000000","message":"Acknowledged","commit_id":"7b89aba74d5f17a0470cc3847bd366201c618533"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"aa71698bff1b6b9c99e6dba1a2431a43397ac4c2","unresolved":true,"context_lines":[{"line_number":3427,"context_line":"    )"},{"line_number":3428,"context_line":"    @ddt.unpack"},{"line_number":3429,"context_line":"    @mock.patch.object(sqlalchemy_api, \"authorize_project_context\")"},{"line_number":3430,"context_line":"    def test_backup_get_parent_for_incremental_positive(self, mock_authorize,"},{"line_number":3431,"context_line":"                                                        status):"},{"line_number":3432,"context_line":""},{"line_number":3433,"context_line":"        UTC_NOW_TZ \u003d timeutils.utcnow(with_timezone\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":33,"id":"6340f44a_a2e0e8ed","line":3430,"updated":"2026-08-07 13:48:47.000000000","message":"Reopening the PS32 point here as it still isn\u0027t covered.\n\nEvery negative ddt case creates exactly one backup, and the positive test\nbuilds a homogeneous chain (all available, or all restoring). There\u0027s still no\ncase where the newest backup is creating/error and an older available one\nexists, asserting the query falls back to the older one. That\u0027s the core\nproperty the old max()-with-sentinel expression provided, and it\u0027s the one\nthing that most needs pinning against future changes to this query.\n\nRelated, the assertion above:\n\n  self.assertIn(parent_backup[\"status\"], [\"available\", \"restoring\"])\n\nis a tautology as written — every backup in that chain has the same status by\nconstruction, so it can\u0027t fail. Mixing statuses in the chain would make both\nthis and the before_data_timestamp bound meaningful.","commit_id":"97d48fd8b1970585cf6ada25926d5e7295731ebb"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"2c4bd71021703f17f56bf54e1301647e8956edba","unresolved":false,"context_lines":[{"line_number":3427,"context_line":"    )"},{"line_number":3428,"context_line":"    @ddt.unpack"},{"line_number":3429,"context_line":"    @mock.patch.object(sqlalchemy_api, \"authorize_project_context\")"},{"line_number":3430,"context_line":"    def test_backup_get_parent_for_incremental_positive(self, mock_authorize,"},{"line_number":3431,"context_line":"                                                        status):"},{"line_number":3432,"context_line":""},{"line_number":3433,"context_line":"        UTC_NOW_TZ \u003d timeutils.utcnow(with_timezone\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":33,"id":"2bf128d6_24da6110","line":3430,"in_reply_to":"6340f44a_a2e0e8ed","updated":"2026-08-18 14:13:49.000000000","message":"Done\n\nI read you were happy with the negative tests?\n\nI updated the positives to now take a whole status history as ddt to create more complex history scenarios. PTAL if I covered the usual suspects.","commit_id":"97d48fd8b1970585cf6ada25926d5e7295731ebb"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"24e6442df670dd76c1b613d577297ca249768d09","unresolved":true,"context_lines":[{"line_number":3485,"context_line":"            parent_backup[\"data_timestamp\"],"},{"line_number":3486,"context_line":"            UTC_NOW_TZ + datetime.timedelta(hours\u003d2)"},{"line_number":3487,"context_line":"        )"},{"line_number":3488,"context_line":"        self.assertIn("},{"line_number":3489,"context_line":"            parent_backup[\"status\"],"},{"line_number":3490,"context_line":"            [\"available\", \"restoring\"]"},{"line_number":3491,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":35,"id":"7f2bc53d_740cf8ce","line":3488,"updated":"2026-08-14 00:05:10.000000000","message":"tautology - see comment above","commit_id":"5b0e05c6116516fd26dcecab3c39c53ff787786b"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"2c4bd71021703f17f56bf54e1301647e8956edba","unresolved":false,"context_lines":[{"line_number":3485,"context_line":"            parent_backup[\"data_timestamp\"],"},{"line_number":3486,"context_line":"            UTC_NOW_TZ + datetime.timedelta(hours\u003d2)"},{"line_number":3487,"context_line":"        )"},{"line_number":3488,"context_line":"        self.assertIn("},{"line_number":3489,"context_line":"            parent_backup[\"status\"],"},{"line_number":3490,"context_line":"            [\"available\", \"restoring\"]"},{"line_number":3491,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":35,"id":"1e6c0b1f_ddbe63dd","line":3488,"in_reply_to":"7f2bc53d_740cf8ce","updated":"2026-08-18 14:13:49.000000000","message":"Done","commit_id":"5b0e05c6116516fd26dcecab3c39c53ff787786b"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"092122d1194fc0968c78bdc6b447b9997a86e106","unresolved":true,"context_lines":[{"line_number":3462,"context_line":"        for idx, backup in enumerate(backup_history, start\u003d1):"},{"line_number":3463,"context_line":"            new_backup_params \u003d {"},{"line_number":3464,"context_line":"                \"display_name\": f\"Backup generation {idx} with \""},{"line_number":3465,"context_line":"                                f\"status {backup[\"status\"]} and data from \""},{"line_number":3466,"context_line":"                                f\"{backup[\"data_age_offset_hours\"]} hours ago\","},{"line_number":3467,"context_line":"                \"project_id\": fake.PROJECT_ID,"},{"line_number":3468,"context_line":"                \"user_id\": fake.USER_ID,"}],"source_content_type":"text/x-python","patch_set":36,"id":"356da7b5_1803fa0f","line":3465,"updated":"2026-08-18 15:27:51.000000000","message":"Py311 syntax error with the \"\n```\n  f\"status {backup[\u0027status\u0027]} and data from \"\n  f\"{backup[\u0027data_age_offset_hours\u0027]} hours ago\",\n```\nshould fix it","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"7351beb8b1a9afb1f47f7dbbc6a3892f326c06e3","unresolved":false,"context_lines":[{"line_number":3462,"context_line":"        for idx, backup in enumerate(backup_history, start\u003d1):"},{"line_number":3463,"context_line":"            new_backup_params \u003d {"},{"line_number":3464,"context_line":"                \"display_name\": f\"Backup generation {idx} with \""},{"line_number":3465,"context_line":"                                f\"status {backup[\"status\"]} and data from \""},{"line_number":3466,"context_line":"                                f\"{backup[\"data_age_offset_hours\"]} hours ago\","},{"line_number":3467,"context_line":"                \"project_id\": fake.PROJECT_ID,"},{"line_number":3468,"context_line":"                \"user_id\": fake.USER_ID,"}],"source_content_type":"text/x-python","patch_set":36,"id":"4b82a860_133c40af","line":3465,"in_reply_to":"356da7b5_1803fa0f","updated":"2026-09-02 07:49:40.000000000","message":"Done","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"092122d1194fc0968c78bdc6b447b9997a86e106","unresolved":true,"context_lines":[{"line_number":3492,"context_line":"        )"},{"line_number":3493,"context_line":""},{"line_number":3494,"context_line":"        # parent has data_timestamp before new backup"},{"line_number":3495,"context_line":"        self.assertLessEqual("},{"line_number":3496,"context_line":"            parent_backup[\"data_timestamp\"],"},{"line_number":3497,"context_line":"            UTC_NOW_TZ"},{"line_number":3498,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":36,"id":"e69c0ba1_ca261dcc","line":3495,"updated":"2026-08-18 15:27:51.000000000","message":"Assestions not upgraded to match the data setup.\n\nAdd one more assertion:\n`self.assertEqual(created[expected_idx].id, parent_backup.id)`","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"6b35acc600a7eac0a22c378c4dccca5e50355b25","unresolved":false,"context_lines":[{"line_number":3492,"context_line":"        )"},{"line_number":3493,"context_line":""},{"line_number":3494,"context_line":"        # parent has data_timestamp before new backup"},{"line_number":3495,"context_line":"        self.assertLessEqual("},{"line_number":3496,"context_line":"            parent_backup[\"data_timestamp\"],"},{"line_number":3497,"context_line":"            UTC_NOW_TZ"},{"line_number":3498,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":36,"id":"1e384e32_baaecef5","line":3495,"in_reply_to":"063e3261_a43f43ea","updated":"2026-09-04 15:29:46.000000000","message":"Done","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"c34db7071d3d020715b1c3bc7387aa245a493189","unresolved":true,"context_lines":[{"line_number":3492,"context_line":"        )"},{"line_number":3493,"context_line":""},{"line_number":3494,"context_line":"        # parent has data_timestamp before new backup"},{"line_number":3495,"context_line":"        self.assertLessEqual("},{"line_number":3496,"context_line":"            parent_backup[\"data_timestamp\"],"},{"line_number":3497,"context_line":"            UTC_NOW_TZ"},{"line_number":3498,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":36,"id":"063e3261_a43f43ea","line":3495,"in_reply_to":"90aa43b4_6fd68951","updated":"2026-09-03 12:52:51.000000000","message":"Not quite — nothing here creates a new backup through the API, so parent_id\nisn\u0027t the thing to check. What I\u0027m after is *which row the query returned*.\n\nRight now the two assertions only restate the query\u0027s own WHERE clause:\n\n    assertLessEqual(parent_backup[\"data_timestamp\"], UTC_NOW_TZ)\n        -\u003e guaranteed by the data_timestamp \u003c before_data_timestamp filter\n    assertIn(parent_backup[\"status\"], [\"available\", \"restoring\"])\n        -\u003e guaranteed by the status.in_(...) filter\n\nSo neither can fail, and neither checks that the *most recent eligible* row\nwon. Concretely, case 1 is\n\n    available@-99h, error@-88h, error@-77h, available@-10h\n\nand the correct answer is the -10h backup. But the test passes just as well if\nthe query returns the -99h one, i.e. if the ORDER BY were dropped or reversed.\n\nConcretely: collect the backups as you create them, and put the expected index\nin each ddt case:\n\n    created \u003d []\n    for idx, backup in enumerate(backup_history, start\u003d1):\n        ...\n        new_backup.create()\n        created.append(new_backup)\n    ...\n    self.assertEqual(created[expected_idx].id, parent_backup.id)\n\nwith expected_idx \u003d 3 for case 1, 0 for case 2, and 2 for case 3 (the -1h\nfuture-dated one being excluded by the age filter). That pins the \"newest\neligible parent wins, skipping ineligible newer ones\" property, which is the\nbehaviour the old max()-with-sentinel expression provided.","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"7351beb8b1a9afb1f47f7dbbc6a3892f326c06e3","unresolved":true,"context_lines":[{"line_number":3492,"context_line":"        )"},{"line_number":3493,"context_line":""},{"line_number":3494,"context_line":"        # parent has data_timestamp before new backup"},{"line_number":3495,"context_line":"        self.assertLessEqual("},{"line_number":3496,"context_line":"            parent_backup[\"data_timestamp\"],"},{"line_number":3497,"context_line":"            UTC_NOW_TZ"},{"line_number":3498,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":36,"id":"90aa43b4_6fd68951","line":3495,"in_reply_to":"e69c0ba1_ca261dcc","updated":"2026-09-02 07:49:40.000000000","message":"Sorry  I don\u0027t yet get this. Can you explain this missing test little more? What is this testing? Do you want to asset that the newly created backup has the determined parent backup as id? Like so  ...\n\nself.assertEqual(self.created[0].parent_id, parent_backup[\"id\"])","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"c34db7071d3d020715b1c3bc7387aa245a493189","unresolved":true,"context_lines":[{"line_number":3396,"context_line":"        self, mock_authorize, existing_backup_params, new_backup_params"},{"line_number":3397,"context_line":"    ):"},{"line_number":3398,"context_line":""},{"line_number":3399,"context_line":"        user_context \u003d context.RequestContext("},{"line_number":3400,"context_line":"            fake.USER_ID, fake.PROJECT_ID, is_admin\u003dFalse"},{"line_number":3401,"context_line":"        )"},{"line_number":3402,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"0d77d8af_d9242e17","line":3399,"updated":"2026-09-03 12:52:51.000000000","message":"This is why the unit tests didn\u0027t catch the regression in `cinder/db/api.py`: every negative ddt case runs the query as\n\n    context.RequestContext(fake.USER_ID, fake.PROJECT_ID, is_admin\u003dFalse)\n\nand self.ctxt (admin) is only used to *create* the fixtures. So the \"different\nproject id\" case passes on the project_only filter, and the one code path that\nactually regressed — an admin context, where project_only does nothing — is\nnever exercised.\n\nPlease add a negative case that runs the query with an admin context whose\nproject differs from the volume\u0027s, and asserts None is returned. That test\nwould have failed on PS37 and is the thing that keeps this from regressing\nagain.","commit_id":"72fec9c71d666ed471ee9a0ae16a036e55f6cd0b"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"6b35acc600a7eac0a22c378c4dccca5e50355b25","unresolved":false,"context_lines":[{"line_number":3396,"context_line":"        self, mock_authorize, existing_backup_params, new_backup_params"},{"line_number":3397,"context_line":"    ):"},{"line_number":3398,"context_line":""},{"line_number":3399,"context_line":"        user_context \u003d context.RequestContext("},{"line_number":3400,"context_line":"            fake.USER_ID, fake.PROJECT_ID, is_admin\u003dFalse"},{"line_number":3401,"context_line":"        )"},{"line_number":3402,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"2047067d_aed8ad17","line":3399,"in_reply_to":"0d77d8af_d9242e17","updated":"2026-09-04 15:29:46.000000000","message":"Done","commit_id":"72fec9c71d666ed471ee9a0ae16a036e55f6cd0b"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"9d23875e53ddb80cf87cb7428539de6db0862dba","unresolved":true,"context_lines":[{"line_number":3419,"context_line":""},{"line_number":3420,"context_line":"        # Test as admin again to ensure cross-project isolation is maintained"},{"line_number":3421,"context_line":"        parent_backup \u003d objects.Backup.get_parent_for_incremental("},{"line_number":3422,"context_line":"            context\u003dself.ctxt,"},{"line_number":3423,"context_line":"            volume_id\u003dnew_backup_params[\"volume_id\"],"},{"line_number":3424,"context_line":"            volume_project_id\u003dnew_backup_params[\"project_id\"],"},{"line_number":3425,"context_line":"            before_data_timestamp\u003dnew_backup_params[\"before_data_timestamp\"],"}],"source_content_type":"text/x-python","patch_set":43,"id":"241c7e10_f0709785","line":3422,"updated":"2026-09-04 18:30:04.000000000","message":"Strengthening suggestion, not a defect — this case does its job as written.\n\nself.ctxt comes from BaseTest.setUp as context.get_admin_context(), which is\nRequestContext(user_id\u003dNone, project_id\u003dNone, is_admin\u003dTrue). So\nfilter_by(project_id\u003dcontext.project_id) compiles to \"project_id IS NULL\", and\nno fixture row has a NULL project — every case returns None partly by\ndegeneracy.\n\nIt still catches the PS37 regression (without the explicit filter, case 1\u0027s\nPROJECT2_ID backup matches on volume_id + available and comes back), so this is\na genuine guard. But it doesn\u0027t model the situation the tempest tests actually\nhit: an admin holding a real token scoped to a *different* project. Something\nlike\n\n    admin_context \u003d context.RequestContext(\n        fake.USER_ID, fake.PROJECT3_ID, is_admin\u003dTrue)\n\nwould be closer. fake.PROJECT3_ID already exists in fake_constants, and using\nit avoids colliding with case 1\u0027s PROJECT2_ID fixture. It would also catch a\nhypothetical filter_by(project_id\u003dvolume_project_id) mistake, which the IS NULL\nform can\u0027t distinguish.\n\nHappy for this to be a follow-up rather than another patchset.","commit_id":"ed4a49cc5ca31c9c4045b184ce1825ab42e2d1b9"}],"releasenotes/notes/bug1696715-dfb21549c4238b88.yaml":[{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"1f922bbf6373e6c84c1860c16528fb6a510609ad","unresolved":true,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    `Bug #1696715 \u003chttps://bugs.launchpad.net/cinder/+bug/1696715\u003e`_:"},{"line_number":5,"context_line":"    Optimize getting parent backup for new incremental by not fetching all"},{"line_number":6,"context_line":"    backups from the database, but actually select the suitable parent."}],"source_content_type":"text/x-yaml","patch_set":32,"id":"afea144b_a2626c00","line":6,"updated":"2026-07-13 18:38:45.000000000","message":"If the RESTORING-as-eligible-parent change is kept, please mention it here — it\u0027s user-visible (an incremental can now base on a backup that\u0027s currently being restored), not just an internal optimization.","commit_id":"7b89aba74d5f17a0470cc3847bd366201c618533"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"448f499bb292ccf30aaad8522491d249a7019b2d","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    `Bug #1696715 \u003chttps://bugs.launchpad.net/cinder/+bug/1696715\u003e`_:"},{"line_number":5,"context_line":"    Optimize getting parent backup for new incremental by not fetching all"},{"line_number":6,"context_line":"    backups from the database, but actually select the suitable parent."}],"source_content_type":"text/x-yaml","patch_set":32,"id":"d5fd34d6_22c5eeaf","line":6,"in_reply_to":"afea144b_a2626c00","updated":"2026-07-16 13:27:21.000000000","message":"Acknowledged","commit_id":"7b89aba74d5f17a0470cc3847bd366201c618533"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"24e6442df670dd76c1b613d577297ca249768d09","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":35,"id":"745b3e6b_8468b3c3","line":10,"updated":"2026-08-14 00:05:10.000000000","message":"there\u0027s a second behaviour that needs noting.\n\nConsider a snapshot-based incremental where every existing backup is *newer*\nthan the snapshot. The old max() gave each ineligible backup the year-1\nsentinel key, so with all keys equal it returned backups.objects[0]; if that\none happened to be AVAILABLE, the `!\u003d AVAILABLE` guard passed and it was used\nas the parent — precisely the wrong-parent case the NOTE(xyang) block warns\nabout. The new query applies `data_timestamp \u003c before_data_timestamp`, finds\nnothing, and raises InvalidBackup.\n\nThat\u0027s the correct fix, but it turns a request that previously succeeded into\na 400 with \"No backups available to do an incremental backup.\" Operators who\nhit this path will see new failures after upgrade, so it deserves a sentence\nhere alongside the RESTORING note.","commit_id":"5b0e05c6116516fd26dcecab3c39c53ff787786b"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"2c4bd71021703f17f56bf54e1301647e8956edba","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":35,"id":"dd433385_99f7fc3e","line":10,"in_reply_to":"745b3e6b_8468b3c3","updated":"2026-08-18 14:13:49.000000000","message":"Done","commit_id":"5b0e05c6116516fd26dcecab3c39c53ff787786b"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"092122d1194fc0968c78bdc6b447b9997a86e106","unresolved":true,"context_lines":[{"line_number":5,"context_line":"    Optimize getting parent backup for new incremental by not fetching all"},{"line_number":6,"context_line":"    backups from the database, but using a DB query to find the suitable"},{"line_number":7,"context_line":"    parent. Alongside, a previous bug with finding parents for snapshot backups"},{"line_number":8,"context_line":"    was fixed. This caused to sometimes return an actually ineligible parent."},{"line_number":9,"context_line":"    This case properly trigger an error now."},{"line_number":10,"context_line":"    Additionally the backup status of `RESTORING` was added to the list"},{"line_number":11,"context_line":"    of valid status, as a restoring backup is indeed a valid, read-only source"},{"line_number":12,"context_line":"    for a new incremental backup."}],"source_content_type":"text/x-yaml","patch_set":36,"id":"7c1e699f_ba9773fc","line":9,"range":{"start_line":8,"start_character":15,"end_line":9,"end_character":44},"updated":"2026-08-18 15:27:51.000000000","message":"grammer seems off - maybe:\nPreviously this could select an ineligible parent when backing up a snapshot older than existing backups; that case now raises an error instead of silently creating an incremental against the wrong parent.","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"7351beb8b1a9afb1f47f7dbbc6a3892f326c06e3","unresolved":false,"context_lines":[{"line_number":5,"context_line":"    Optimize getting parent backup for new incremental by not fetching all"},{"line_number":6,"context_line":"    backups from the database, but using a DB query to find the suitable"},{"line_number":7,"context_line":"    parent. Alongside, a previous bug with finding parents for snapshot backups"},{"line_number":8,"context_line":"    was fixed. This caused to sometimes return an actually ineligible parent."},{"line_number":9,"context_line":"    This case properly trigger an error now."},{"line_number":10,"context_line":"    Additionally the backup status of `RESTORING` was added to the list"},{"line_number":11,"context_line":"    of valid status, as a restoring backup is indeed a valid, read-only source"},{"line_number":12,"context_line":"    for a new incremental backup."}],"source_content_type":"text/x-yaml","patch_set":36,"id":"926e8531_5bc06a83","line":9,"range":{"start_line":8,"start_character":15,"end_line":9,"end_character":44},"in_reply_to":"7c1e699f_ba9773fc","updated":"2026-09-02 07:49:40.000000000","message":"Done","commit_id":"5e843ea8282137390ac7947c933da256419353b2"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"c34db7071d3d020715b1c3bc7387aa245a493189","unresolved":true,"context_lines":[{"line_number":10,"context_line":"    instead of silently creating an incremental against the wrong parent."},{"line_number":11,"context_line":"    Additionally the backup status of `RESTORING` was added to the list"},{"line_number":12,"context_line":"    of valid status, as a restoring backup is indeed a valid, read-only source"},{"line_number":13,"context_line":"    for a new incremental backup."}],"source_content_type":"text/x-yaml","patch_set":42,"id":"d4ea8764_82bf8e3c","line":13,"updated":"2026-09-03 12:52:51.000000000","message":"Still worth a line about the new index. To your earlier question — you\u0027re\nright that operators run db sync every upgrade and don\u0027t need telling. The\nreason to mention it isn\u0027t the mechanics, it\u0027s the cost: creating an index on\na backups table with millions of rows can block or take a long while, so\noperators planning a maintenance window want to know it\u0027s there. An \"upgrade:\"\nsection with one sentence naming backups_volume_id_data_timestamp_idx is\nenough.","commit_id":"72fec9c71d666ed471ee9a0ae16a036e55f6cd0b"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"6b35acc600a7eac0a22c378c4dccca5e50355b25","unresolved":false,"context_lines":[{"line_number":10,"context_line":"    instead of silently creating an incremental against the wrong parent."},{"line_number":11,"context_line":"    Additionally the backup status of `RESTORING` was added to the list"},{"line_number":12,"context_line":"    of valid status, as a restoring backup is indeed a valid, read-only source"},{"line_number":13,"context_line":"    for a new incremental backup."}],"source_content_type":"text/x-yaml","patch_set":42,"id":"6f4c1a4f_11ca6c3f","line":13,"in_reply_to":"d4ea8764_82bf8e3c","updated":"2026-09-04 15:29:46.000000000","message":"Done","commit_id":"72fec9c71d666ed471ee9a0ae16a036e55f6cd0b"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"9d23875e53ddb80cf87cb7428539de6db0862dba","unresolved":true,"context_lines":[{"line_number":13,"context_line":"    for a new incremental backup."},{"line_number":14,"context_line":"upgrade:"},{"line_number":15,"context_line":"  - |"},{"line_number":16,"context_line":"    The ``cinder-manage db sync`` command for this verison of cinder will add"},{"line_number":17,"context_line":"    an additional database index ``backups_volume_id_data_timestamp_idx`` on"},{"line_number":18,"context_line":"    the backups table. Depending on the table size, this will take time to"},{"line_number":19,"context_line":"    complete."}],"source_content_type":"text/x-yaml","patch_set":43,"id":"7fee8216_d026ff47","line":16,"range":{"start_line":16,"start_character":51,"end_line":16,"end_character":59},"updated":"2026-09-04 18:30:04.000000000","message":"nit: version","commit_id":"ed4a49cc5ca31c9c4045b184ce1825ab42e2d1b9"}]}