)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":30407,"name":"haixin","email":"haixin_haixin@qq.com","username":"haixin"},"change_message_id":"68044c8aad3d8d629d9f69cf8b0d59678ad497ae","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"1273caef_667645e8","updated":"2022-12-11 07:02:52.000000000","message":"thanks for your change.","commit_id":"83d6304422ba1d66e6788fd1d54b83c3d1e01eac"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"4085f2c064ed097029da0d1a55298c40bad09ed0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"bec0541b_d5eda392","updated":"2023-01-17 17:04:32.000000000","message":"Also, make sure to set \"2.74\" as the microversion via config.py","commit_id":"bb5aa60fb00a498a4e867dd4a9651ca5c69eab44"},{"author":{"_account_id":32919,"name":"kiran pawar","display_name":"Kiran Pawar","email":"kinpaa@gmail.com","username":"kpdev"},"change_message_id":"5e9ad6891aa67401637bad9f4fc3909539c228e5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"24a6c59b_ba672cf9","in_reply_to":"bec0541b_d5eda392","updated":"2023-01-17 17:06:50.000000000","message":"Done","commit_id":"bb5aa60fb00a498a4e867dd4a9651ca5c69eab44"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"2da79efcc1fe7eca147540e05ae632c57ec8d87e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"13a5d485_80f6a24d","updated":"2023-01-18 21:44:26.000000000","message":"Thanks for responding!","commit_id":"fea1c7ce42bbd3a95faec4a536367695aea3a771"},{"author":{"_account_id":32919,"name":"kiran pawar","display_name":"Kiran Pawar","email":"kinpaa@gmail.com","username":"kpdev"},"change_message_id":"309945ef076a98f7fd594fb860b427aff1737803","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"625cd6cd_e1b215d8","updated":"2023-01-18 15:07:18.000000000","message":"ready to merge.","commit_id":"fea1c7ce42bbd3a95faec4a536367695aea3a771"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"8557c87b408560d62e432ab1477f3246775b5c31","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"41bfceda_79ec3dff","updated":"2023-01-23 20:02:53.000000000","message":"Kiran, thanks for proposing this tests enhancement. Please take a look at the comment inline.","commit_id":"e46e2a6e415e5817ac498fead6581564d2da37b1"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"ffac4dceb1f56cf28feaca40617f63fabb7a403e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"fa1b9c98_4469b3c1","updated":"2023-01-19 19:09:02.000000000","message":"LGTM; thanks!","commit_id":"e46e2a6e415e5817ac498fead6581564d2da37b1"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"1830fa0eb8363e328f81cf120437c42800293735","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"f1a76f2f_e835cfb0","updated":"2023-01-23 21:19:09.000000000","message":"Let it go through the gate - It\u0027s been some time since this change was proposed. If we have the chance to modify it some day, we can.\nThanks Kiran","commit_id":"e46e2a6e415e5817ac498fead6581564d2da37b1"}],"manila_tempest_tests/tests/api/test_replication.py":[{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"b4671992662757acd23a43d6ced801fbb993ece4","unresolved":true,"context_lines":[{"line_number":363,"context_line":"    @decorators.idempotent_id(\u0027600a13d2-5cf0-482e-97af-9f598b55a406\u0027)"},{"line_number":364,"context_line":"    @tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND)"},{"line_number":365,"context_line":"    @utils.skip_if_microversion_not_supported(\"2.74\")"},{"line_number":366,"context_line":"    def test_add_access_rule_share_replica_error_status_new(self):"},{"line_number":367,"context_line":"        access_type, access_to \u003d self._get_access_rule_data_from_config()"},{"line_number":368,"context_line":"        # Create the replica"},{"line_number":369,"context_line":"        share_replica \u003d self._verify_create_replica()"}],"source_content_type":"text/x-python","patch_set":7,"id":"73110f0e_c00cb7dc","line":366,"range":{"start_line":366,"start_character":55,"end_line":366,"end_character":59},"updated":"2023-01-17 15:46:59.000000000","message":"\"new\" isn\u0027t very descriptive.. \n\n\ninstead of adding _new, add a doc string below calling out that this behavior debuted in version 2.74..","commit_id":"882c7bd5add80749630c985f004e06ac5d550539"},{"author":{"_account_id":32919,"name":"kiran pawar","display_name":"Kiran Pawar","email":"kinpaa@gmail.com","username":"kpdev"},"change_message_id":"309945ef076a98f7fd594fb860b427aff1737803","unresolved":false,"context_lines":[{"line_number":363,"context_line":"    @decorators.idempotent_id(\u0027600a13d2-5cf0-482e-97af-9f598b55a406\u0027)"},{"line_number":364,"context_line":"    @tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND)"},{"line_number":365,"context_line":"    @utils.skip_if_microversion_not_supported(\"2.74\")"},{"line_number":366,"context_line":"    def test_add_access_rule_share_replica_error_status_new(self):"},{"line_number":367,"context_line":"        access_type, access_to \u003d self._get_access_rule_data_from_config()"},{"line_number":368,"context_line":"        # Create the replica"},{"line_number":369,"context_line":"        share_replica \u003d self._verify_create_replica()"}],"source_content_type":"text/x-python","patch_set":7,"id":"1170545a_4335a52e","line":366,"range":{"start_line":366,"start_character":55,"end_line":366,"end_character":59},"in_reply_to":"73110f0e_c00cb7dc","updated":"2023-01-18 15:07:18.000000000","message":"Done","commit_id":"882c7bd5add80749630c985f004e06ac5d550539"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"6a7af92756f45d108c0187827c38b2d1cb00fd2c","unresolved":true,"context_lines":[{"line_number":365,"context_line":"    @utils.skip_if_microversion_not_supported(\"2.74\")"},{"line_number":366,"context_line":"    def test_add_access_rule_share_replica_error_status(self):"},{"line_number":367,"context_line":"        \u0027\u0027\u0027From 2.74, we can add rules even if replicas are in error state.\u0027\u0027\u0027"},{"line_number":368,"context_line":"        access_type, access_to \u003d self._get_access_rule_data_from_config()"},{"line_number":369,"context_line":"        # Create the replica"},{"line_number":370,"context_line":"        share_replica \u003d self._verify_create_replica()"},{"line_number":371,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ee5aa676_6e885edf","line":368,"range":{"start_line":368,"start_character":38,"end_line":368,"end_character":71},"updated":"2023-01-17 23:15:14.000000000","message":"method was recently renamed","commit_id":"6a61e7a82f3ee9f716f1f3a1fe7d445cfdc48d97"},{"author":{"_account_id":32919,"name":"kiran pawar","display_name":"Kiran Pawar","email":"kinpaa@gmail.com","username":"kpdev"},"change_message_id":"309945ef076a98f7fd594fb860b427aff1737803","unresolved":false,"context_lines":[{"line_number":365,"context_line":"    @utils.skip_if_microversion_not_supported(\"2.74\")"},{"line_number":366,"context_line":"    def test_add_access_rule_share_replica_error_status(self):"},{"line_number":367,"context_line":"        \u0027\u0027\u0027From 2.74, we can add rules even if replicas are in error state.\u0027\u0027\u0027"},{"line_number":368,"context_line":"        access_type, access_to \u003d self._get_access_rule_data_from_config()"},{"line_number":369,"context_line":"        # Create the replica"},{"line_number":370,"context_line":"        share_replica \u003d self._verify_create_replica()"},{"line_number":371,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"5860de35_58bfd204","line":368,"range":{"start_line":368,"start_character":38,"end_line":368,"end_character":71},"in_reply_to":"ee5aa676_6e885edf","updated":"2023-01-18 15:07:18.000000000","message":"Done","commit_id":"6a61e7a82f3ee9f716f1f3a1fe7d445cfdc48d97"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"63fbfc36b97c81f06830a0a9dfc54effe736333c","unresolved":true,"context_lines":[{"line_number":375,"context_line":"            share_replica[\u0027id\u0027], constants.STATUS_ERROR)"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"        # Verify access rule will be added in error state"},{"line_number":378,"context_line":"        self.admin_client.create_access_rule("},{"line_number":379,"context_line":"            self.shares[0][\"id\"], access_type\u003daccess_type, access_to\u003daccess_to,"},{"line_number":380,"context_line":"            access_level\u003d\u0027ro\u0027)"},{"line_number":381,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"d7e563c6_23448090","line":378,"range":{"start_line":378,"start_character":13,"end_line":378,"end_character":25},"updated":"2023-01-18 20:51:00.000000000","message":"by using the admin client, it feels like you\u0027re missig testing for a scenario - \n\nthis should be shares_v2_client - i.e., a user with a non-privleged role (by default any user with a non-reader role, belonging to the project owning the share)  must be able to create an access rule on a share when its replica is in \"error\" state. if such a user can do it, then users with an admin role surely can, and we don\u0027t need to test that - it\u0027s just role hierarchy.","commit_id":"fea1c7ce42bbd3a95faec4a536367695aea3a771"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"63fbfc36b97c81f06830a0a9dfc54effe736333c","unresolved":true,"context_lines":[{"line_number":381,"context_line":""},{"line_number":382,"context_line":"        # Verify access_rules_status transitions to \u0027active\u0027 state."},{"line_number":383,"context_line":"        waiters.wait_for_resource_status("},{"line_number":384,"context_line":"            self.admin_client, self.shares[0][\"id\"],"},{"line_number":385,"context_line":"            constants.RULE_STATE_ACTIVE, status_attr\u003d\u0027access_rules_status\u0027)"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    @decorators.idempotent_id(\u0027a542c179-ea41-4bc0-bd80-e06eaddf5253\u0027)"}],"source_content_type":"text/x-python","patch_set":12,"id":"d23ec311_41ac1cf6","line":384,"range":{"start_line":384,"start_character":17,"end_line":384,"end_character":29},"updated":"2023-01-18 20:51:00.000000000","message":"again, why \"admin\" role - in general avoid testing with admin what you really want to test with the default member\u0027s role..","commit_id":"fea1c7ce42bbd3a95faec4a536367695aea3a771"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"2da79efcc1fe7eca147540e05ae632c57ec8d87e","unresolved":true,"context_lines":[{"line_number":381,"context_line":""},{"line_number":382,"context_line":"        # Verify access_rules_status transitions to \u0027active\u0027 state."},{"line_number":383,"context_line":"        waiters.wait_for_resource_status("},{"line_number":384,"context_line":"            self.admin_client, self.shares[0][\"id\"],"},{"line_number":385,"context_line":"            constants.RULE_STATE_ACTIVE, status_attr\u003d\u0027access_rules_status\u0027)"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    @decorators.idempotent_id(\u0027a542c179-ea41-4bc0-bd80-e06eaddf5253\u0027)"}],"source_content_type":"text/x-python","patch_set":12,"id":"530447f6_ce788df8","line":384,"range":{"start_line":384,"start_character":17,"end_line":384,"end_character":29},"in_reply_to":"9839147c_d6c7a23b","updated":"2023-01-18 21:44:26.000000000","message":"The failure occurs at line 374 -- an ordinary user cannot reset the replica\u0027s status by the virtue of default RBAC. So, it makes sense for that call to be made with the admin_client. \n\nI agree the current code could have used the regular user client (self.shares_v2_client) to make the \"create_access_rule\" call - feel free to fix it in this patch... \n\nDoes my explanation above (https://review.opendev.org/c/openstack/manila-tempest-plugin/+/865603/comments/d7e563c6_23448090) make sense wrt the testing philosophy here?","commit_id":"fea1c7ce42bbd3a95faec4a536367695aea3a771"},{"author":{"_account_id":32919,"name":"kiran pawar","display_name":"Kiran Pawar","email":"kinpaa@gmail.com","username":"kpdev"},"change_message_id":"84506464108e4cf4b4348bc89e72b7012d0aa888","unresolved":true,"context_lines":[{"line_number":381,"context_line":""},{"line_number":382,"context_line":"        # Verify access_rules_status transitions to \u0027active\u0027 state."},{"line_number":383,"context_line":"        waiters.wait_for_resource_status("},{"line_number":384,"context_line":"            self.admin_client, self.shares[0][\"id\"],"},{"line_number":385,"context_line":"            constants.RULE_STATE_ACTIVE, status_attr\u003d\u0027access_rules_status\u0027)"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    @decorators.idempotent_id(\u0027a542c179-ea41-4bc0-bd80-e06eaddf5253\u0027)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9839147c_d6c7a23b","line":384,"range":{"start_line":384,"start_character":17,"end_line":384,"end_character":29},"in_reply_to":"d23ec311_41ac1cf6","updated":"2023-01-18 21:09:59.000000000","message":"I see earlier test failed for the same reason - https://zuul.opendev.org/t/openstack/build/98b1a1539efa46e3a6b87e389213cbb3\n\nexisting code is using admin only, https://github.com/openstack/manila-tempest-plugin/blob/master/manila_tempest_tests/tests/api/test_replication_negative.py#L204","commit_id":"fea1c7ce42bbd3a95faec4a536367695aea3a771"}],"manila_tempest_tests/tests/api/test_replication_negative.py":[{"author":{"_account_id":30407,"name":"haixin","email":"haixin_haixin@qq.com","username":"haixin"},"change_message_id":"68044c8aad3d8d629d9f69cf8b0d59678ad497ae","unresolved":true,"context_lines":[{"line_number":190,"context_line":"    @decorators.idempotent_id(\u0027600a13d2-5cf0-482e-97af-9f598b55a407\u0027)"},{"line_number":191,"context_line":"    @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)"},{"line_number":192,"context_line":"    @utils.skip_if_microversion_not_supported(\"2.74\")"},{"line_number":193,"context_line":"    def test_add_access_rule_share_replica_error_status(self):"},{"line_number":194,"context_line":"        access_type, access_to \u003d self._get_access_rule_data_from_config()"},{"line_number":195,"context_line":"        # Create the replica"},{"line_number":196,"context_line":"        share_replica \u003d self.create_share_replica(self.share1[\"id\"],"}],"source_content_type":"text/x-python","patch_set":5,"id":"acefb349_343c03dd","line":193,"range":{"start_line":193,"start_character":8,"end_line":193,"end_character":55},"updated":"2022-12-11 07:02:52.000000000","message":"if we support add access rule after 2.74, so test will not be negative test, we\u0027d better move this test to test/api/test_replication.py.","commit_id":"83d6304422ba1d66e6788fd1d54b83c3d1e01eac"},{"author":{"_account_id":32919,"name":"kiran pawar","display_name":"Kiran Pawar","email":"kinpaa@gmail.com","username":"kpdev"},"change_message_id":"eb9af66f2a617749d5a486ccc1f30ed969eac51d","unresolved":false,"context_lines":[{"line_number":190,"context_line":"    @decorators.idempotent_id(\u0027600a13d2-5cf0-482e-97af-9f598b55a407\u0027)"},{"line_number":191,"context_line":"    @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)"},{"line_number":192,"context_line":"    @utils.skip_if_microversion_not_supported(\"2.74\")"},{"line_number":193,"context_line":"    def test_add_access_rule_share_replica_error_status(self):"},{"line_number":194,"context_line":"        access_type, access_to \u003d self._get_access_rule_data_from_config()"},{"line_number":195,"context_line":"        # Create the replica"},{"line_number":196,"context_line":"        share_replica \u003d self.create_share_replica(self.share1[\"id\"],"}],"source_content_type":"text/x-python","patch_set":5,"id":"105c50e9_3f70926c","line":193,"range":{"start_line":193,"start_character":8,"end_line":193,"end_character":55},"in_reply_to":"acefb349_343c03dd","updated":"2022-12-11 08:32:48.000000000","message":"Done","commit_id":"83d6304422ba1d66e6788fd1d54b83c3d1e01eac"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"c250d94ceda67c7d21b91ddeb80d64f9f4918f87","unresolved":true,"context_lines":[{"line_number":187,"context_line":"        # Try promoting the replica"},{"line_number":188,"context_line":"        self.shares_v2_client.promote_share_replica(replica[\u0027id\u0027])"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    @decorators.idempotent_id(\u0027600a13d2-5cf0-482e-97af-9f598b55a407\u0027)"},{"line_number":191,"context_line":"    @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)"},{"line_number":192,"context_line":"    def test_add_access_rule_share_replica_error_status(self):"},{"line_number":193,"context_line":"        access_type, access_to \u003d self._get_access_rule_data_from_config()"},{"line_number":194,"context_line":"        # Create the replica"},{"line_number":195,"context_line":"        share_replica \u003d self.create_share_replica(self.share1[\"id\"],"},{"line_number":196,"context_line":"                                                  self.replica_zone,"},{"line_number":197,"context_line":"                                                  cleanup_in_class\u003dFalse)"},{"line_number":198,"context_line":"        # Reset the replica status to error"},{"line_number":199,"context_line":"        self.admin_client.reset_share_replica_status("},{"line_number":200,"context_line":"            share_replica[\u0027id\u0027], constants.STATUS_ERROR)"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"        # Verify access rule cannot be added"},{"line_number":203,"context_line":"        self.assertRaises(lib_exc.BadRequest,"},{"line_number":204,"context_line":"                          self.admin_client.create_access_rule,"},{"line_number":205,"context_line":"                          self.share1[\"id\"], access_type, access_to, \u0027ro\u0027)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    @decorators.idempotent_id(\u002791b93b71-4048-412b-bb42-0fe88edfb987\u0027)"},{"line_number":208,"context_line":"    @testtools.skipUnless(CONF.share.run_host_assisted_migration_tests or"}],"source_content_type":"text/x-python","patch_set":6,"id":"a9d4e88e_57f7a711","side":"PARENT","line":205,"range":{"start_line":190,"start_character":0,"end_line":205,"end_character":74},"updated":"2023-01-03 18:19:47.000000000","message":"We seldom delete tests; the changing API behavior is being microversioned. Perhaps use version \"2.73\" in the API request for 203-205 instead of deleting this test case. \n\nIf \"2.73\" isn\u0027t supported by tempest configuration (or manila\u0027s API in the cloud being tested), you can default to \"2.11\" (_MIN_SUPPORTED_MICROVERSION, line 30)","commit_id":"8dc165888132bbf43c63f2d501717e5308f54956"},{"author":{"_account_id":32919,"name":"kiran pawar","display_name":"Kiran Pawar","email":"kinpaa@gmail.com","username":"kpdev"},"change_message_id":"b667397756be76ccac3a5ebee8697c4006ba8cca","unresolved":false,"context_lines":[{"line_number":187,"context_line":"        # Try promoting the replica"},{"line_number":188,"context_line":"        self.shares_v2_client.promote_share_replica(replica[\u0027id\u0027])"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    @decorators.idempotent_id(\u0027600a13d2-5cf0-482e-97af-9f598b55a407\u0027)"},{"line_number":191,"context_line":"    @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)"},{"line_number":192,"context_line":"    def test_add_access_rule_share_replica_error_status(self):"},{"line_number":193,"context_line":"        access_type, access_to \u003d self._get_access_rule_data_from_config()"},{"line_number":194,"context_line":"        # Create the replica"},{"line_number":195,"context_line":"        share_replica \u003d self.create_share_replica(self.share1[\"id\"],"},{"line_number":196,"context_line":"                                                  self.replica_zone,"},{"line_number":197,"context_line":"                                                  cleanup_in_class\u003dFalse)"},{"line_number":198,"context_line":"        # Reset the replica status to error"},{"line_number":199,"context_line":"        self.admin_client.reset_share_replica_status("},{"line_number":200,"context_line":"            share_replica[\u0027id\u0027], constants.STATUS_ERROR)"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"        # Verify access rule cannot be added"},{"line_number":203,"context_line":"        self.assertRaises(lib_exc.BadRequest,"},{"line_number":204,"context_line":"                          self.admin_client.create_access_rule,"},{"line_number":205,"context_line":"                          self.share1[\"id\"], access_type, access_to, \u0027ro\u0027)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    @decorators.idempotent_id(\u002791b93b71-4048-412b-bb42-0fe88edfb987\u0027)"},{"line_number":208,"context_line":"    @testtools.skipUnless(CONF.share.run_host_assisted_migration_tests or"}],"source_content_type":"text/x-python","patch_set":6,"id":"ec8ba7b8_e9d546ef","side":"PARENT","line":205,"range":{"start_line":190,"start_character":0,"end_line":205,"end_character":74},"in_reply_to":"a9d4e88e_57f7a711","updated":"2023-01-10 18:05:24.000000000","message":"Done","commit_id":"8dc165888132bbf43c63f2d501717e5308f54956"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"b4671992662757acd23a43d6ced801fbb993ece4","unresolved":true,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":"    @decorators.idempotent_id(\u0027600a13d2-5cf0-482e-97af-9f598b55a407\u0027)"},{"line_number":192,"context_line":"    @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)"},{"line_number":193,"context_line":"    @ddt.data(\u00272.11\u0027, \u00272.74\u0027)"},{"line_number":194,"context_line":"    def test_add_access_rule_share_replica_error_status(self, version):"},{"line_number":195,"context_line":"        access_type, access_to \u003d utils.get_access_rule_data_from_config("},{"line_number":196,"context_line":"            self.shares_v2_client.share_protocol)"}],"source_content_type":"text/x-python","patch_set":7,"id":"62e95086_3746bb4d","line":193,"range":{"start_line":193,"start_character":21,"end_line":193,"end_character":29},"updated":"2023-01-17 15:46:59.000000000","message":"this test is no longer a NEGATIVE test (test tag on the line above) with version 2.74... Did you mean \"2.73\"? If not, with \"2.74\" it is a duplicate of the \"test_add_access_rule_share_replica_error_status_new\" test you\u0027r introducing. \n\n\nYou can fix this by replacing \"2.74\" with \"2.73\" in the DDT input --- it is always good practice to test micro-versioned API changes. The idea is testing whether the behavior that\u0027s changing in the newer version stays unchanged in the older version.\n\nI suggest also adding a docstring below calling out that with/after version 2.74, you can now add rules even when replicas are in \"error\" state.","commit_id":"882c7bd5add80749630c985f004e06ac5d550539"},{"author":{"_account_id":32919,"name":"kiran pawar","display_name":"Kiran Pawar","email":"kinpaa@gmail.com","username":"kpdev"},"change_message_id":"177d305be9391b2c15d2e33d8f81d0a8812578d1","unresolved":false,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":"    @decorators.idempotent_id(\u0027600a13d2-5cf0-482e-97af-9f598b55a407\u0027)"},{"line_number":192,"context_line":"    @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)"},{"line_number":193,"context_line":"    @ddt.data(\u00272.11\u0027, \u00272.74\u0027)"},{"line_number":194,"context_line":"    def test_add_access_rule_share_replica_error_status(self, version):"},{"line_number":195,"context_line":"        access_type, access_to \u003d utils.get_access_rule_data_from_config("},{"line_number":196,"context_line":"            self.shares_v2_client.share_protocol)"}],"source_content_type":"text/x-python","patch_set":7,"id":"f8b4153a_6dec426f","line":193,"range":{"start_line":193,"start_character":21,"end_line":193,"end_character":29},"in_reply_to":"62e95086_3746bb4d","updated":"2023-01-17 16:48:59.000000000","message":"Done","commit_id":"882c7bd5add80749630c985f004e06ac5d550539"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"b4671992662757acd23a43d6ced801fbb993ece4","unresolved":true,"context_lines":[{"line_number":202,"context_line":"        self.admin_client.reset_share_replica_status("},{"line_number":203,"context_line":"            share_replica[\u0027id\u0027], constants.STATUS_ERROR)"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"        if utils.is_microversion_lt(version, \u00272.74\u0027):"},{"line_number":206,"context_line":"            # Verify access rule cannot be added"},{"line_number":207,"context_line":"            self.assertRaises(lib_exc.BadRequest,"},{"line_number":208,"context_line":"                              self.admin_client.create_access_rule,"},{"line_number":209,"context_line":"                              self.share1[\"id\"], access_type, access_to, \u0027ro\u0027)"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    @decorators.idempotent_id(\u002791b93b71-4048-412b-bb42-0fe88edfb987\u0027)"},{"line_number":212,"context_line":"    @testtools.skipUnless(CONF.share.run_host_assisted_migration_tests or"}],"source_content_type":"text/x-python","patch_set":7,"id":"8facc653_e80fe6e3","line":209,"range":{"start_line":205,"start_character":0,"end_line":209,"end_character":78},"updated":"2023-01-17 15:46:59.000000000","message":"else?\n\nAs i suggest above, removing 2.74 from this test makes sense. You should make the \"create_access_rule\" request with a specific microversion that\u0027s the input to this test..","commit_id":"882c7bd5add80749630c985f004e06ac5d550539"},{"author":{"_account_id":32919,"name":"kiran pawar","display_name":"Kiran Pawar","email":"kinpaa@gmail.com","username":"kpdev"},"change_message_id":"177d305be9391b2c15d2e33d8f81d0a8812578d1","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        self.admin_client.reset_share_replica_status("},{"line_number":203,"context_line":"            share_replica[\u0027id\u0027], constants.STATUS_ERROR)"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"        if utils.is_microversion_lt(version, \u00272.74\u0027):"},{"line_number":206,"context_line":"            # Verify access rule cannot be added"},{"line_number":207,"context_line":"            self.assertRaises(lib_exc.BadRequest,"},{"line_number":208,"context_line":"                              self.admin_client.create_access_rule,"},{"line_number":209,"context_line":"                              self.share1[\"id\"], access_type, access_to, \u0027ro\u0027)"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    @decorators.idempotent_id(\u002791b93b71-4048-412b-bb42-0fe88edfb987\u0027)"},{"line_number":212,"context_line":"    @testtools.skipUnless(CONF.share.run_host_assisted_migration_tests or"}],"source_content_type":"text/x-python","patch_set":7,"id":"a86ad5af_f653281e","line":209,"range":{"start_line":205,"start_character":0,"end_line":209,"end_character":78},"in_reply_to":"8facc653_e80fe6e3","updated":"2023-01-17 16:48:59.000000000","message":"Done","commit_id":"882c7bd5add80749630c985f004e06ac5d550539"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"8557c87b408560d62e432ab1477f3246775b5c31","unresolved":true,"context_lines":[{"line_number":204,"context_line":"        self.admin_client.reset_share_replica_status("},{"line_number":205,"context_line":"            share_replica[\u0027id\u0027], constants.STATUS_ERROR)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"        if utils.is_microversion_lt(version, \u00272.74\u0027):"},{"line_number":208,"context_line":"            # Verify access rule cannot be added"},{"line_number":209,"context_line":"            self.assertRaises(lib_exc.BadRequest,"},{"line_number":210,"context_line":"                              self.shares_v2_client.create_access_rule,"}],"source_content_type":"text/x-python","patch_set":13,"id":"3dd3d541_663a3f7f","line":207,"range":{"start_line":207,"start_character":8,"end_line":207,"end_character":53},"updated":"2023-01-23 20:02:53.000000000","message":"As there is no else condition, I believe there is no need to run this test if microversion is 2.74 or higher, otherwise at the end it will be a test that will create a share replica and reset the status of the share replica, and the cleanup will occur right after. So my suggestion is: at the beginning of the test, skip it if the microversion is \u003e\u003d 2.74. I think there\u0027s a decorator for that too :)","commit_id":"e46e2a6e415e5817ac498fead6581564d2da37b1"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"4b6b4fbfead87ab7eb4064be6283f59b88acfe4f","unresolved":true,"context_lines":[{"line_number":204,"context_line":"        self.admin_client.reset_share_replica_status("},{"line_number":205,"context_line":"            share_replica[\u0027id\u0027], constants.STATUS_ERROR)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"        if utils.is_microversion_lt(version, \u00272.74\u0027):"},{"line_number":208,"context_line":"            # Verify access rule cannot be added"},{"line_number":209,"context_line":"            self.assertRaises(lib_exc.BadRequest,"},{"line_number":210,"context_line":"                              self.shares_v2_client.create_access_rule,"}],"source_content_type":"text/x-python","patch_set":13,"id":"ddc5e7fc_e5332a7a","line":207,"range":{"start_line":207,"start_character":8,"end_line":207,"end_character":53},"in_reply_to":"3dd3d541_663a3f7f","updated":"2023-01-23 20:08:40.000000000","message":"Missed the DDT of the test, but I think my comment could still be valid. This if statement might be invalid, considering that you are explicitly tagging the version on the ddt and sending it in the request - As 2.74 would never be the case, you might only drop this if statement?","commit_id":"e46e2a6e415e5817ac498fead6581564d2da37b1"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"5f2e9e103f778a7306a2c961d92fdaf3694f6853","unresolved":true,"context_lines":[{"line_number":204,"context_line":"        self.admin_client.reset_share_replica_status("},{"line_number":205,"context_line":"            share_replica[\u0027id\u0027], constants.STATUS_ERROR)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"        if utils.is_microversion_lt(version, \u00272.74\u0027):"},{"line_number":208,"context_line":"            # Verify access rule cannot be added"},{"line_number":209,"context_line":"            self.assertRaises(lib_exc.BadRequest,"},{"line_number":210,"context_line":"                              self.shares_v2_client.create_access_rule,"}],"source_content_type":"text/x-python","patch_set":13,"id":"0b48cc8d_4a4616c4","line":207,"range":{"start_line":207,"start_character":8,"end_line":207,"end_character":53},"in_reply_to":"4755c5c8_9efca874","updated":"2023-01-23 21:17:00.000000000","message":"I understand your point and I\u0027m okay if we want to leave it as is, unless someone else has the same concern or something else requires a new PS to be uploaded.","commit_id":"e46e2a6e415e5817ac498fead6581564d2da37b1"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"69a5b997c2aad3a2b3b8fd8ad1c5a75542370a19","unresolved":true,"context_lines":[{"line_number":204,"context_line":"        self.admin_client.reset_share_replica_status("},{"line_number":205,"context_line":"            share_replica[\u0027id\u0027], constants.STATUS_ERROR)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"        if utils.is_microversion_lt(version, \u00272.74\u0027):"},{"line_number":208,"context_line":"            # Verify access rule cannot be added"},{"line_number":209,"context_line":"            self.assertRaises(lib_exc.BadRequest,"},{"line_number":210,"context_line":"                              self.shares_v2_client.create_access_rule,"}],"source_content_type":"text/x-python","patch_set":13,"id":"4755c5c8_9efca874","line":207,"range":{"start_line":207,"start_character":8,"end_line":207,"end_character":53},"in_reply_to":"cdaea4e1_54bcb666","updated":"2023-01-23 21:15:16.000000000","message":"You have two scenarios, 2.11 and 2.73. As it is `utils.is_microversion_lt(version, \u00272.74\u0027):` will *never* be false. The if is there only for adding one more step for interpretation, nothing else. If the intention is showing the person reading the code that it should not be that behavior for 2.74, IMHO the best way to signalize would be through a code comment, not an extra if statement. We have several tests like this in the code base, i.e. [1].\n\n\n[1] https://github.com/openstack/manila-tempest-plugin/blob/master/manila_tempest_tests/tests/api/admin/test_share_types_extra_specs_negative.py#L332-L335","commit_id":"e46e2a6e415e5817ac498fead6581564d2da37b1"},{"author":{"_account_id":32919,"name":"kiran pawar","display_name":"Kiran Pawar","email":"kinpaa@gmail.com","username":"kpdev"},"change_message_id":"0b6e844372705e55de6eb2aea305e948c9462acd","unresolved":true,"context_lines":[{"line_number":204,"context_line":"        self.admin_client.reset_share_replica_status("},{"line_number":205,"context_line":"            share_replica[\u0027id\u0027], constants.STATUS_ERROR)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"        if utils.is_microversion_lt(version, \u00272.74\u0027):"},{"line_number":208,"context_line":"            # Verify access rule cannot be added"},{"line_number":209,"context_line":"            self.assertRaises(lib_exc.BadRequest,"},{"line_number":210,"context_line":"                              self.shares_v2_client.create_access_rule,"}],"source_content_type":"text/x-python","patch_set":13,"id":"cdaea4e1_54bcb666","line":207,"range":{"start_line":207,"start_character":8,"end_line":207,"end_character":53},"in_reply_to":"ddc5e7fc_e5332a7a","updated":"2023-01-23 20:33:05.000000000","message":"Even if ddt data is passing 2.73, the version parameter makes sense only if its validated/checked inside function definition. so if check should be there.","commit_id":"e46e2a6e415e5817ac498fead6581564d2da37b1"}],"tags":[{"author":{"_account_id":30407,"name":"haixin","email":"haixin_haixin@qq.com","username":"haixin"},"change_message_id":"1bc5379167176ff991c386f5b0e5dc2e23db8edd","unresolved":true,"context_lines":[{"line_number":1,"context_line":"!_TAG_FILE_FORMAT\t2\t/extended format; --format\u003d1 will not append ;\" to lines/"},{"line_number":2,"context_line":"!_TAG_FILE_SORTED\t1\t/0\u003dunsorted, 1\u003dsorted, 2\u003dfoldcase/"},{"line_number":3,"context_line":"!_TAG_PROGRAM_AUTHOR\tDarren Hiebert\t/dhiebert@users.sourceforge.net/"},{"line_number":4,"context_line":"!_TAG_PROGRAM_NAME\tExuberant Ctags\t//"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"7cb2defc_c65a9c91","line":1,"updated":"2022-12-05 10:31:53.000000000","message":"what is this tags file?","commit_id":"b58f798e8ecba988e289744e3014a3cd33f00a24"},{"author":{"_account_id":32919,"name":"kiran pawar","display_name":"Kiran Pawar","email":"kinpaa@gmail.com","username":"kpdev"},"change_message_id":"a6d796728cb27661e9ce608809023a5ce3b69943","unresolved":false,"context_lines":[{"line_number":1,"context_line":"!_TAG_FILE_FORMAT\t2\t/extended format; --format\u003d1 will not append ;\" to lines/"},{"line_number":2,"context_line":"!_TAG_FILE_SORTED\t1\t/0\u003dunsorted, 1\u003dsorted, 2\u003dfoldcase/"},{"line_number":3,"context_line":"!_TAG_PROGRAM_AUTHOR\tDarren Hiebert\t/dhiebert@users.sourceforge.net/"},{"line_number":4,"context_line":"!_TAG_PROGRAM_NAME\tExuberant Ctags\t//"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"c16a8058_d5bb167d","line":1,"in_reply_to":"7cb2defc_c65a9c91","updated":"2022-12-05 10:39:59.000000000","message":"it was added by mistake, removed","commit_id":"b58f798e8ecba988e289744e3014a3cd33f00a24"}]}