)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"f53488267777a681e48ef15199bfa9ee168db38d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"905235cb_23ed0392","updated":"2023-10-17 07:12:46.000000000","message":"lgtm","commit_id":"e39ebce8b7be98221df8570f15393d58e529e22f"}],"cinder/api/contrib/backups.py":[{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"8422487355765f9e5f914991d05875f1ee307337","unresolved":true,"context_lines":[{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        backup \u003d body[\u0027backup\u0027]"},{"line_number":161,"context_line":"        container \u003d backup.get(\u0027container\u0027, None)"},{"line_number":162,"context_line":"        volume_id \u003d backup[\u0027volume_id\u0027]"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        self.validate_name_and_description(backup, check_length\u003dFalse)"},{"line_number":165,"context_line":"        name \u003d backup.get(\u0027name\u0027, None)"}],"source_content_type":"text/x-python","patch_set":1,"id":"e9d8e4d5_db082157","line":162,"updated":"2021-08-25 06:31:44.000000000","message":"This makes me think that \u0027volume_id\u0027 is expected to always be in the backup dict (I believe if it wasn\u0027t then a key error will be thrown here) - as it does not follow the \"backup.get(\u0027key\u0027. None)\" pattern.\n\nWhat also leads me to believe that is that often volume APIs that deal with snapshots still include a volume id","commit_id":"e677ce3072be50e44dcdda57b7557659118b765e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"80109a68a5c459257d2274a14ffa1033e49f320e","unresolved":true,"context_lines":[{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        backup \u003d body[\u0027backup\u0027]"},{"line_number":161,"context_line":"        container \u003d backup.get(\u0027container\u0027, None)"},{"line_number":162,"context_line":"        volume_id \u003d backup[\u0027volume_id\u0027]"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        self.validate_name_and_description(backup, check_length\u003dFalse)"},{"line_number":165,"context_line":"        name \u003d backup.get(\u0027name\u0027, None)"}],"source_content_type":"text/x-python","patch_set":1,"id":"8a296b2f_7ce3f706","line":162,"in_reply_to":"e9d8e4d5_db082157","updated":"2023-10-13 09:13:29.000000000","message":"Yes, you are correct. the volume ID is always expected whereas the snapshot ID is optional.","commit_id":"e677ce3072be50e44dcdda57b7557659118b765e"},{"author":{"_account_id":16721,"name":"Zohar Mamedov","email":"zohar.cloud@gmail.com","username":"zohar"},"change_message_id":"8422487355765f9e5f914991d05875f1ee307337","unresolved":true,"context_lines":[{"line_number":184,"context_line":"                     {\u0027volume_id\u0027: volume_id, \u0027container\u0027: container,"},{"line_number":185,"context_line":"                      \u0027az\u0027: az_text},"},{"line_number":186,"context_line":"                     context\u003dcontext)"},{"line_number":187,"context_line":"        elif snapshot_id:"},{"line_number":188,"context_line":"            LOG.info(\"Creating backup of snapshot %(snapshot_id)s in container\""},{"line_number":189,"context_line":"                     \" %(container)s%(az)s\","},{"line_number":190,"context_line":"                     {\u0027snapshot_id\u0027: snapshot_id, \u0027container\u0027: container,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5d2c9f66_a7cc3707","line":187,"updated":"2021-08-25 06:31:44.000000000","message":"Line 162 makes me think that \u0027volume_id\u0027 is always expected to be in the backup dict.\n\nSo, the backup create call for snapshots will still have a volume_id\n\nIf that is really the case, I believe this condition will never be reached. In that case, I suggest reversing the order here (checking for snapshot_id first).","commit_id":"e677ce3072be50e44dcdda57b7557659118b765e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"80109a68a5c459257d2274a14ffa1033e49f320e","unresolved":false,"context_lines":[{"line_number":184,"context_line":"                     {\u0027volume_id\u0027: volume_id, \u0027container\u0027: container,"},{"line_number":185,"context_line":"                      \u0027az\u0027: az_text},"},{"line_number":186,"context_line":"                     context\u003dcontext)"},{"line_number":187,"context_line":"        elif snapshot_id:"},{"line_number":188,"context_line":"            LOG.info(\"Creating backup of snapshot %(snapshot_id)s in container\""},{"line_number":189,"context_line":"                     \" %(container)s%(az)s\","},{"line_number":190,"context_line":"                     {\u0027snapshot_id\u0027: snapshot_id, \u0027container\u0027: container,"}],"source_content_type":"text/x-python","patch_set":1,"id":"c82b0ac0_11a4745d","line":187,"in_reply_to":"29ffd233_d1982560","updated":"2023-10-13 09:13:29.000000000","message":"Sorry i totally lost track of this. I agree with Zohar\u0027s comment and thanks Pete for updating it, though I haven\u0027t tested it.","commit_id":"e677ce3072be50e44dcdda57b7557659118b765e"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"7aac2d66836f69213a0b4c2cb35fd90128a1dc73","unresolved":true,"context_lines":[{"line_number":184,"context_line":"                     {\u0027volume_id\u0027: volume_id, \u0027container\u0027: container,"},{"line_number":185,"context_line":"                      \u0027az\u0027: az_text},"},{"line_number":186,"context_line":"                     context\u003dcontext)"},{"line_number":187,"context_line":"        elif snapshot_id:"},{"line_number":188,"context_line":"            LOG.info(\"Creating backup of snapshot %(snapshot_id)s in container\""},{"line_number":189,"context_line":"                     \" %(container)s%(az)s\","},{"line_number":190,"context_line":"                     {\u0027snapshot_id\u0027: snapshot_id, \u0027container\u0027: container,"}],"source_content_type":"text/x-python","patch_set":1,"id":"29ffd233_d1982560","line":187,"in_reply_to":"5d2c9f66_a7cc3707","updated":"2023-10-12 14:25:48.000000000","message":"I think Zohar is right.\n\nHow about I respin this? Give me a few moments.\n\nRajat, please take a quick look at and agree or reject.","commit_id":"e677ce3072be50e44dcdda57b7557659118b765e"}]}