)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"713e0aa5342ec2b6c374270621c78daa48a6408a","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"32e825d7_e4347590","updated":"2023-07-04 09:28:51.000000000","message":"Thanks for fixing the bug.\nPlease add a release note and simplify the unit tests changes.","commit_id":"21f54fc02b041659934a6da61d333569e222d339"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"87e29675c3ba658cb66224e63e790e9a85c460cf","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"da7e71ad_384b4771","updated":"2023-07-12 10:50:07.000000000","message":"Hi Pete,\n\nThe code looks good; thank you so much for proposing this fix. Just two minor things:\n\n:nit: Can you add a test like test_create_from_backup_missing_volume_is_new() or something similar to test if an error should be raised?\n\n:-1: We also need a release note for this.\n\nCheers,","commit_id":"ed3676758b37417ec004fae1c2a201f5ce027985"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"a0e54692133c761703ab5b6c9d8db00189f373cc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e1ce021f_b5d9cc38","updated":"2023-07-07 13:59:51.000000000","message":"recheck\n\ntimeout in test tempest.api.volume.test_volumes_negative.VolumesNegativeTest.test_create_volume_from_deactivated_image\nin one of Tempest jobs (with Barbician)","commit_id":"ed3676758b37417ec004fae1c2a201f5ce027985"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"174f3fd5383a8ff5adf6990dc880f16f5626d83d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"bad85fd7_aecfe536","in_reply_to":"d510ba03_56189ea8","updated":"2023-07-12 15:11:42.000000000","message":"I\u0027ll add the release note.\nAs for testing between the layers, we have this:\nhttps://review.opendev.org/c/openstack/cinder/+/887851\nIt\u0027s a good test to get this in, re-base the mypy update, and see if it starts passing.","commit_id":"ed3676758b37417ec004fae1c2a201f5ce027985"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"48e296f652451fae1ff80f8193382c6f795261d1","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"d510ba03_56189ea8","in_reply_to":"da7e71ad_384b4771","updated":"2023-07-12 14:51:13.000000000","message":"\u003e Can you add a test like test_create_from_backup_missing_volume_is_new() or something similar to test if an error should be raised?\n\nThis doesn\u0027t sound like the right thing to do, or maybe I\u0027m missing the idea being proposed.  The problem here was a missing arg to a call -- I\u0027m not sure what value is added by a unit test that reproduces that.","commit_id":"ed3676758b37417ec004fae1c2a201f5ce027985"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"00a0bd362847f7479158ab30ea876d8d321aeb8b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"bfdfa24e_5e3d9926","updated":"2023-07-14 23:05:03.000000000","message":"Looks like this fixes the regression.  Needs to be backported to stable/2023.1","commit_id":"2e031e1cacd914a18a6d5f279177f6eeba2f2d46"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"c3227e398197d3406294946179c5579f91711ec2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"6f0cbee5_8306bf6f","updated":"2023-07-15 02:59:00.000000000","message":"recheck\n\nNot sure if legitimate... let\u0027s see if it repeats:\n\n{3} tempest.api.compute.volumes.test_attach_volume.AttachVolumeMultiAttachTest.test_resize_server_with_multiattached_volume [133.212338s] ... FAILED","commit_id":"2e031e1cacd914a18a6d5f279177f6eeba2f2d46"}],"cinder/tests/unit/volume/flows/test_create_volume_flow.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"713e0aa5342ec2b6c374270621c78daa48a6408a","unresolved":true,"context_lines":[{"line_number":1279,"context_line":"            mock_create_volume.assert_called_once_with(self.ctxt, volume_obj)"},{"line_number":1280,"context_line":"            mock_get_backup_host.assert_called_once_with("},{"line_number":1281,"context_line":"                backup_obj.host, backup_obj.availability_zone)"},{"line_number":1282,"context_line":"            mock_restore_backup.assert_called_once_with(self.ctxt,"},{"line_number":1283,"context_line":"                                                        backup_host,"},{"line_number":1284,"context_line":"                                                        backup_obj,"},{"line_number":1285,"context_line":"                                                        volume_obj[\u0027id\u0027])"},{"line_number":1286,"context_line":"        else:"},{"line_number":1287,"context_line":"            fake_driver.create_volume_from_backup.assert_called_once_with("},{"line_number":1288,"context_line":"                volume_obj, backup_obj)"}],"source_content_type":"text/x-python","patch_set":1,"id":"45768ed3_421a21e8","side":"PARENT","line":1285,"range":{"start_line":1282,"start_character":0,"end_line":1285,"end_character":73},"updated":"2023-07-04 09:28:51.000000000","message":"-1: The only changed needed is to pass True as a new argument here:\n\n            mock_restore_backup.assert_called_once_with(self.ctxt,\n                                                        backup_host,\n                                                        backup_obj,\n                                                        volume_obj[\u0027id\u0027],\n                                                        True)\n                                                        \nAnd if we go with the suggested alternative to pass it as a named argument it would be:\n\n            mock_restore_backup.assert_called_once_with(self.ctxt,\n                                                        backup_host,\n                                                        backup_obj,\n                                                        volume_obj[\u0027id\u0027],\n                                                        volume_is_new\u003dTrue)","commit_id":"f1569abd66bdccf5536e6cb258b5b7bbc2f30b59"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"68b7205af4ac410335432342b8c83b476fe8c290","unresolved":true,"context_lines":[{"line_number":1279,"context_line":"            mock_create_volume.assert_called_once_with(self.ctxt, volume_obj)"},{"line_number":1280,"context_line":"            mock_get_backup_host.assert_called_once_with("},{"line_number":1281,"context_line":"                backup_obj.host, backup_obj.availability_zone)"},{"line_number":1282,"context_line":"            mock_restore_backup.assert_called_once_with(self.ctxt,"},{"line_number":1283,"context_line":"                                                        backup_host,"},{"line_number":1284,"context_line":"                                                        backup_obj,"},{"line_number":1285,"context_line":"                                                        volume_obj[\u0027id\u0027])"},{"line_number":1286,"context_line":"        else:"},{"line_number":1287,"context_line":"            fake_driver.create_volume_from_backup.assert_called_once_with("},{"line_number":1288,"context_line":"                volume_obj, backup_obj)"}],"source_content_type":"text/x-python","patch_set":1,"id":"e059edea_2cbb4951","side":"PARENT","line":1285,"range":{"start_line":1282,"start_character":0,"end_line":1285,"end_character":73},"in_reply_to":"45768ed3_421a21e8","updated":"2023-07-05 19:50:34.000000000","message":"I don\u0027t quite understand what you mean. The motivation behind the changes here was to move the boundary a little deeper, so that the real code is used where the function arguments are changed. The reason why existing tests didn\u0027t catch is that the tests mock on the boundary where the change happened.\n\nThe problem is that the existing tests cannot catch a change in protocol or API if they mock that very API.\n\nOne argument that I see against what I\u0027ve done is that it\u0027s all fine and now I made the tests better, but only for restore_backup. But we have a number of these API calls, which remain vulnerable to API mismatch in the future.\n\nI certainly can just add the extra argument and be done.","commit_id":"f1569abd66bdccf5536e6cb258b5b7bbc2f30b59"}],"cinder/volume/flows/manager/create_volume.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"713e0aa5342ec2b6c374270621c78daa48a6408a","unresolved":true,"context_lines":[{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"            backuprpcapi \u003d backup_rpcapi.BackupAPI()"},{"line_number":1174,"context_line":"            backuprpcapi.restore_backup(context, backup.host, backup,"},{"line_number":1175,"context_line":"                                        volume.id, True)"},{"line_number":1176,"context_line":"            need_update_volume \u003d False"},{"line_number":1177,"context_line":""},{"line_number":1178,"context_line":"        LOG.info(\"Created volume %(volume_id)s from backup %(backup_id)s \""}],"source_content_type":"text/x-python","patch_set":1,"id":"c3ee97e5_705c4185","line":1175,"updated":"2023-07-04 09:28:51.000000000","message":"nit: If possible pass it as a named argument to know what this is:\n\n                                        volume.id, volume_is_new\u003dTrue)","commit_id":"21f54fc02b041659934a6da61d333569e222d339"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"68b7205af4ac410335432342b8c83b476fe8c290","unresolved":true,"context_lines":[{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"            backuprpcapi \u003d backup_rpcapi.BackupAPI()"},{"line_number":1174,"context_line":"            backuprpcapi.restore_backup(context, backup.host, backup,"},{"line_number":1175,"context_line":"                                        volume.id, True)"},{"line_number":1176,"context_line":"            need_update_volume \u003d False"},{"line_number":1177,"context_line":""},{"line_number":1178,"context_line":"        LOG.info(\"Created volume %(volume_id)s from backup %(backup_id)s \""}],"source_content_type":"text/x-python","patch_set":1,"id":"4d10eaac_4c3cb2d0","line":1175,"in_reply_to":"c3ee97e5_705c4185","updated":"2023-07-05 19:50:34.000000000","message":"Sure.\nI thought this was a change that was not intended to be compatible, which is why I used a positional argument.","commit_id":"21f54fc02b041659934a6da61d333569e222d339"}]}