)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":22692,"name":"guo yunxian","email":"yunxian.guo@easystack.cn","username":"guoyunxian"},"change_message_id":"4eff6909110ff092ce0d0a9b3eb55fb55ac0c01e","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add a generic method for backup creation"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The purpose of this method is to create a volume backup, wait for completion"},{"line_number":10,"context_line":"and delete it in the end fo the test case."},{"line_number":11,"context_line":"Using this method can eliminate a duplicated code."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: Ibdfa66399391e284939b39d550a9dbbe2bffa561"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"1a95cdbc_acea0f72","line":10,"range":{"start_line":10,"start_character":25,"end_line":10,"end_character":27},"updated":"2016-10-08 10:55:13.000000000","message":"A small mistake， should be of","commit_id":"ba20676e6fd8593100af41a8426ed9e56a9f2a64"}],"tempest/api/volume/base.py":[{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"81385003b8a8519d5df62ede86447e07799cb3b7","unresolved":false,"context_lines":[{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    @classmethod"},{"line_number":138,"context_line":"    def create_backup(cls, volume_id, **kwargs):"},{"line_number":139,"context_line":"        \"\"\"Wrapper utility that returns a test backup.\"\"\""},{"line_number":140,"context_line":"        backup \u003d cls.backups_client.create_backup("},{"line_number":141,"context_line":"            volume_id\u003dvolume_id, **kwargs)[\u0027backup\u0027]"},{"line_number":142,"context_line":"        cls.backups.append(backup)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3ac371cc_4d68bc87","line":139,"updated":"2016-08-17 10:37:04.000000000","message":"Hi Liron ,\nThe create_backup is used by admin client too , i dont see any reference to admin client .\nMaybe you can handle an admin client too and fix all places for admin and non-admin clients.\n\nThanks,\nBenny","commit_id":"8fc38eb5a61e1cd8141f45674e22a0e580dbcb4f"},{"author":{"_account_id":9152,"name":"Castulo J. Martinez","email":"castulo.martinez@intel.com","username":"cas"},"change_message_id":"87e54b32ae75648910dcca624f5ba51138d52783","unresolved":false,"context_lines":[{"line_number":185,"context_line":"    def clear_backups(cls):"},{"line_number":186,"context_line":"        for backup in cls.backups:"},{"line_number":187,"context_line":"            try:"},{"line_number":188,"context_line":"                cls.backups_client.delete_backup(backup[\u0027id\u0027])"},{"line_number":189,"context_line":"            except Exception:"},{"line_number":190,"context_line":"                pass"},{"line_number":191,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_0391aa52","line":188,"range":{"start_line":188,"start_character":16,"end_line":188,"end_character":62},"updated":"2016-08-25 15:54:59.000000000","message":"I think you should be using the call_and_ignore_notfound_exc function:\nhttps://github.com/openstack/tempest/blob/master/tempest/lib/common/utils/test_utils.py#L80\n\nHere is an example:\nhttps://github.com/openstack/tempest/blob/master/tempest/api/volume/base.py#L258","commit_id":"863bff13bd12f327105cd7bd148320043c453870"},{"author":{"_account_id":19262,"name":"Liron Kuchlani","email":"lkuchlan@redhat.com","username":"lkuchlan"},"change_message_id":"b4942873ed60c7e90bd729f51b934c309ca88d7a","unresolved":false,"context_lines":[{"line_number":185,"context_line":"    def clear_backups(cls):"},{"line_number":186,"context_line":"        for backup in cls.backups:"},{"line_number":187,"context_line":"            try:"},{"line_number":188,"context_line":"                cls.backups_client.delete_backup(backup[\u0027id\u0027])"},{"line_number":189,"context_line":"            except Exception:"},{"line_number":190,"context_line":"                pass"},{"line_number":191,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa7ab95a_a7bbac71","line":188,"range":{"start_line":188,"start_character":16,"end_line":188,"end_character":62},"in_reply_to":"1ac06dbe_0391aa52","updated":"2016-08-26 10:47:35.000000000","message":"Thanks for your comment, I\u0027ll update the method to use the helper function","commit_id":"863bff13bd12f327105cd7bd148320043c453870"},{"author":{"_account_id":9152,"name":"Castulo J. Martinez","email":"castulo.martinez@intel.com","username":"cas"},"change_message_id":"87e54b32ae75648910dcca624f5ba51138d52783","unresolved":false,"context_lines":[{"line_number":190,"context_line":"                pass"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"        for backup in cls.backups:"},{"line_number":193,"context_line":"            try:"},{"line_number":194,"context_line":"                cls.backups_client.wait_for_resource_deletion(backup[\u0027id\u0027])"},{"line_number":195,"context_line":"            except Exception:"},{"line_number":196,"context_line":"                pass"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_838bda1d","line":193,"range":{"start_line":193,"start_character":12,"end_line":193,"end_character":16},"updated":"2016-08-25 15:54:59.000000000","message":"Same as above","commit_id":"863bff13bd12f327105cd7bd148320043c453870"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"f1ea84280cbdde07036330e6c74aac9a8b6fbf5e","unresolved":false,"context_lines":[{"line_number":135,"context_line":"        return snapshot"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    @classmethod"},{"line_number":138,"context_line":"    def create_backup(cls, volume_id, **kwargs):"},{"line_number":139,"context_line":"        \"\"\"Wrapper utility that returns a test backup.\"\"\""},{"line_number":140,"context_line":"        backup \u003d cls.backups_client.create_backup("},{"line_number":141,"context_line":"            volume_id\u003dvolume_id, **kwargs)[\u0027backup\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa7ab95a_41a517b3","line":138,"updated":"2016-08-29 08:30:05.000000000","message":"Could tests in in api/volume/admin/test_volumes_backup.py also use this helper ?","commit_id":"428afe6a312325194de00bfeee98168ee8dfde01"},{"author":{"_account_id":19262,"name":"Liron Kuchlani","email":"lkuchlan@redhat.com","username":"lkuchlan"},"change_message_id":"97b2b56620286ac50f88b4fbd8e02fcf4ec64563","unresolved":false,"context_lines":[{"line_number":135,"context_line":"        return snapshot"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    @classmethod"},{"line_number":138,"context_line":"    def create_backup(cls, volume_id, **kwargs):"},{"line_number":139,"context_line":"        \"\"\"Wrapper utility that returns a test backup.\"\"\""},{"line_number":140,"context_line":"        backup \u003d cls.backups_client.create_backup("},{"line_number":141,"context_line":"            volume_id\u003dvolume_id, **kwargs)[\u0027backup\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa7ab95a_faedf4ef","line":138,"in_reply_to":"fa7ab95a_41a517b3","updated":"2016-08-29 09:20:41.000000000","message":"No, the method is under BaseVolumeTest class which is not be intended for an admin actions","commit_id":"428afe6a312325194de00bfeee98168ee8dfde01"},{"author":{"_account_id":19262,"name":"Liron Kuchlani","email":"lkuchlan@redhat.com","username":"lkuchlan"},"change_message_id":"cc4564ff218f7d7a2e388ded10adb037c1522fef","unresolved":false,"context_lines":[{"line_number":135,"context_line":"        return snapshot"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    @classmethod"},{"line_number":138,"context_line":"    def create_backup(cls, volume_id, **kwargs):"},{"line_number":139,"context_line":"        \"\"\"Wrapper utility that returns a test backup.\"\"\""},{"line_number":140,"context_line":"        backup \u003d cls.backups_client.create_backup("},{"line_number":141,"context_line":"            volume_id\u003dvolume_id, **kwargs)[\u0027backup\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa7ab95a_d8dace4a","line":138,"in_reply_to":"fa7ab95a_54605ea5","updated":"2016-08-29 20:03:30.000000000","message":"Yes I know, sorry for my misleading response,\nbut for that I should rename both clients in\nsetup_clients method: cls.backups_client \nand cls.backups_admin_client to a common name.\nAnd I am not sure about this. \n\nfor example:\n-------------\nin BaseVolumeTest\ndef setup_client(cls)\n    client \u003d cls.os.backups_v2_client\n\nin BaseVolumeAdminTest\ndef setup_client(cls)\n    client \u003d cls.os_adm.backups_v2_client\n\n\n@classmethod\ndef create_backup(cls, volume_id, **kwargs):\n    backup \u003d client.create_backup(\n             volume_id\u003dvolume_id, **kwargs)[\u0027backup\u0027]\n\nWhat do you suggest?","commit_id":"428afe6a312325194de00bfeee98168ee8dfde01"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"538462f02611e3d693cc75b49424e89e37dd0ba2","unresolved":false,"context_lines":[{"line_number":135,"context_line":"        return snapshot"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    @classmethod"},{"line_number":138,"context_line":"    def create_backup(cls, volume_id, **kwargs):"},{"line_number":139,"context_line":"        \"\"\"Wrapper utility that returns a test backup.\"\"\""},{"line_number":140,"context_line":"        backup \u003d cls.backups_client.create_backup("},{"line_number":141,"context_line":"            volume_id\u003dvolume_id, **kwargs)[\u0027backup\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa7ab95a_54605ea5","line":138,"in_reply_to":"fa7ab95a_faedf4ef","updated":"2016-08-29 14:56:38.000000000","message":"But BaseVolumeAdminTest inherits from BaseVolumeTest, so any method defined in BaseVolumeTest could be used in BaseVolumeAdminTest.","commit_id":"428afe6a312325194de00bfeee98168ee8dfde01"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"29d7b92c774e3af3990a37faf3a95c853493d721","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        if cls._api_version \u003d\u003d 1:"},{"line_number":69,"context_line":"            cls.snapshots_client \u003d cls.os.snapshots_client"},{"line_number":70,"context_line":"            cls.volumes_client \u003d cls.os.volumes_client"},{"line_number":71,"context_line":"            cls.backups_client \u003d cls.backupClient \u003d cls.os.backups_client"},{"line_number":72,"context_line":"            cls.volume_services_client \u003d cls.os.volume_services_client"},{"line_number":73,"context_line":"            cls.volumes_extension_client \u003d cls.os.volumes_extension_client"},{"line_number":74,"context_line":"            cls.availability_zone_client \u003d ("}],"source_content_type":"text/x-python","patch_set":6,"id":"fa7ab95a_dbc38136","line":71,"updated":"2016-08-30 11:26:39.000000000","message":"Introducing a new variable \"backupClient\" is really not nice. Especially without a good code comment about it.\n\nI suggest that your new create_backup method takes a 2nd argument named \u0027backup_client\u003dNone\u0027, where the caller pass the actual backup client he wants to actually use. Inside the method, if the passed backup_client is None, then use self.backups_client.","commit_id":"8b1316ebb31fff5ada279b3d68323b86d59f71c9"},{"author":{"_account_id":19262,"name":"Liron Kuchlani","email":"lkuchlan@redhat.com","username":"lkuchlan"},"change_message_id":"b894031cbc883b5a476063ef9cb2561544eadd5c","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        if cls._api_version \u003d\u003d 1:"},{"line_number":69,"context_line":"            cls.snapshots_client \u003d cls.os.snapshots_client"},{"line_number":70,"context_line":"            cls.volumes_client \u003d cls.os.volumes_client"},{"line_number":71,"context_line":"            cls.backups_client \u003d cls.backupClient \u003d cls.os.backups_client"},{"line_number":72,"context_line":"            cls.volume_services_client \u003d cls.os.volume_services_client"},{"line_number":73,"context_line":"            cls.volumes_extension_client \u003d cls.os.volumes_extension_client"},{"line_number":74,"context_line":"            cls.availability_zone_client \u003d ("}],"source_content_type":"text/x-python","patch_set":6,"id":"fa7ab95a_4d432f6c","line":71,"in_reply_to":"fa7ab95a_dbc38136","updated":"2016-08-30 13:28:50.000000000","message":"Ok I accept your offer, thanks","commit_id":"8b1316ebb31fff5ada279b3d68323b86d59f71c9"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"d5d5c820878b4f9496e5c61e4aaced19fc81e872","unresolved":false,"context_lines":[{"line_number":135,"context_line":"        return snapshot"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    @classmethod"},{"line_number":138,"context_line":"    def create_backup(cls, volume_id, backup_client\u003dNone, **kwargs):"},{"line_number":139,"context_line":"        \"\"\"Wrapper utility that returns a test backup.\"\"\""},{"line_number":140,"context_line":"        if backup_client is None:"},{"line_number":141,"context_line":"            backup \u003d cls.backups_client.create_backup("}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_c4e6ee15","line":138,"updated":"2016-08-31 12:07:23.000000000","message":"You are duplicating too much code. Try this:\n\n\ndef create_backup(cls, volume_id, backup_client\u003dNone, **kwargs):\n    \"\"\"Wrapper utility that returns a test backup.\"\"\"\n    if backup_client is None:\n        backup_client \u003d cls.backups_client\n\n    backup \u003d backup_client.create_backup(\n        volume_id\u003dvolume_id, **kwargs)[\u0027backup\u0027]\n    backup_client.wait_for_backup_status(backup[\u0027id\u0027], \u0027available\u0027)\n    cls.backups.append(backup)\n    return backup\n\n\nBut the problem anyway is that you are defining a class method. You should use an instance method and use self.addCleanup() instead.\n\nIf you use addCleanup you don\u0027t have to introduce a new clear_backups method, less code :)","commit_id":"6dce8b265ff0bc08f9a815b48761f86ff780e268"},{"author":{"_account_id":19262,"name":"Liron Kuchlani","email":"lkuchlan@redhat.com","username":"lkuchlan"},"change_message_id":"dd77bc355e821e189eb44d8697c8cf430bfcfbb5","unresolved":false,"context_lines":[{"line_number":135,"context_line":"        return snapshot"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    @classmethod"},{"line_number":138,"context_line":"    def create_backup(cls, volume_id, backup_client\u003dNone, **kwargs):"},{"line_number":139,"context_line":"        \"\"\"Wrapper utility that returns a test backup.\"\"\""},{"line_number":140,"context_line":"        if backup_client is None:"},{"line_number":141,"context_line":"            backup \u003d cls.backups_client.create_backup("}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_ed64b5e6","line":138,"in_reply_to":"fa7ab95a_c4e6ee15","updated":"2016-08-31 12:43:24.000000000","message":"Done","commit_id":"6dce8b265ff0bc08f9a815b48761f86ff780e268"}],"tempest/api/volume/test_volumes_backup.py":[{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"90413d26623f78987b1042e75070f1880cf1a742","unresolved":false,"context_lines":[{"line_number":47,"context_line":"                               name\u003dbackup_name)[\u0027backup\u0027]"},{"line_number":48,"context_line":"        self.addCleanup(self.backups_client.delete_backup,"},{"line_number":49,"context_line":"                        backup[\u0027id\u0027])"},{"line_number":50,"context_line":"        self.assertEqual(backup_name, backup[\u0027name\u0027])"},{"line_number":51,"context_line":"        waiters.wait_for_volume_status(self.volumes_client,"},{"line_number":52,"context_line":"                                       volume[\u0027id\u0027], \u0027available\u0027)"},{"line_number":53,"context_line":"        self.backups_client.wait_for_backup_status(backup[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_b0875860","side":"PARENT","line":50,"updated":"2016-09-05 11:33:58.000000000","message":"I am not sure why you deleted that line.","commit_id":"c68b1493c2e935db917f05523b2b7e1a6574cd4e"},{"author":{"_account_id":19262,"name":"Liron Kuchlani","email":"lkuchlan@redhat.com","username":"lkuchlan"},"change_message_id":"2f289fc3989488477a70a7708990e0226cea7711","unresolved":false,"context_lines":[{"line_number":47,"context_line":"                               name\u003dbackup_name)[\u0027backup\u0027]"},{"line_number":48,"context_line":"        self.addCleanup(self.backups_client.delete_backup,"},{"line_number":49,"context_line":"                        backup[\u0027id\u0027])"},{"line_number":50,"context_line":"        self.assertEqual(backup_name, backup[\u0027name\u0027])"},{"line_number":51,"context_line":"        waiters.wait_for_volume_status(self.volumes_client,"},{"line_number":52,"context_line":"                                       volume[\u0027id\u0027], \u0027available\u0027)"},{"line_number":53,"context_line":"        self.backups_client.wait_for_backup_status(backup[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_f01480f6","side":"PARENT","line":50,"in_reply_to":"9a89bdaa_b0875860","updated":"2016-09-05 11:38:44.000000000","message":"My mistake","commit_id":"c68b1493c2e935db917f05523b2b7e1a6574cd4e"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"90413d26623f78987b1042e75070f1880cf1a742","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        self.addCleanup(self.backups_client.delete_backup,"},{"line_number":49,"context_line":"                        backup[\u0027id\u0027])"},{"line_number":50,"context_line":"        self.assertEqual(backup_name, backup[\u0027name\u0027])"},{"line_number":51,"context_line":"        waiters.wait_for_volume_status(self.volumes_client,"},{"line_number":52,"context_line":"                                       volume[\u0027id\u0027], \u0027available\u0027)"},{"line_number":53,"context_line":"        self.backups_client.wait_for_backup_status(backup[\u0027id\u0027],"},{"line_number":54,"context_line":"                                                   \u0027available\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_30ad08ea","side":"PARENT","line":51,"updated":"2016-09-05 11:33:58.000000000","message":"You shouldn\u0027t delete this line, it\u0027s important. It checks that the volume get back to the \"available\" status after being backed up.","commit_id":"c68b1493c2e935db917f05523b2b7e1a6574cd4e"},{"author":{"_account_id":19262,"name":"Liron Kuchlani","email":"lkuchlan@redhat.com","username":"lkuchlan"},"change_message_id":"2f289fc3989488477a70a7708990e0226cea7711","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        self.addCleanup(self.backups_client.delete_backup,"},{"line_number":49,"context_line":"                        backup[\u0027id\u0027])"},{"line_number":50,"context_line":"        self.assertEqual(backup_name, backup[\u0027name\u0027])"},{"line_number":51,"context_line":"        waiters.wait_for_volume_status(self.volumes_client,"},{"line_number":52,"context_line":"                                       volume[\u0027id\u0027], \u0027available\u0027)"},{"line_number":53,"context_line":"        self.backups_client.wait_for_backup_status(backup[\u0027id\u0027],"},{"line_number":54,"context_line":"                                                   \u0027available\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_50044cbf","side":"PARENT","line":51,"in_reply_to":"9a89bdaa_30ad08ea","updated":"2016-09-05 11:38:44.000000000","message":"Right","commit_id":"c68b1493c2e935db917f05523b2b7e1a6574cd4e"}]}