)]}'
{"manila_tempest_tests/tests/api/admin/test_share_manage.py":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"910e67e72ce4912e014a7344af64c53f6a315ef1","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        utils.skip_if_manage_not_supported_for_version(version)"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        share_type \u003d self._create_share_type(cleanup\u003dFalse)"},{"line_number":55,"context_line":"        self.addCleanup(self.admin_shares_v2_client.delete_share_type,"},{"line_number":56,"context_line":"                        share_type[\u0027id\u0027])"},{"line_number":57,"context_line":"        share \u003d self._create_share_for_manage(share_type\u003dshare_type)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        name \u003d \"Name for \u0027managed\u0027 share that had ID %s\" % share[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":5,"id":"1f621f24_0ba2ccae","line":56,"range":{"start_line":54,"start_character":0,"end_line":56,"end_character":41},"updated":"2020-11-04 20:08:51.000000000","message":"Hi Liron, I still can\u0027t see why you can\u0027t use \u0027cleanup_in_class\u003dFalse\u0027 here since this is the first resource created within all tests that call \u0027_test_manage\u0027. If this is the first resource created, means that will be the last one to be deleted in the end of the test.","commit_id":"38d24f43a3238add02015cae3d8a12831b240f67"},{"author":{"_account_id":19262,"name":"Liron Kuchlani","email":"lkuchlan@redhat.com","username":"lkuchlan"},"change_message_id":"14c9b770e928092b7b55d805262cd9d453e5f5d7","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        utils.skip_if_manage_not_supported_for_version(version)"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        share_type \u003d self._create_share_type(cleanup\u003dFalse)"},{"line_number":55,"context_line":"        self.addCleanup(self.admin_shares_v2_client.delete_share_type,"},{"line_number":56,"context_line":"                        share_type[\u0027id\u0027])"},{"line_number":57,"context_line":"        share \u003d self._create_share_for_manage(share_type\u003dshare_type)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        name \u003d \"Name for \u0027managed\u0027 share that had ID %s\" % share[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":5,"id":"1f621f24_1264b921","line":56,"range":{"start_line":54,"start_character":0,"end_line":56,"end_character":41},"in_reply_to":"1f621f24_0ba2ccae","updated":"2020-11-05 08:26:04.000000000","message":"Hi Douglas, at first before uploading this patch I used \u0027cleanup_in_class\u003dFalse\u0027, but still the share type was left behind\nand caused other tests to fail.\nI suspect the reason it that we are holding a list for the resources\n(method_resources) and this list contains resources from other tests and according to this list we are deleting the resources in loop.","commit_id":"38d24f43a3238add02015cae3d8a12831b240f67"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"ff119292fc6cee11f357aa616c9fff8f8dd258ba","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        utils.skip_if_manage_not_supported_for_version(version)"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        share_type \u003d self._create_share_type(cleanup\u003dFalse)"},{"line_number":55,"context_line":"        self.addCleanup(self.admin_shares_v2_client.delete_share_type,"},{"line_number":56,"context_line":"                        share_type[\u0027id\u0027])"},{"line_number":57,"context_line":"        share \u003d self._create_share_for_manage(share_type\u003dshare_type)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        name \u003d \"Name for \u0027managed\u0027 share that had ID %s\" % share[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":5,"id":"d32bd46b_144a52e2","line":56,"range":{"start_line":54,"start_character":0,"end_line":56,"end_character":41},"in_reply_to":"1f621f24_1264b921","updated":"2020-12-03 00:14:33.000000000","message":"Liron, what you\u0027re doing here isn\u0027t incorrect, feels redundant -\nI completely agree with creating a share type just in time and cleaning it up after the test method is done. but, if there\u0027s a problem with this method based cleanup mechanism (\"cleanup_in_class\u003dFalse\"), we should address that issue perhaps?\n\nPerhaps we can step through the code and dump the method resources to understand why they\u0027re not getting cleared?","commit_id":"38d24f43a3238add02015cae3d8a12831b240f67"}],"manila_tempest_tests/tests/api/admin/test_share_manage_negative.py":[{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"01c2030d249c104d72c6368199b609a98799e58c","unresolved":false,"context_lines":[{"line_number":80,"context_line":"        # second part of this test also tests that manage operation with a"},{"line_number":81,"context_line":"        # proper share type that works."},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        share_type \u003d self._create_share_type(cleanup\u003dFalse)"},{"line_number":84,"context_line":"        self.addCleanup(self.admin_shares_v2_client.delete_share_type,"},{"line_number":85,"context_line":"                        share_type[\u0027id\u0027])"},{"line_number":86,"context_line":"        share \u003d self._create_share_for_manage(share_type\u003dshare_type)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_ceed5126","line":83,"updated":"2020-09-08 04:06:20.000000000","message":"Seems we use this same code for several tests on this class, maybe we can refactor this?","commit_id":"12ca282f1f52d0f4c62c73f383d86261d7c27802"},{"author":{"_account_id":19262,"name":"Liron Kuchlani","email":"lkuchlan@redhat.com","username":"lkuchlan"},"change_message_id":"b9cdad4c14cb2b45caf866cbd3237ff2895102de","unresolved":false,"context_lines":[{"line_number":80,"context_line":"        # second part of this test also tests that manage operation with a"},{"line_number":81,"context_line":"        # proper share type that works."},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        share_type \u003d self._create_share_type(cleanup\u003dFalse)"},{"line_number":84,"context_line":"        self.addCleanup(self.admin_shares_v2_client.delete_share_type,"},{"line_number":85,"context_line":"                        share_type[\u0027id\u0027])"},{"line_number":86,"context_line":"        share \u003d self._create_share_for_manage(share_type\u003dshare_type)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_820a0d23","line":83,"in_reply_to":"9f560f44_ceed5126","updated":"2020-09-08 07:52:54.000000000","message":"Yes, I agree.\nIf we decide to create a method for creating a share type\nthat uses addCleanup it code will be more clean.","commit_id":"12ca282f1f52d0f4c62c73f383d86261d7c27802"}],"manila_tempest_tests/tests/api/base.py":[{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"01c2030d249c104d72c6368199b609a98799e58c","unresolved":false,"context_lines":[{"line_number":821,"context_line":"            \"id\": share_type[\"share_type\"][\"id\"],"},{"line_number":822,"context_line":"            \"client\": client,"},{"line_number":823,"context_line":"        }"},{"line_number":824,"context_line":"        if cleanup:"},{"line_number":825,"context_line":"            if cleanup_in_class:"},{"line_number":826,"context_line":"                cls.class_resources.insert(0, resource)"},{"line_number":827,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_eee81517","line":824,"updated":"2020-09-08 04:06:20.000000000","message":"Why this extra check? I\u0027m afraid this adds some inconsistency on how we handle cleanups for resources in general and might generate issues in the future","commit_id":"12ca282f1f52d0f4c62c73f383d86261d7c27802"},{"author":{"_account_id":19262,"name":"Liron Kuchlani","email":"lkuchlan@redhat.com","username":"lkuchlan"},"change_message_id":"b9cdad4c14cb2b45caf866cbd3237ff2895102de","unresolved":false,"context_lines":[{"line_number":821,"context_line":"            \"id\": share_type[\"share_type\"][\"id\"],"},{"line_number":822,"context_line":"            \"client\": client,"},{"line_number":823,"context_line":"        }"},{"line_number":824,"context_line":"        if cleanup:"},{"line_number":825,"context_line":"            if cleanup_in_class:"},{"line_number":826,"context_line":"                cls.class_resources.insert(0, resource)"},{"line_number":827,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f560f44_6237f9ff","line":824,"in_reply_to":"9f560f44_eee81517","updated":"2020-09-08 07:52:54.000000000","message":"This resource should not be included in the list of class_resources and\nmethod_resources, otherwise, we are left with the same problem.\nSo if \"test_manage_share_duplicate\" fails other tests will fail\nbecause of the deletion operation order.\nWe have to delete the share type at the end of the test by using addCleanup.\nAt the first I thought to create another method at the base class for creating\na share type that uses addCleanup but I don\u0027t sure about it because will\nhave two methods for creating a share type.\nMaybe we can create a wrapper method for choosing which method to use.\nWhat do you think?","commit_id":"12ca282f1f52d0f4c62c73f383d86261d7c27802"}]}