)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":33431,"name":"Fábio Oliveira","email":"fabioaurelio1269@gmail.com","username":"fabiooliveira1"},"change_message_id":"48fa05df004dd721895790e92dd2c28709fc3b3c","unresolved":true,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2021-08-11 19:16:21 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"StorPool driver: implement revert_to_snapshot"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I3b8126cf1706921eac9556add11c558d973cf272"},{"line_number":10,"context_line":"Implements: blueprint storpool-revert-to-snapshot"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"1bef5f0b_183c9c3f","line":8,"updated":"2021-08-12 15:44:17.000000000","message":"you could add a more detailed description, not just the commit title","commit_id":"ebb8c08631506236666bd1fe621856f35cd3673d"},{"author":{"_account_id":12988,"name":"Peter Penchev","email":"openstack-dev@storpool.com","username":"ppenchev"},"change_message_id":"840188273c18fde09da2d99403652ece77e18bc1","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2021-08-11 19:16:21 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"StorPool driver: implement revert_to_snapshot"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I3b8126cf1706921eac9556add11c558d973cf272"},{"line_number":10,"context_line":"Implements: blueprint storpool-revert-to-snapshot"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"2f5be47c_b157f88a","line":8,"in_reply_to":"1bef5f0b_183c9c3f","updated":"2021-11-15 12:00:05.000000000","message":"Thanks, fixed in patchset 10.","commit_id":"ebb8c08631506236666bd1fe621856f35cd3673d"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":33431,"name":"Fábio Oliveira","email":"fabioaurelio1269@gmail.com","username":"fabiooliveira1"},"change_message_id":"05b2779385ac620f9da33f8c607f9b0504f0d494","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"24e32333_c3080e72","updated":"2022-01-20 19:45:52.000000000","message":"Nice work!\nI addressed some comments, please check them inline.\nThank you for your effort in this.","commit_id":"278b85622bec40b063c789ce7cf26f324cbe7597"},{"author":{"_account_id":29122,"name":"Raghavendra Tilay","email":"raghavendra-uddhav.tilay@hpe.com","username":"raghavendrat"},"change_message_id":"69872383aa5f3042b96390a5e077171a2730e0cc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"6bfc4d24_99055ffa","updated":"2024-06-17 09:55:08.000000000","message":"If another patchset is submitted, a release note can be added.\nIt would be good, if StorPool OpenStack CI gets passed on latest patchset.","commit_id":"d6221d5accff83c971e26e6e15ec08e56f062f2f"},{"author":{"_account_id":35429,"name":"Biser Milanov","email":"biser.milanov@storpool.com","username":"sp-bmilanov"},"change_message_id":"9f41e28f83013a0e35b515d1392d69bb8b5f79be","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"b2cdec33_4503ac77","updated":"2024-06-17 12:40:44.000000000","message":"run-storpoolci","commit_id":"d6221d5accff83c971e26e6e15ec08e56f062f2f"},{"author":{"_account_id":29122,"name":"Raghavendra Tilay","email":"raghavendra-uddhav.tilay@hpe.com","username":"raghavendrat"},"change_message_id":"1caf4f71ceb7036a6400caa5f2c5325293ec298b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"083ecf8e_d5382579","updated":"2024-06-19 12:30:43.000000000","message":"Code and UT look good. My comments have been addressed.\nZuul and StorPool OpenStack CI have passed.","commit_id":"0867c4d3e1a930044cf9d618bf53c36f104d2d3d"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"0db25f8484a4004cc7b8efe2c61794bd47a7fd73","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"67ea1b7d_6af57e08","updated":"2024-06-19 15:03:23.000000000","message":"LGTM.  The volume_revert tests are enabled in the StorPool CI:\nhttps://spfactory.storpool.com/zuul/t/local/build/299d8cf4e82f4619bae6040b89199398/log/controller/logs/tempest_conf.txt#106\n\nand the tests run:\nhttps://spfactory.storpool.com/zuul/t/local/build/299d8cf4e82f4619bae6040b89199398/log/job-output.txt#24540\n\nbtw (something to look at later), the Unit Test Report in the Artifacts tab gives a 404:\nhttps://spfactory.storpool.com/logs/89/680889/12/check/cinder-storpool-tempest/299d8cf/testr_results.html","commit_id":"0867c4d3e1a930044cf9d618bf53c36f104d2d3d"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"c83c1de03c8a8bc1a22246f6c479403cb8603ba7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"2238de2b_0a0ef610","updated":"2024-06-20 14:11:48.000000000","message":"This has a +2 from Brian.  Code looks good to me.  Think it can merge.","commit_id":"0867c4d3e1a930044cf9d618bf53c36f104d2d3d"},{"author":{"_account_id":35429,"name":"Biser Milanov","email":"biser.milanov@storpool.com","username":"sp-bmilanov"},"change_message_id":"33acc45313a0d1cd3e4d1a5eb715b960b179608c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"53fa0efc_3ee1e1a5","updated":"2024-06-18 06:19:05.000000000","message":"run-storpoolci","commit_id":"0867c4d3e1a930044cf9d618bf53c36f104d2d3d"}],"cinder/tests/unit/volume/drivers/test_storpool.py":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"16dfbe288ccfadae79cbbf5b5ffcbe359d2223fa","unresolved":false,"context_lines":[{"line_number":216,"context_line":"        self.driver \u003d driver.StorPoolDriver(execute\u003dmock_exec,"},{"line_number":217,"context_line":"                                            configuration\u003dself.cfg)"},{"line_number":218,"context_line":"        self.driver.check_for_setup_error()"},{"line_number":219,"context_line":"        assert not self.driver._attach.api().clearToRaise()"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        volumes.clear()"},{"line_number":222,"context_line":"        snapshots.clear()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_e0d6f727","line":219,"range":{"start_line":219,"start_character":8,"end_line":219,"end_character":19},"updated":"2019-09-10 14:27:54.000000000","message":"Really would be better to use self.assertFalse","commit_id":"ca346ee35bff2950712c7e85a56c4c78d0c71f07"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"492968892d937ec23c83f081d48b3ebf15201057","unresolved":false,"context_lines":[{"line_number":216,"context_line":"        self.driver \u003d driver.StorPoolDriver(execute\u003dmock_exec,"},{"line_number":217,"context_line":"                                            configuration\u003dself.cfg)"},{"line_number":218,"context_line":"        self.driver.check_for_setup_error()"},{"line_number":219,"context_line":"        assert not self.driver._attach.api().clearToRaise()"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        volumes.clear()"},{"line_number":222,"context_line":"        snapshots.clear()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_3b0c413a","line":219,"range":{"start_line":219,"start_character":8,"end_line":219,"end_character":19},"in_reply_to":"5faad753_e0d6f727","updated":"2019-09-10 15:03:58.000000000","message":"+1","commit_id":"ca346ee35bff2950712c7e85a56c4c78d0c71f07"},{"author":{"_account_id":12988,"name":"Peter Penchev","email":"openstack-dev@storpool.com","username":"ppenchev"},"change_message_id":"800fb14edf7c98d66371b85a477e321286155b0f","unresolved":false,"context_lines":[{"line_number":216,"context_line":"        self.driver \u003d driver.StorPoolDriver(execute\u003dmock_exec,"},{"line_number":217,"context_line":"                                            configuration\u003dself.cfg)"},{"line_number":218,"context_line":"        self.driver.check_for_setup_error()"},{"line_number":219,"context_line":"        assert not self.driver._attach.api().clearToRaise()"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        volumes.clear()"},{"line_number":222,"context_line":"        snapshots.clear()"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_b0d1b825","line":219,"range":{"start_line":219,"start_character":8,"end_line":219,"end_character":19},"in_reply_to":"5faad753_e0d6f727","updated":"2019-10-14 12:17:53.000000000","message":"Sorry about that one, not quite sure what I was thinking. Fixed in patchset 2.","commit_id":"ca346ee35bff2950712c7e85a56c4c78d0c71f07"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"16dfbe288ccfadae79cbbf5b5ffcbe359d2223fa","unresolved":false,"context_lines":[{"line_number":364,"context_line":"        volume_created \u003d False"},{"line_number":365,"context_line":"        snapshot_created \u003d False"},{"line_number":366,"context_line":""},{"line_number":367,"context_line":"        try:"},{"line_number":368,"context_line":"            # Create a volume"},{"line_number":369,"context_line":"            self.driver.create_volume({\u0027id\u0027: \u00271\u0027, \u0027name\u0027: \u0027v1\u0027, \u0027size\u0027: 1,"},{"line_number":370,"context_line":"                                       \u0027volume_type\u0027: None})"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_e03117a4","line":367,"range":{"start_line":367,"start_character":8,"end_line":367,"end_character":11},"updated":"2019-09-10 14:27:54.000000000","message":"Why do you have try/finally blocks in unit tests? The tests should do what is expected and validate that.","commit_id":"ca346ee35bff2950712c7e85a56c4c78d0c71f07"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"492968892d937ec23c83f081d48b3ebf15201057","unresolved":false,"context_lines":[{"line_number":364,"context_line":"        volume_created \u003d False"},{"line_number":365,"context_line":"        snapshot_created \u003d False"},{"line_number":366,"context_line":""},{"line_number":367,"context_line":"        try:"},{"line_number":368,"context_line":"            # Create a volume"},{"line_number":369,"context_line":"            self.driver.create_volume({\u0027id\u0027: \u00271\u0027, \u0027name\u0027: \u0027v1\u0027, \u0027size\u0027: 1,"},{"line_number":370,"context_line":"                                       \u0027volume_type\u0027: None})"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_3b2521ac","line":367,"range":{"start_line":367,"start_character":8,"end_line":367,"end_character":11},"in_reply_to":"5faad753_e03117a4","updated":"2019-09-10 15:03:58.000000000","message":"+1 to Sean here. We should omit try-except usage in unit tests","commit_id":"ca346ee35bff2950712c7e85a56c4c78d0c71f07"},{"author":{"_account_id":12988,"name":"Peter Penchev","email":"openstack-dev@storpool.com","username":"ppenchev"},"change_message_id":"800fb14edf7c98d66371b85a477e321286155b0f","unresolved":false,"context_lines":[{"line_number":364,"context_line":"        volume_created \u003d False"},{"line_number":365,"context_line":"        snapshot_created \u003d False"},{"line_number":366,"context_line":""},{"line_number":367,"context_line":"        try:"},{"line_number":368,"context_line":"            # Create a volume"},{"line_number":369,"context_line":"            self.driver.create_volume({\u0027id\u0027: \u00271\u0027, \u0027name\u0027: \u0027v1\u0027, \u0027size\u0027: 1,"},{"line_number":370,"context_line":"                                       \u0027volume_type\u0027: None})"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_50a644c3","line":367,"range":{"start_line":367,"start_character":8,"end_line":367,"end_character":11},"in_reply_to":"5faad753_e03117a4","updated":"2019-10-14 12:17:53.000000000","message":"Riiight... This was the result of a couple of later tests failing, me thinking \"of course we need to clean up after this test even if it fails\", *then* me realizing that the cleanup belongs in setUp(), not during or after the test, and then me forgetting to remove it from here. Sorry... fixed in patchset 2.","commit_id":"ca346ee35bff2950712c7e85a56c4c78d0c71f07"},{"author":{"_account_id":33431,"name":"Fábio Oliveira","email":"fabioaurelio1269@gmail.com","username":"fabiooliveira1"},"change_message_id":"48fa05df004dd721895790e92dd2c28709fc3b3c","unresolved":true,"context_lines":[{"line_number":378,"context_line":"        self.assertDictEqual({}, snapshots)"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        # Create a snapshot"},{"line_number":381,"context_line":"        self.driver.create_snapshot({\u0027id\u0027: \u002711\u0027, \u0027volume_id\u0027: \u00271\u0027})"},{"line_number":382,"context_line":"        self.assertEqual([volumeName(\u00271\u0027)], list(volumes.keys()))"},{"line_number":383,"context_line":"        self.assertNotIn(\u0027parent\u0027, volumes[volumeName(\u00271\u0027)])"},{"line_number":384,"context_line":"        self.assertNotIn(\u0027marco\u0027, volumes[volumeName(\u00271\u0027)])"}],"source_content_type":"text/x-python","patch_set":9,"id":"ac07a2ef_4c40fe2f","line":381,"range":{"start_line":381,"start_character":36,"end_line":381,"end_character":66},"updated":"2021-08-12 15:44:17.000000000","message":"you could use variables to avoid typos on the following repetitions","commit_id":"ebb8c08631506236666bd1fe621856f35cd3673d"},{"author":{"_account_id":12988,"name":"Peter Penchev","email":"openstack-dev@storpool.com","username":"ppenchev"},"change_message_id":"840188273c18fde09da2d99403652ece77e18bc1","unresolved":false,"context_lines":[{"line_number":378,"context_line":"        self.assertDictEqual({}, snapshots)"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        # Create a snapshot"},{"line_number":381,"context_line":"        self.driver.create_snapshot({\u0027id\u0027: \u002711\u0027, \u0027volume_id\u0027: \u00271\u0027})"},{"line_number":382,"context_line":"        self.assertEqual([volumeName(\u00271\u0027)], list(volumes.keys()))"},{"line_number":383,"context_line":"        self.assertNotIn(\u0027parent\u0027, volumes[volumeName(\u00271\u0027)])"},{"line_number":384,"context_line":"        self.assertNotIn(\u0027marco\u0027, volumes[volumeName(\u00271\u0027)])"}],"source_content_type":"text/x-python","patch_set":9,"id":"90056576_3412c37e","line":381,"range":{"start_line":381,"start_character":36,"end_line":381,"end_character":66},"in_reply_to":"ac07a2ef_4c40fe2f","updated":"2021-11-15 12:00:05.000000000","message":"The test was redone in patchset 10 (actually it went back to its earlier version from patchset 5, my clumsy fingers seem to have uploaded a series of outdated versions...), and it already used variable names for some of these values, but thanks, it uses more now.","commit_id":"ebb8c08631506236666bd1fe621856f35cd3673d"},{"author":{"_account_id":33431,"name":"Fábio Oliveira","email":"fabioaurelio1269@gmail.com","username":"fabiooliveira1"},"change_message_id":"48fa05df004dd721895790e92dd2c28709fc3b3c","unresolved":true,"context_lines":[{"line_number":388,"context_line":"        # Revert the volume to its last snapshot"},{"line_number":389,"context_line":"        self.driver.revert_to_snapshot("},{"line_number":390,"context_line":"            None,"},{"line_number":391,"context_line":"            {\u0027id\u0027: \u00271\u0027},"},{"line_number":392,"context_line":"            {\u0027id\u0027: \u002711\u0027, \u0027volume_id\u0027: \u00271\u0027})"},{"line_number":393,"context_line":"        self.assertEqual([volumeName(\u00271\u0027)], list(volumes.keys()))"},{"line_number":394,"context_line":"        self.assertIn(\u0027parent\u0027, volumes[volumeName(\u00271\u0027)])"}],"source_content_type":"text/x-python","patch_set":9,"id":"005b4dc8_87b89257","line":391,"range":{"start_line":391,"start_character":12,"end_line":391,"end_character":23},"updated":"2021-08-12 15:44:17.000000000","message":"same here","commit_id":"ebb8c08631506236666bd1fe621856f35cd3673d"},{"author":{"_account_id":12988,"name":"Peter Penchev","email":"openstack-dev@storpool.com","username":"ppenchev"},"change_message_id":"840188273c18fde09da2d99403652ece77e18bc1","unresolved":false,"context_lines":[{"line_number":388,"context_line":"        # Revert the volume to its last snapshot"},{"line_number":389,"context_line":"        self.driver.revert_to_snapshot("},{"line_number":390,"context_line":"            None,"},{"line_number":391,"context_line":"            {\u0027id\u0027: \u00271\u0027},"},{"line_number":392,"context_line":"            {\u0027id\u0027: \u002711\u0027, \u0027volume_id\u0027: \u00271\u0027})"},{"line_number":393,"context_line":"        self.assertEqual([volumeName(\u00271\u0027)], list(volumes.keys()))"},{"line_number":394,"context_line":"        self.assertIn(\u0027parent\u0027, volumes[volumeName(\u00271\u0027)])"}],"source_content_type":"text/x-python","patch_set":9,"id":"6abad458_4a97e5ae","line":391,"range":{"start_line":391,"start_character":12,"end_line":391,"end_character":23},"in_reply_to":"005b4dc8_87b89257","updated":"2021-11-15 12:00:05.000000000","message":"Ack","commit_id":"ebb8c08631506236666bd1fe621856f35cd3673d"},{"author":{"_account_id":33431,"name":"Fábio Oliveira","email":"fabioaurelio1269@gmail.com","username":"fabiooliveira1"},"change_message_id":"05b2779385ac620f9da33f8c607f9b0504f0d494","unresolved":true,"context_lines":[{"line_number":535,"context_line":"                             \u0027volume_type\u0027: volume_type"},{"line_number":536,"context_line":"                         }))"},{"line_number":537,"context_line":""},{"line_number":538,"context_line":"    def test_volume_revert(self):"},{"line_number":539,"context_line":"        vol_id \u003d \u0027rev1\u0027"},{"line_number":540,"context_line":"        vol_name \u003d volumeName(vol_id)"},{"line_number":541,"context_line":"        snap_id \u003d \u0027rev-s1\u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"1f0b5d2c_9ff6f540","line":538,"range":{"start_line":538,"start_character":4,"end_line":538,"end_character":33},"updated":"2022-01-20 19:45:52.000000000","message":"I believe a unit test should test only one function, so this one should cover, mock and assert only the functions called inside revert_to_snapshot.\nthis test is looking more like a functional/integration test.\n\ntake a look at this:\n\nhttps://opendev.org/openstack/cinder/src/branch/master/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py#L229\nhttps://opendev.org/openstack/cinder/src/branch/master/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_cmode.py#L2222\n\nin other words, you souldn\u0027t need one test to cover everything if you have many tests for each part/unit :D","commit_id":"278b85622bec40b063c789ce7cf26f324cbe7597"},{"author":{"_account_id":32919,"name":"kiran pawar","display_name":"Kiran Pawar","email":"kinpaa@gmail.com","username":"kpdev"},"change_message_id":"4773cd82a782de8c0ddad83bdafa742ecd0963c3","unresolved":false,"context_lines":[{"line_number":535,"context_line":"                             \u0027volume_type\u0027: volume_type"},{"line_number":536,"context_line":"                         }))"},{"line_number":537,"context_line":""},{"line_number":538,"context_line":"    def test_volume_revert(self):"},{"line_number":539,"context_line":"        vol_id \u003d \u0027rev1\u0027"},{"line_number":540,"context_line":"        vol_name \u003d volumeName(vol_id)"},{"line_number":541,"context_line":"        snap_id \u003d \u0027rev-s1\u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"014c2f99_6e75344a","line":538,"range":{"start_line":538,"start_character":4,"end_line":538,"end_character":33},"in_reply_to":"1f0b5d2c_9ff6f540","updated":"2024-06-12 11:26:46.000000000","message":"this is aligned with other tests in this file.","commit_id":"278b85622bec40b063c789ce7cf26f324cbe7597"}],"cinder/volume/drivers/storpool.py":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"16dfbe288ccfadae79cbbf5b5ffcbe359d2223fa","unresolved":false,"context_lines":[{"line_number":522,"context_line":"        volname \u003d self._attach.volumeName(volume[\u0027id\u0027])"},{"line_number":523,"context_line":"        snapname \u003d self._attach.snapshotName(\u0027snap\u0027, snapshot[\u0027id\u0027])"},{"line_number":524,"context_line":"        try:"},{"line_number":525,"context_line":"            self._attach.api().volumeDelete(volname)"},{"line_number":526,"context_line":"        except spapi.ApiError as err:"},{"line_number":527,"context_line":"            msg \u003d ("},{"line_number":528,"context_line":"                \u0027StorPool revert to snapshot failed: \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_c0b89bc9","line":525,"range":{"start_line":525,"start_character":31,"end_line":525,"end_character":43},"updated":"2019-09-10 14:27:54.000000000","message":"I don\u0027t think this can be done by actually deleting the existing volume and just creating a new one with the same name. Will Storpool keep existing identifiers when this happens? (if it does, that would be pretty unique)","commit_id":"ca346ee35bff2950712c7e85a56c4c78d0c71f07"},{"author":{"_account_id":12988,"name":"Peter Penchev","email":"openstack-dev@storpool.com","username":"ppenchev"},"change_message_id":"800fb14edf7c98d66371b85a477e321286155b0f","unresolved":false,"context_lines":[{"line_number":522,"context_line":"        volname \u003d self._attach.volumeName(volume[\u0027id\u0027])"},{"line_number":523,"context_line":"        snapname \u003d self._attach.snapshotName(\u0027snap\u0027, snapshot[\u0027id\u0027])"},{"line_number":524,"context_line":"        try:"},{"line_number":525,"context_line":"            self._attach.api().volumeDelete(volname)"},{"line_number":526,"context_line":"        except spapi.ApiError as err:"},{"line_number":527,"context_line":"            msg \u003d ("},{"line_number":528,"context_line":"                \u0027StorPool revert to snapshot failed: \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_508f2429","line":525,"range":{"start_line":525,"start_character":31,"end_line":525,"end_character":43},"in_reply_to":"5faad753_c0b89bc9","updated":"2019-10-14 12:17:53.000000000","message":"Well, this is how StorPool volumes and snapshots work best - the absolutely fastest and most reliable way to revert a volume to a snapshot is to recreate it - it is practically instantaneous, there is absolutely no data being transferred anywhere, just a couple of system tables with object versions being updated.\n\nThis would only be a problem if the volume is in a state different from \"available\" and \"error\", i.e. if it is attached somewhere... but I certainly do hope that no one will ever try to revert an attached volume to a previous snapshot, since this will break in all kinds of funny ways regarding the operating system\u0027s block cache - since the disk driver has no idea about the cache in the kernel\u0027s higher-level layers, it has no way to invalidate it, so if a volume\u0027s data changes, the operating system has no way of knowing that. This sounds like a one-way ticket for the data corruption express...\n\nSo, if the volume is not attached, recreating it from the snapshot does the job of practically reverting it to that snapshot. Other than the volume ID (which is part of the name of the underlying StorPool volume, so it is preserved) what are the \"existing identifiers\" that you are referring to?","commit_id":"ca346ee35bff2950712c7e85a56c4c78d0c71f07"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"16dfbe288ccfadae79cbbf5b5ffcbe359d2223fa","unresolved":false,"context_lines":[{"line_number":528,"context_line":"                \u0027StorPool revert to snapshot failed: \u0027"},{"line_number":529,"context_line":"                \u0027could not delete volume {vname}: {err}\u0027"},{"line_number":530,"context_line":"            ).format(vname\u003dvolname, err\u003dsix.text_type(err))"},{"line_number":531,"context_line":"            raise self._backendException(msg)"},{"line_number":532,"context_line":"        try:"},{"line_number":533,"context_line":"            self._attach.api().volumeCreate({"},{"line_number":534,"context_line":"                \u0027name\u0027: volname,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_a0aebf95","line":531,"range":{"start_line":531,"start_character":23,"end_line":531,"end_character":40},"updated":"2019-09-10 14:27:54.000000000","message":"This appears to assume usage of passing in an exception object, not a string.\n\nException messages should use the _() translation marker so the strings get translated.","commit_id":"ca346ee35bff2950712c7e85a56c4c78d0c71f07"},{"author":{"_account_id":12988,"name":"Peter Penchev","email":"openstack-dev@storpool.com","username":"ppenchev"},"change_message_id":"800fb14edf7c98d66371b85a477e321286155b0f","unresolved":false,"context_lines":[{"line_number":528,"context_line":"                \u0027StorPool revert to snapshot failed: \u0027"},{"line_number":529,"context_line":"                \u0027could not delete volume {vname}: {err}\u0027"},{"line_number":530,"context_line":"            ).format(vname\u003dvolname, err\u003dsix.text_type(err))"},{"line_number":531,"context_line":"            raise self._backendException(msg)"},{"line_number":532,"context_line":"        try:"},{"line_number":533,"context_line":"            self._attach.api().volumeCreate({"},{"line_number":534,"context_line":"                \u0027name\u0027: volname,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_f096503e","line":531,"range":{"start_line":531,"start_character":23,"end_line":531,"end_character":40},"in_reply_to":"5faad753_a0aebf95","updated":"2019-10-14 12:17:53.000000000","message":"Right, I was thinking I would provide some more information, but now I don\u0027t think it is necessary any more, so this part has been changed (kind of fixed) in patchset 2. Thanks!","commit_id":"ca346ee35bff2950712c7e85a56c4c78d0c71f07"},{"author":{"_account_id":33431,"name":"Fábio Oliveira","email":"fabioaurelio1269@gmail.com","username":"fabiooliveira1"},"change_message_id":"48fa05df004dd721895790e92dd2c28709fc3b3c","unresolved":true,"context_lines":[{"line_number":451,"context_line":"        snapname \u003d self._attach.snapshotName(\u0027snap\u0027, snapshot[\u0027id\u0027])"},{"line_number":452,"context_line":"        try:"},{"line_number":453,"context_line":"            self._attach.api().volumeDelete(volname)"},{"line_number":454,"context_line":"        except spapi.ApiError as err:"},{"line_number":455,"context_line":"            raise self._backendException(err)"},{"line_number":456,"context_line":"        try:"},{"line_number":457,"context_line":"            self._attach.api().volumeCreate({"},{"line_number":458,"context_line":"                \u0027name\u0027: volname,"}],"source_content_type":"text/x-python","patch_set":9,"id":"58f7f4cd_d4b2b70b","line":455,"range":{"start_line":454,"start_character":8,"end_line":455,"end_character":45},"updated":"2021-08-12 15:44:17.000000000","message":"it would be nice to have helpful LOG messages on the exceptions to inform the users","commit_id":"ebb8c08631506236666bd1fe621856f35cd3673d"},{"author":{"_account_id":12988,"name":"Peter Penchev","email":"openstack-dev@storpool.com","username":"ppenchev"},"change_message_id":"840188273c18fde09da2d99403652ece77e18bc1","unresolved":false,"context_lines":[{"line_number":451,"context_line":"        snapname \u003d self._attach.snapshotName(\u0027snap\u0027, snapshot[\u0027id\u0027])"},{"line_number":452,"context_line":"        try:"},{"line_number":453,"context_line":"            self._attach.api().volumeDelete(volname)"},{"line_number":454,"context_line":"        except spapi.ApiError as err:"},{"line_number":455,"context_line":"            raise self._backendException(err)"},{"line_number":456,"context_line":"        try:"},{"line_number":457,"context_line":"            self._attach.api().volumeCreate({"},{"line_number":458,"context_line":"                \u0027name\u0027: volname,"}],"source_content_type":"text/x-python","patch_set":9,"id":"037d1620_831b7986","line":455,"range":{"start_line":454,"start_character":8,"end_line":455,"end_character":45},"in_reply_to":"58f7f4cd_d4b2b70b","updated":"2021-11-15 12:00:05.000000000","message":"Right, I had missed that here (it is done in other places in the driver). Thanks a lot!","commit_id":"ebb8c08631506236666bd1fe621856f35cd3673d"},{"author":{"_account_id":33431,"name":"Fábio Oliveira","email":"fabioaurelio1269@gmail.com","username":"fabiooliveira1"},"change_message_id":"48fa05df004dd721895790e92dd2c28709fc3b3c","unresolved":true,"context_lines":[{"line_number":453,"context_line":"            self._attach.api().volumeDelete(volname)"},{"line_number":454,"context_line":"        except spapi.ApiError as err:"},{"line_number":455,"context_line":"            raise self._backendException(err)"},{"line_number":456,"context_line":"        try:"},{"line_number":457,"context_line":"            self._attach.api().volumeCreate({"},{"line_number":458,"context_line":"                \u0027name\u0027: volname,"},{"line_number":459,"context_line":"                \u0027parent\u0027: snapname,"},{"line_number":460,"context_line":"            })"},{"line_number":461,"context_line":"        except spapi.ApiError as err:"},{"line_number":462,"context_line":"            raise self._backendException(err)"}],"source_content_type":"text/x-python","patch_set":9,"id":"343374db_941aeb8c","line":462,"range":{"start_line":456,"start_character":8,"end_line":462,"end_character":45},"updated":"2021-08-12 15:44:17.000000000","message":"If the volumeCreate fail, the original volume will be lost, since it has already been deleted on line 453.\nYou could use a temporary volume to make sure that if the revert fails the original volume won\u0027t be lost","commit_id":"ebb8c08631506236666bd1fe621856f35cd3673d"},{"author":{"_account_id":12988,"name":"Peter Penchev","email":"openstack-dev@storpool.com","username":"ppenchev"},"change_message_id":"840188273c18fde09da2d99403652ece77e18bc1","unresolved":false,"context_lines":[{"line_number":453,"context_line":"            self._attach.api().volumeDelete(volname)"},{"line_number":454,"context_line":"        except spapi.ApiError as err:"},{"line_number":455,"context_line":"            raise self._backendException(err)"},{"line_number":456,"context_line":"        try:"},{"line_number":457,"context_line":"            self._attach.api().volumeCreate({"},{"line_number":458,"context_line":"                \u0027name\u0027: volname,"},{"line_number":459,"context_line":"                \u0027parent\u0027: snapname,"},{"line_number":460,"context_line":"            })"},{"line_number":461,"context_line":"        except spapi.ApiError as err:"},{"line_number":462,"context_line":"            raise self._backendException(err)"}],"source_content_type":"text/x-python","patch_set":9,"id":"0c6f9114_5dda21df","line":462,"range":{"start_line":456,"start_character":8,"end_line":462,"end_character":45},"in_reply_to":"343374db_941aeb8c","updated":"2021-11-15 12:00:05.000000000","message":"This....... should not have been seen in patchsets after 5 at all; see my other comment about uploading the wrong changes :( But yeah, your comment was indeed valid for this version.","commit_id":"ebb8c08631506236666bd1fe621856f35cd3673d"},{"author":{"_account_id":33431,"name":"Fábio Oliveira","email":"fabioaurelio1269@gmail.com","username":"fabiooliveira1"},"change_message_id":"05b2779385ac620f9da33f8c607f9b0504f0d494","unresolved":true,"context_lines":[{"line_number":95,"context_line":"                - Implement revert_to_snapshot()."},{"line_number":96,"context_line":"    \"\"\""},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    VERSION \u003d \u00271.2.3\u0027"},{"line_number":99,"context_line":"    CI_WIKI_NAME \u003d \u0027StorPool_distributed_storage_CI\u0027"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def __init__(self, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":10,"id":"8f92806d_847483b5","line":98,"range":{"start_line":98,"start_character":4,"end_line":98,"end_character":21},"updated":"2022-01-20 19:45:52.000000000","message":"it doesn\u0027t match you version number history","commit_id":"278b85622bec40b063c789ce7cf26f324cbe7597"},{"author":{"_account_id":32919,"name":"kiran pawar","display_name":"Kiran Pawar","email":"kinpaa@gmail.com","username":"kpdev"},"change_message_id":"4773cd82a782de8c0ddad83bdafa742ecd0963c3","unresolved":false,"context_lines":[{"line_number":95,"context_line":"                - Implement revert_to_snapshot()."},{"line_number":96,"context_line":"    \"\"\""},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    VERSION \u003d \u00271.2.3\u0027"},{"line_number":99,"context_line":"    CI_WIKI_NAME \u003d \u0027StorPool_distributed_storage_CI\u0027"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def __init__(self, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":10,"id":"49c2a2a0_2a2f4dc9","line":98,"range":{"start_line":98,"start_character":4,"end_line":98,"end_character":21},"in_reply_to":"19a7a02e_791688d0","updated":"2024-06-12 11:26:46.000000000","message":"Done","commit_id":"278b85622bec40b063c789ce7cf26f324cbe7597"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"00f8edda48f59025b7e817248771f3fd95c49c3b","unresolved":true,"context_lines":[{"line_number":95,"context_line":"                - Implement revert_to_snapshot()."},{"line_number":96,"context_line":"    \"\"\""},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    VERSION \u003d \u00271.2.3\u0027"},{"line_number":99,"context_line":"    CI_WIKI_NAME \u003d \u0027StorPool_distributed_storage_CI\u0027"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def __init__(self, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":10,"id":"19a7a02e_791688d0","line":98,"range":{"start_line":98,"start_character":4,"end_line":98,"end_character":21},"in_reply_to":"8f92806d_847483b5","updated":"2024-05-29 13:54:50.000000000","message":"Yes, should be updated to 2.0.0.","commit_id":"278b85622bec40b063c789ce7cf26f324cbe7597"}]}
