)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":19144,"name":"José Mello","email":"jose.eduardo.jr@gmail.com","username":"jose-eduardo-jr"},"change_message_id":"992f4f022924487a4afc27b22e6f943d9b8ffd8b","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Improved Cinder snapshot tests"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch cover new edge cases that are impemented by different Cinder"},{"line_number":10,"context_line":"backends."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I34be2f311adddbb8faa3dace9e8b6bc05b4d93f2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3a98d1f4_b2372baf","line":9,"range":{"start_line":9,"start_character":41,"end_line":9,"end_character":51},"updated":"2016-09-29 19:30:08.000000000","message":"In case you submit a new patchset: implemented","commit_id":"1fe62cb042e0bae3090f7dd2568d4fad13b6a147"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"a3ff85c0b656df87362814d893a94f70645c6316","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Improved Cinder snapshot tests"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch cover new edge cases that are impemented by different Cinder"},{"line_number":10,"context_line":"backends."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I34be2f311adddbb8faa3dace9e8b6bc05b4d93f2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3a98d1f4_fe678fe6","line":9,"range":{"start_line":9,"start_character":41,"end_line":9,"end_character":51},"in_reply_to":"3a98d1f4_b2372baf","updated":"2016-09-30 14:15:43.000000000","message":"OK.","commit_id":"1fe62cb042e0bae3090f7dd2568d4fad13b6a147"}],"tempest/api/volume/base.py":[{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"844a7bdb6ac7b1d94440286033a3175b3407f511","unresolved":false,"context_lines":[{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    def attach_volume(self, server_id):"},{"line_number":159,"context_line":"        \"\"\"Attachs a volume so a server\"\"\""},{"line_number":160,"context_line":"        volume_id \u003d self.volume_origin[\u0027id\u0027]"},{"line_number":161,"context_line":"        self.servers_client.attach_volume("},{"line_number":162,"context_line":"            server_id, volumeId\u003dvolume_id,"},{"line_number":163,"context_line":"            device\u003d\u0027/dev/%s\u0027 % CONF.compute.volume_device_name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a629dbe_0aeb2b14","line":160,"updated":"2016-11-09 15:40:34.000000000","message":"What if a test class decided to not use a \u0027self.volume_origin\u0027 variable or what if the test class wanted to name that variable \u0027self.origin_volume\u0027 ?\n\nThis method should take another argument called \u0027volume_id\u0027 and not assume/force subclasses to use a specific variable.","commit_id":"d488e77ab70d56784b97d8d21bd6e81f9a80c71f"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"135fb82771e7e1ad8edf2b172819023149affd0c","unresolved":false,"context_lines":[{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    def attach_volume(self, server_id):"},{"line_number":159,"context_line":"        \"\"\"Attachs a volume so a server\"\"\""},{"line_number":160,"context_line":"        volume_id \u003d self.volume_origin[\u0027id\u0027]"},{"line_number":161,"context_line":"        self.servers_client.attach_volume("},{"line_number":162,"context_line":"            server_id, volumeId\u003dvolume_id,"},{"line_number":163,"context_line":"            device\u003d\u0027/dev/%s\u0027 % CONF.compute.volume_device_name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a629dbe_0f777d02","line":160,"in_reply_to":"9a629dbe_0aeb2b14","updated":"2016-11-11 10:43:18.000000000","message":"Done","commit_id":"d488e77ab70d56784b97d8d21bd6e81f9a80c71f"},{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"c3f85b47c821f9602a5697762af0a3e412cdcdc7","unresolved":false,"context_lines":[{"line_number":143,"context_line":"    def delete_snapshot(cls, snapshot):"},{"line_number":144,"context_line":"        \"\"\"Wrapper utility that deletes a snapshot.\"\"\""},{"line_number":145,"context_line":"        body \u003d cls.snapshots_client.delete_snapshot(snapshot[\u0027id\u0027])"},{"line_number":146,"context_line":"        cls.snapshots_client.wait_for_resource_deletion(snapshot[\u0027id\u0027])"},{"line_number":147,"context_line":"        return body"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"    # NOTE(afazekas): these create_* and clean_* could be defined"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a77a97e_38a2cc4b","line":146,"updated":"2016-11-18 02:10:33.000000000","message":"If you want to add this method, I think \n\n cls.snapshots.remove(snapshot)\n\nshould be added. Otherwise, it would be remained.\nBut I think it\u0027s almost same as clear_snapshot() in test_volumes_snapshots.py.","commit_id":"b4f9a1a05fec0dafb81a086f3dd6497e49eb1ae0"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"68923b1172e988db22a9395c64a306294c31fad0","unresolved":false,"context_lines":[{"line_number":143,"context_line":"    def delete_snapshot(cls, snapshot):"},{"line_number":144,"context_line":"        \"\"\"Wrapper utility that deletes a snapshot.\"\"\""},{"line_number":145,"context_line":"        body \u003d cls.snapshots_client.delete_snapshot(snapshot[\u0027id\u0027])"},{"line_number":146,"context_line":"        cls.snapshots_client.wait_for_resource_deletion(snapshot[\u0027id\u0027])"},{"line_number":147,"context_line":"        return body"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"    # NOTE(afazekas): these create_* and clean_* could be defined"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a77a97e_da145a4e","line":146,"in_reply_to":"7a77a97e_38a2cc4b","updated":"2016-11-18 15:50:14.000000000","message":"Good point. I\u0027ll remove this and use clear_snapshot().","commit_id":"b4f9a1a05fec0dafb81a086f3dd6497e49eb1ae0"},{"author":{"_account_id":9152,"name":"Castulo J. Martinez","email":"castulo.martinez@intel.com","username":"cas"},"change_message_id":"1d9cb1ceda1b233eb3459fe5b32a50fd2d59ecd1","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        \"\"\"Wrapper utility that deletes a snapshot.\"\"\""},{"line_number":145,"context_line":"        body \u003d cls.snapshots_client.delete_snapshot(snapshot[\u0027id\u0027])"},{"line_number":146,"context_line":"        cls.snapshots_client.wait_for_resource_deletion(snapshot[\u0027id\u0027])"},{"line_number":147,"context_line":"        return body"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"    # NOTE(afazekas): these create_* and clean_* could be defined"},{"line_number":150,"context_line":"    # only in a single location in the source, and could be more general."}],"source_content_type":"text/x-python","patch_set":8,"id":"7a77a97e_f1d323d3","line":147,"range":{"start_line":147,"start_character":8,"end_line":147,"end_character":19},"updated":"2016-11-17 17:56:03.000000000","message":"What is the point of returning the body? Other delete functions like delete_volume below does not return anything.","commit_id":"b4f9a1a05fec0dafb81a086f3dd6497e49eb1ae0"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"68923b1172e988db22a9395c64a306294c31fad0","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        \"\"\"Wrapper utility that deletes a snapshot.\"\"\""},{"line_number":145,"context_line":"        body \u003d cls.snapshots_client.delete_snapshot(snapshot[\u0027id\u0027])"},{"line_number":146,"context_line":"        cls.snapshots_client.wait_for_resource_deletion(snapshot[\u0027id\u0027])"},{"line_number":147,"context_line":"        return body"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"    # NOTE(afazekas): these create_* and clean_* could be defined"},{"line_number":150,"context_line":"    # only in a single location in the source, and could be more general."}],"source_content_type":"text/x-python","patch_set":8,"id":"7a77a97e_1a0b322b","line":147,"range":{"start_line":147,"start_character":8,"end_line":147,"end_character":19},"in_reply_to":"7a77a97e_f1d323d3","updated":"2016-11-18 15:50:14.000000000","message":"Done","commit_id":"b4f9a1a05fec0dafb81a086f3dd6497e49eb1ae0"},{"author":{"_account_id":9152,"name":"Castulo J. Martinez","email":"castulo.martinez@intel.com","username":"cas"},"change_message_id":"1d9cb1ceda1b233eb3459fe5b32a50fd2d59ecd1","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        client.wait_for_resource_deletion(volume_id)"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    def attach_volume(self, server_id, volume_id):"},{"line_number":159,"context_line":"        \"\"\"Attachs a volume so a server\"\"\""},{"line_number":160,"context_line":"        self.servers_client.attach_volume("},{"line_number":161,"context_line":"            server_id, volumeId\u003dvolume_id,"},{"line_number":162,"context_line":"            device\u003d\u0027/dev/%s\u0027 % CONF.compute.volume_device_name)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a77a97e_f1cc03bd","line":159,"range":{"start_line":159,"start_character":28,"end_line":159,"end_character":30},"updated":"2016-11-17 17:56:03.000000000","message":"typo","commit_id":"b4f9a1a05fec0dafb81a086f3dd6497e49eb1ae0"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"68923b1172e988db22a9395c64a306294c31fad0","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        client.wait_for_resource_deletion(volume_id)"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    def attach_volume(self, server_id, volume_id):"},{"line_number":159,"context_line":"        \"\"\"Attachs a volume so a server\"\"\""},{"line_number":160,"context_line":"        self.servers_client.attach_volume("},{"line_number":161,"context_line":"            server_id, volumeId\u003dvolume_id,"},{"line_number":162,"context_line":"            device\u003d\u0027/dev/%s\u0027 % CONF.compute.volume_device_name)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a77a97e_3c054b10","line":159,"range":{"start_line":159,"start_character":28,"end_line":159,"end_character":30},"in_reply_to":"7a77a97e_f1cc03bd","updated":"2016-11-18 15:50:14.000000000","message":"Done","commit_id":"b4f9a1a05fec0dafb81a086f3dd6497e49eb1ae0"},{"author":{"_account_id":9152,"name":"Castulo J. Martinez","email":"castulo.martinez@intel.com","username":"cas"},"change_message_id":"1d9cb1ceda1b233eb3459fe5b32a50fd2d59ecd1","unresolved":false,"context_lines":[{"line_number":162,"context_line":"            device\u003d\u0027/dev/%s\u0027 % CONF.compute.volume_device_name)"},{"line_number":163,"context_line":"        waiters.wait_for_volume_status(self.volumes_client,"},{"line_number":164,"context_line":"                                       volume_id, \u0027in-use\u0027)"},{"line_number":165,"context_line":"        self.addCleanup(waiters.wait_for_volume_status, self.volumes_client,"},{"line_number":166,"context_line":"                        volume_id, \u0027available\u0027)"},{"line_number":167,"context_line":"        self.addCleanup(self.servers_client.detach_volume, server_id,"},{"line_number":168,"context_line":"                        self.volume_origin[\u0027id\u0027])"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    @classmethod"},{"line_number":171,"context_line":"    def clear_volumes(cls):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a77a97e_714473b6","line":168,"range":{"start_line":165,"start_character":8,"end_line":168,"end_character":49},"updated":"2016-11-17 17:56:03.000000000","message":"IMO this would look cleaner if the addCleanup would call a detach_volume function that had the detach plus the waiting, it kind of looks confusing like this","commit_id":"b4f9a1a05fec0dafb81a086f3dd6497e49eb1ae0"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"77e7de57249c335db1e24b5ab25b372a5505dbe1","unresolved":false,"context_lines":[{"line_number":162,"context_line":"            device\u003d\u0027/dev/%s\u0027 % CONF.compute.volume_device_name)"},{"line_number":163,"context_line":"        waiters.wait_for_volume_status(self.volumes_client,"},{"line_number":164,"context_line":"                                       volume_id, \u0027in-use\u0027)"},{"line_number":165,"context_line":"        self.addCleanup(waiters.wait_for_volume_status, self.volumes_client,"},{"line_number":166,"context_line":"                        volume_id, \u0027available\u0027)"},{"line_number":167,"context_line":"        self.addCleanup(self.servers_client.detach_volume, server_id,"},{"line_number":168,"context_line":"                        self.volume_origin[\u0027id\u0027])"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    @classmethod"},{"line_number":171,"context_line":"    def clear_volumes(cls):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a77a97e_ca410c65","line":168,"range":{"start_line":165,"start_character":8,"end_line":168,"end_character":49},"in_reply_to":"7a77a97e_714473b6","updated":"2016-11-17 18:31:15.000000000","message":"I am not sure I agree. It\u0027s quite common to chain the addCleanup like this and I am not sure adding yet another function would help readability.","commit_id":"b4f9a1a05fec0dafb81a086f3dd6497e49eb1ae0"}],"tempest/api/volume/test_volumes_snapshots.py":[{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"07daa88212d7d33302627932d76917c2aec68233","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        # Create a snapshot while it is not attached"},{"line_number":103,"context_line":"        snapshot1 \u003d self.create_snapshot(self.volume_origin[\u0027id\u0027])"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        # Create a srver and attach it"},{"line_number":106,"context_line":"        server_name \u003d data_utils.rand_name("},{"line_number":107,"context_line":"            self.__class__.__name__ + \u0027-instance\u0027)"},{"line_number":108,"context_line":"        server \u003d self.create_server("}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_3b888c22","line":105,"updated":"2016-09-29 04:17:07.000000000","message":"s/srver/server","commit_id":"1fe62cb042e0bae3090f7dd2568d4fad13b6a147"},{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"07daa88212d7d33302627932d76917c2aec68233","unresolved":false,"context_lines":[{"line_number":124,"context_line":""},{"line_number":125,"context_line":"        # Delete the snapshots. Some snapshot implementations can take"},{"line_number":126,"context_line":"        # different paths according to order they are deleted."},{"line_number":127,"context_line":"        self.delete_snapshot(snapshot3)"},{"line_number":128,"context_line":"        self.delete_snapshot(snapshot1)"},{"line_number":129,"context_line":"        self.delete_snapshot(snapshot2)"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_5b8d5830","line":127,"updated":"2016-09-29 04:17:07.000000000","message":"I\u0027m just curious. Why should we delete snapshot3 first here? In the above attached test, you delete snapshot1 first.","commit_id":"1fe62cb042e0bae3090f7dd2568d4fad13b6a147"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"4f9746fc03e4c9fc9cc84de4b2e4c9fde10113d6","unresolved":false,"context_lines":[{"line_number":124,"context_line":""},{"line_number":125,"context_line":"        # Delete the snapshots. Some snapshot implementations can take"},{"line_number":126,"context_line":"        # different paths according to order they are deleted."},{"line_number":127,"context_line":"        self.delete_snapshot(snapshot3)"},{"line_number":128,"context_line":"        self.delete_snapshot(snapshot1)"},{"line_number":129,"context_line":"        self.delete_snapshot(snapshot2)"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_018a7abf","line":127,"in_reply_to":"3a98d1f4_5b8d5830","updated":"2016-09-29 08:45:41.000000000","message":"Because the order of each volume in the chain of snapshots is relevant and testing several different scenarios will increase coverage.","commit_id":"1fe62cb042e0bae3090f7dd2568d4fad13b6a147"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"6abcd87b1d7f87a22beed193411d94d8a2be8828","unresolved":false,"context_lines":[{"line_number":124,"context_line":""},{"line_number":125,"context_line":"        # Delete the snapshots. Some snapshot implementations can take"},{"line_number":126,"context_line":"        # different paths according to order they are deleted."},{"line_number":127,"context_line":"        self.delete_snapshot(snapshot3)"},{"line_number":128,"context_line":"        self.delete_snapshot(snapshot1)"},{"line_number":129,"context_line":"        self.delete_snapshot(snapshot2)"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_54def46d","line":127,"in_reply_to":"3a98d1f4_5b8d5830","updated":"2016-09-29 11:43:29.000000000","message":"Hi Masayuki, this is for coverage. If snap3 is deleted, the driver (if it uses QEMU QCOW images) will perform a block rebase operation, moving all data from the snap3 file to the active image. If snap1 is deleted, the driver will do a block commit[1]. Using one order in each test will cover those 2 ways.\n\n[1] https://kashyapc.fedorapeople.org/virt/lc-2012/snapshots-handout.html","commit_id":"1fe62cb042e0bae3090f7dd2568d4fad13b6a147"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"d0f8ec1ca91521de486c62addf2c1145fc188eac","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        server \u003d self.create_server("},{"line_number":75,"context_line":"            name\u003dserver_name,"},{"line_number":76,"context_line":"            wait_until\u003d\u0027ACTIVE\u0027)"},{"line_number":77,"context_line":"        self.servers_client.attach_volume("},{"line_number":78,"context_line":"            server[\u0027id\u0027], volumeId\u003dself.volume_origin[\u0027id\u0027],"},{"line_number":79,"context_line":"            device\u003d\u0027/dev/%s\u0027 % CONF.compute.volume_device_name)"},{"line_number":80,"context_line":"        waiters.wait_for_volume_status(self.volumes_client,"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a95cdbc_46c62849","line":77,"updated":"2016-10-06 11:13:07.000000000","message":"Could have a look at the attach_volume method tempest/api/compute/base.py and put it also in tempest/api/volume/base.py ? Or move it in a common place.\n\nTo avoid code duplication","commit_id":"308ff6660ab75804737dd9bb4292bea9a362b141"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"f1be84bd457a15a860d507e7c5cab613815a4b2e","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        server \u003d self.create_server("},{"line_number":75,"context_line":"            name\u003dserver_name,"},{"line_number":76,"context_line":"            wait_until\u003d\u0027ACTIVE\u0027)"},{"line_number":77,"context_line":"        self.servers_client.attach_volume("},{"line_number":78,"context_line":"            server[\u0027id\u0027], volumeId\u003dself.volume_origin[\u0027id\u0027],"},{"line_number":79,"context_line":"            device\u003d\u0027/dev/%s\u0027 % CONF.compute.volume_device_name)"},{"line_number":80,"context_line":"        waiters.wait_for_volume_status(self.volumes_client,"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a95cdbc_f5e6e5b2","line":77,"in_reply_to":"1a95cdbc_46c62849","updated":"2016-10-06 16:39:31.000000000","message":"You mean: tempest/services/volume/base/base_volumes_client.py:102\ntempest/lib/services/compute/servers_client.py:380\n?\nBut, I think this is work for another patch as I\u0027m not adding the attach_volume method.","commit_id":"308ff6660ab75804737dd9bb4292bea9a362b141"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"3fb6ea60ecc1e6297aef445f0efd67dd6d54db41","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        server \u003d self.create_server("},{"line_number":75,"context_line":"            name\u003dserver_name,"},{"line_number":76,"context_line":"            wait_until\u003d\u0027ACTIVE\u0027)"},{"line_number":77,"context_line":"        self.servers_client.attach_volume("},{"line_number":78,"context_line":"            server[\u0027id\u0027], volumeId\u003dself.volume_origin[\u0027id\u0027],"},{"line_number":79,"context_line":"            device\u003d\u0027/dev/%s\u0027 % CONF.compute.volume_device_name)"},{"line_number":80,"context_line":"        waiters.wait_for_volume_status(self.volumes_client,"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a95cdbc_c9ae9680","line":77,"in_reply_to":"1a95cdbc_686f5cc5","updated":"2016-10-07 09:23:16.000000000","message":"No I really meant \"attach_volume method tempest/api/compute/base.py\".\n\nWhat I\u0027d like to see is a function that do\n1) servers_client.attach_volume\n2) waiters.wait_for_volume_status\n3) addCleanup(waiters.wait_for_volume_status\n4) addCleanup(self.servers_client.detach_volume\n\nI suggest you put this function in a common place (such as tempest/api/volume/base.py) and that you reuse that function.\n\nI mean L77-L85 is exactly the same than L111-L119, maybe copy/pasta code could be avoided.","commit_id":"308ff6660ab75804737dd9bb4292bea9a362b141"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"c681c6a4793ec8bf48d8e02d2ebeb902f369beae","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        server \u003d self.create_server("},{"line_number":75,"context_line":"            name\u003dserver_name,"},{"line_number":76,"context_line":"            wait_until\u003d\u0027ACTIVE\u0027)"},{"line_number":77,"context_line":"        self.servers_client.attach_volume("},{"line_number":78,"context_line":"            server[\u0027id\u0027], volumeId\u003dself.volume_origin[\u0027id\u0027],"},{"line_number":79,"context_line":"            device\u003d\u0027/dev/%s\u0027 % CONF.compute.volume_device_name)"},{"line_number":80,"context_line":"        waiters.wait_for_volume_status(self.volumes_client,"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a95cdbc_686f5cc5","line":77,"in_reply_to":"1a95cdbc_f5e6e5b2","updated":"2016-10-06 16:43:52.000000000","message":"Hmmm, they are not the same. \nThe compute post to: \u0027servers/%s/os-volume_attachments\u0027\nwhile volume client: \u0027volumes/%s/action\u0027","commit_id":"308ff6660ab75804737dd9bb4292bea9a362b141"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"92396760203d1fb8d406cc048fee958943991605","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        snapshot2 \u003d self.create_snapshot(self.volume_origin[\u0027id\u0027], force\u003dTrue)"},{"line_number":66,"context_line":"        snapshot3 \u003d self.create_snapshot(self.volume_origin[\u0027id\u0027], force\u003dTrue)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        # Delete the snapshots. Some snapshot implementations can take"},{"line_number":69,"context_line":"        # different paths according to order they are deleted."},{"line_number":70,"context_line":"        self.delete_snapshot(snapshot1)"},{"line_number":71,"context_line":"        self.delete_snapshot(snapshot3)"},{"line_number":72,"context_line":"        self.delete_snapshot(snapshot2)"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a77a97e_f6d015cc","line":69,"range":{"start_line":68,"start_character":0,"end_line":69,"end_character":62},"updated":"2016-11-14 23:55:04.000000000","message":"Maybe this comment wants to explain the following odd order of snapshot deletions, but I cannot understand the reason TBH.\nAre there any actual bug related to this part?\nIf so, it would be nice to add \"Related-Bug: #XXXXX\" on the commit message.","commit_id":"f7dfe1417676f121dd53f59edc42054b698efad8"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"ca27a7249d8a02faa001fc34935dc755ef5f1ef7","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        snapshot2 \u003d self.create_snapshot(self.volume_origin[\u0027id\u0027], force\u003dTrue)"},{"line_number":66,"context_line":"        snapshot3 \u003d self.create_snapshot(self.volume_origin[\u0027id\u0027], force\u003dTrue)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        # Delete the snapshots. Some snapshot implementations can take"},{"line_number":69,"context_line":"        # different paths according to order they are deleted."},{"line_number":70,"context_line":"        self.delete_snapshot(snapshot1)"},{"line_number":71,"context_line":"        self.delete_snapshot(snapshot3)"},{"line_number":72,"context_line":"        self.delete_snapshot(snapshot2)"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a77a97e_6ea208b0","line":69,"range":{"start_line":68,"start_character":0,"end_line":69,"end_character":62},"in_reply_to":"7a77a97e_f6d015cc","updated":"2016-11-17 11:05:30.000000000","message":"Added a better description in the commit message.","commit_id":"f7dfe1417676f121dd53f59edc42054b698efad8"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"92396760203d1fb8d406cc048fee958943991605","unresolved":false,"context_lines":[{"line_number":75,"context_line":"    @test.services(\u0027compute\u0027)"},{"line_number":76,"context_line":"    def test_snapshot_create_offline_delete_online(self):"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        # Create a snapshot while it is not attached"},{"line_number":79,"context_line":"        snapshot1 \u003d self.create_snapshot(self.volume_origin[\u0027id\u0027])"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"        # Create a server and attach it"},{"line_number":82,"context_line":"        server \u003d self.create_server(wait_until\u003d\u0027ACTIVE\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a77a97e_769325fb","line":79,"range":{"start_line":78,"start_character":0,"end_line":79,"end_character":66},"updated":"2016-11-14 23:55:04.000000000","message":"volume_origin is shared with multiple tests and it would be easy to make the image unexpected status in the future by adding multiple tests later.\nSo how about adding assertion for checking \"available\" status before line 79 ?","commit_id":"f7dfe1417676f121dd53f59edc42054b698efad8"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"ca27a7249d8a02faa001fc34935dc755ef5f1ef7","unresolved":false,"context_lines":[{"line_number":75,"context_line":"    @test.services(\u0027compute\u0027)"},{"line_number":76,"context_line":"    def test_snapshot_create_offline_delete_online(self):"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        # Create a snapshot while it is not attached"},{"line_number":79,"context_line":"        snapshot1 \u003d self.create_snapshot(self.volume_origin[\u0027id\u0027])"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"        # Create a server and attach it"},{"line_number":82,"context_line":"        server \u003d self.create_server(wait_until\u003d\u0027ACTIVE\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a77a97e_aee8a0b5","line":79,"range":{"start_line":78,"start_character":0,"end_line":79,"end_character":66},"in_reply_to":"7a77a97e_769325fb","updated":"2016-11-17 11:05:30.000000000","message":"Taking snapshots of a volume can\u0027t change the status of the volume. If in any case the backend falls in error and put the volume in error, Cinder does not have a way to know that, so, will continue to return \u0027available\u0027 even if the volume is broken in the BE.","commit_id":"f7dfe1417676f121dd53f59edc42054b698efad8"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"92396760203d1fb8d406cc048fee958943991605","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        snapshot2 \u003d self.create_snapshot(self.volume_origin[\u0027id\u0027], force\u003dTrue)"},{"line_number":87,"context_line":"        snapshot3 \u003d self.create_snapshot(self.volume_origin[\u0027id\u0027], force\u003dTrue)"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"        # Delete the snapshots. Some snapshot implementations can take"},{"line_number":90,"context_line":"        # different paths according to order they are deleted."},{"line_number":91,"context_line":"        self.delete_snapshot(snapshot3)"},{"line_number":92,"context_line":"        self.delete_snapshot(snapshot1)"},{"line_number":93,"context_line":"        self.delete_snapshot(snapshot2)"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    @test.idempotent_id(\u00272a8abbe4-d871-46db-b049-c41f5af8216e\u0027)"},{"line_number":96,"context_line":"    def test_snapshot_create_get_list_update_delete(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a77a97e_1672f1e5","line":93,"range":{"start_line":89,"start_character":0,"end_line":93,"end_character":39},"updated":"2016-11-14 23:55:04.000000000","message":"ditto","commit_id":"f7dfe1417676f121dd53f59edc42054b698efad8"},{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"c3f85b47c821f9602a5697762af0a3e412cdcdc7","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        # Delete the snapshots. Some snapshot implementations can take"},{"line_number":69,"context_line":"        # different paths according to order they are deleted."},{"line_number":70,"context_line":"        self.delete_snapshot(snapshot1)"},{"line_number":71,"context_line":"        self.delete_snapshot(snapshot3)"},{"line_number":72,"context_line":"        self.delete_snapshot(snapshot2)"},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7a77a97e_189d100a","line":70,"updated":"2016-11-18 02:10:33.000000000","message":"I agree with Castulo and Jordan here. I\u0027d like to know the reason to add the new delete_snapshot() method.","commit_id":"b4f9a1a05fec0dafb81a086f3dd6497e49eb1ae0"},{"author":{"_account_id":9152,"name":"Castulo J. Martinez","email":"castulo.martinez@intel.com","username":"cas"},"change_message_id":"1d9cb1ceda1b233eb3459fe5b32a50fd2d59ecd1","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        # Delete the snapshots. Some snapshot implementations can take"},{"line_number":69,"context_line":"        # different paths according to order they are deleted."},{"line_number":70,"context_line":"        self.delete_snapshot(snapshot1)"},{"line_number":71,"context_line":"        self.delete_snapshot(snapshot3)"},{"line_number":72,"context_line":"        self.delete_snapshot(snapshot2)"},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7a77a97e_340bc92b","line":70,"range":{"start_line":70,"start_character":13,"end_line":70,"end_character":28},"updated":"2016-11-17 17:56:03.000000000","message":"Why are you using this delete_snapshot function instead of using the cleanup_snapshot function that is in line 36? That is going to cause the snapshots to be left in the snapshots list","commit_id":"b4f9a1a05fec0dafb81a086f3dd6497e49eb1ae0"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"68923b1172e988db22a9395c64a306294c31fad0","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        # Delete the snapshots. Some snapshot implementations can take"},{"line_number":69,"context_line":"        # different paths according to order they are deleted."},{"line_number":70,"context_line":"        self.delete_snapshot(snapshot1)"},{"line_number":71,"context_line":"        self.delete_snapshot(snapshot3)"},{"line_number":72,"context_line":"        self.delete_snapshot(snapshot2)"},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7a77a97e_bc115b54","line":70,"in_reply_to":"7a77a97e_189d100a","updated":"2016-11-18 15:50:14.000000000","message":"Done","commit_id":"b4f9a1a05fec0dafb81a086f3dd6497e49eb1ae0"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"77e7de57249c335db1e24b5ab25b372a5505dbe1","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        # Delete the snapshots. Some snapshot implementations can take"},{"line_number":69,"context_line":"        # different paths according to order they are deleted."},{"line_number":70,"context_line":"        self.delete_snapshot(snapshot1)"},{"line_number":71,"context_line":"        self.delete_snapshot(snapshot3)"},{"line_number":72,"context_line":"        self.delete_snapshot(snapshot2)"},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7a77a97e_a59c01c1","line":70,"range":{"start_line":70,"start_character":13,"end_line":70,"end_character":28},"in_reply_to":"7a77a97e_340bc92b","updated":"2016-11-17 18:31:15.000000000","message":"Good point. If you want to introduce a new method \u0027delete_snapshot\u0027 then you should remove the usage of cleanup_snapshot. otherwise it\u0027s duplication.","commit_id":"b4f9a1a05fec0dafb81a086f3dd6497e49eb1ae0"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"68923b1172e988db22a9395c64a306294c31fad0","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        # Delete the snapshots. Some snapshot implementations can take"},{"line_number":69,"context_line":"        # different paths according to order they are deleted."},{"line_number":70,"context_line":"        self.delete_snapshot(snapshot1)"},{"line_number":71,"context_line":"        self.delete_snapshot(snapshot3)"},{"line_number":72,"context_line":"        self.delete_snapshot(snapshot2)"},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7a77a97e_6a49d8b6","line":70,"range":{"start_line":70,"start_character":13,"end_line":70,"end_character":28},"in_reply_to":"7a77a97e_340bc92b","updated":"2016-11-18 15:50:14.000000000","message":"No it wont","commit_id":"b4f9a1a05fec0dafb81a086f3dd6497e49eb1ae0"}]}