)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b6a6a5466d838420f2ec5bbbc5ea728b3b6c29a7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"ca914670_c0b867cc","updated":"2024-09-17 10:29:34.000000000","message":"Obviously you still need to update/expand tests and add a release note but I think this is a good start. Two comments inline. -1 to catch your eye.","commit_id":"534c8278574d4621ae7421f4edb9071f5d836846"},{"author":{"_account_id":36482,"name":"Oria Weng","display_name":"0weng","email":"oweng@osuosl.org","username":"0weng"},"change_message_id":"89317f8cea0d5be7949eeb5878cff1e453cb9820","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"fc433556_bd752f5d","updated":"2024-09-17 22:10:47.000000000","message":"According to the [API docs](https://docs.openstack.org/api-ref/identity/v3/index.html#os-inherit), OS-INHERIT doesn\u0027t seem to be needed for inheriting from the system, so I\u0027ve removed `inherited` for assigning system roles.","commit_id":"ea956a3f86b0d7e3837091ee9c873e927414ac45"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c0324263b4e0d146e333252d11a19bbbd4656805","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"e4fac1f4_0cb2d77a","updated":"2024-09-20 12:42:52.000000000","message":"I think you just need a release note here now and then you can remove the `Work in Progress` marker.","commit_id":"55effaa4eafecfd6aa2019d058a6a57f2cf90b25"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9e9bccdf392632f3c10b82dd555dc96c42afbeba","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"d9ec496d_4260f046","updated":"2024-09-24 14:48:32.000000000","message":"One outstanding nit inline.","commit_id":"10768d3d9ad26646a323abedc166c0135fa1eaad"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"66909c3f0a37e08de831b914320e40e5066b4178","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":22,"id":"a33dd953_063474e6","updated":"2024-10-21 13:14:49.000000000","message":"Comment from @dtantsur@protonmail.com needs addressing","commit_id":"aab2958a4f0af09b36b56123a96f0957dda15798"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"3d01803fee64b9a1727bb13aedde41e6e3e3b742","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":22,"id":"9215417a_6a10b090","updated":"2024-10-08 17:18:14.000000000","message":"Great work here. Sorry it took so long to get back to it.","commit_id":"aab2958a4f0af09b36b56123a96f0957dda15798"},{"author":{"_account_id":36482,"name":"Oria Weng","display_name":"0weng","email":"oweng@osuosl.org","username":"0weng"},"change_message_id":"ab16cbe67f9b916335ef6a0c2f955157c650d73e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":25,"id":"1c31cdb6_aeaa3600","updated":"2024-10-28 21:27:57.000000000","message":"recheck","commit_id":"aff5e358ac9e9da184129303ac54a8dda268ef29"},{"author":{"_account_id":36482,"name":"Oria Weng","display_name":"0weng","email":"oweng@osuosl.org","username":"0weng"},"change_message_id":"501074135a17e4383cff9018e99eda05dcbec0f8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":25,"id":"72d5b983_032da698","updated":"2024-10-29 15:44:48.000000000","message":"recheck","commit_id":"aff5e358ac9e9da184129303ac54a8dda268ef29"},{"author":{"_account_id":36482,"name":"Oria Weng","display_name":"0weng","email":"oweng@osuosl.org","username":"0weng"},"change_message_id":"bd158b663d525f8eefcfacbee87c155083c5b020","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":25,"id":"0c607dbc_7c439a0f","updated":"2024-10-22 15:41:38.000000000","message":"recheck - unrelated infra failure?","commit_id":"aff5e358ac9e9da184129303ac54a8dda268ef29"}],"openstack/identity/v3/_proxy.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b6a6a5466d838420f2ec5bbbc5ea728b3b6c29a7","unresolved":true,"context_lines":[{"line_number":1233,"context_line":"        \"\"\""},{"line_number":1234,"context_line":"        return self._list(_role_assignment.RoleAssignment, **query)"},{"line_number":1235,"context_line":""},{"line_number":1236,"context_line":"    def assign_domain_role_to_user(self, domain, user, role, inherited\u003dFalse):"},{"line_number":1237,"context_line":"        \"\"\"Assign role to user on a domain"},{"line_number":1238,"context_line":""},{"line_number":1239,"context_line":"        :param domain: Either the ID of a domain or a"}],"source_content_type":"text/x-python","patch_set":3,"id":"d085ba31_36911b87","line":1236,"updated":"2024-09-17 10:29:34.000000000","message":"Could you make this a kwarg-only argument?\n\n```suggestion\n    def assign_domain_role_to_user(self, domain, user, role, *, inherited\u003dFalse):\n```\n\nDitto for the other updated methods below","commit_id":"534c8278574d4621ae7421f4edb9071f5d836846"},{"author":{"_account_id":36482,"name":"Oria Weng","display_name":"0weng","email":"oweng@osuosl.org","username":"0weng"},"change_message_id":"e0314628c5d41d51435d520f311a8bd93c958864","unresolved":false,"context_lines":[{"line_number":1233,"context_line":"        \"\"\""},{"line_number":1234,"context_line":"        return self._list(_role_assignment.RoleAssignment, **query)"},{"line_number":1235,"context_line":""},{"line_number":1236,"context_line":"    def assign_domain_role_to_user(self, domain, user, role, inherited\u003dFalse):"},{"line_number":1237,"context_line":"        \"\"\"Assign role to user on a domain"},{"line_number":1238,"context_line":""},{"line_number":1239,"context_line":"        :param domain: Either the ID of a domain or a"}],"source_content_type":"text/x-python","patch_set":3,"id":"49c37b4d_4be7bf65","line":1236,"in_reply_to":"d085ba31_36911b87","updated":"2024-09-17 22:03:35.000000000","message":"Done","commit_id":"534c8278574d4621ae7421f4edb9071f5d836846"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"3bda773fbea7900182b891b5fa172a0d39d611d4","unresolved":false,"context_lines":[{"line_number":1470,"context_line":"            :class:`~openstack.identity.v3.user.User` instance."},{"line_number":1471,"context_line":"        :param role: Either the ID of a role or a"},{"line_number":1472,"context_line":"            :class:`~openstack.identity.v3.role.Role` instance."},{"line_number":1473,"context_line":"        :param bool inherited: Whether the role assignment is inherited."},{"line_number":1474,"context_line":"        :param system: The system name"},{"line_number":1475,"context_line":"        :return: ``None``"},{"line_number":1476,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":22,"id":"c8ade4c0_85753fac","line":1473,"updated":"2024-10-21 13:00:16.000000000","message":"This method does not have this argument. (same below)","commit_id":"aab2958a4f0af09b36b56123a96f0957dda15798"},{"author":{"_account_id":36482,"name":"Oria Weng","display_name":"0weng","email":"oweng@osuosl.org","username":"0weng"},"change_message_id":"3f798ab7d659031929aa13d4799f80964d258383","unresolved":false,"context_lines":[{"line_number":1470,"context_line":"            :class:`~openstack.identity.v3.user.User` instance."},{"line_number":1471,"context_line":"        :param role: Either the ID of a role or a"},{"line_number":1472,"context_line":"            :class:`~openstack.identity.v3.role.Role` instance."},{"line_number":1473,"context_line":"        :param bool inherited: Whether the role assignment is inherited."},{"line_number":1474,"context_line":"        :param system: The system name"},{"line_number":1475,"context_line":"        :return: ``None``"},{"line_number":1476,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":22,"id":"8ddbd5b0_010197f9","line":1473,"in_reply_to":"c8ade4c0_85753fac","updated":"2024-10-21 23:05:52.000000000","message":"Oops, you\u0027re right, I forgot to remove that comment - fixed!","commit_id":"aab2958a4f0af09b36b56123a96f0957dda15798"}],"openstack/identity/v3/domain.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b6a6a5466d838420f2ec5bbbc5ea728b3b6c29a7","unresolved":true,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def assign_role_to_user(self, session, user, role, inherited):"},{"line_number":52,"context_line":"        \"\"\"Assign role to user on domain\"\"\""},{"line_number":53,"context_line":"        url \u003d utils.urljoin("},{"line_number":54,"context_line":"            (\u0027OS_INHERIT\u0027 if inherited else \u0027\u0027),"},{"line_number":55,"context_line":"            self.base_path,"},{"line_number":56,"context_line":"            self.id,"},{"line_number":57,"context_line":"            \u0027users\u0027,"},{"line_number":58,"context_line":"            user.id,"},{"line_number":59,"context_line":"            \u0027roles\u0027,"},{"line_number":60,"context_line":"            role.id,"},{"line_number":61,"context_line":"            (\u0027inherited_to_projects\u0027 if inherited else \u0027\u0027),"},{"line_number":62,"context_line":"        )"},{"line_number":63,"context_line":"        resp \u003d session.put("},{"line_number":64,"context_line":"            url,"},{"line_number":65,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":3,"id":"182fbf57_46930b1a","line":62,"range":{"start_line":53,"start_character":0,"end_line":62,"end_character":9},"updated":"2024-09-17 10:29:34.000000000","message":"Will this result in two trailing slashes if `inherited\u003d\u003dFalse`? How about this instead:\n\n```suggestion\n        url \u003d utils.urljoin(\n            (\u0027OS_INHERIT\u0027 if inherited else \u0027\u0027),\n            self.base_path,\n            self.id,\n            \u0027users\u0027,\n            user.id,\n            \u0027roles\u0027,\n            role.id,\n        )\n        if inherited:\n            url \u003d utils.urljoin(url, \u0027inherited_to_projects\u0027)\n```\n\nDitto for the other [hunk](https://stackoverflow.com/questions/37620729/in-the-context-of-git-and-diff-what-is-a-hunk) in this file and the next two files","commit_id":"534c8278574d4621ae7421f4edb9071f5d836846"},{"author":{"_account_id":36482,"name":"Oria Weng","display_name":"0weng","email":"oweng@osuosl.org","username":"0weng"},"change_message_id":"e0314628c5d41d51435d520f311a8bd93c958864","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def assign_role_to_user(self, session, user, role, inherited):"},{"line_number":52,"context_line":"        \"\"\"Assign role to user on domain\"\"\""},{"line_number":53,"context_line":"        url \u003d utils.urljoin("},{"line_number":54,"context_line":"            (\u0027OS_INHERIT\u0027 if inherited else \u0027\u0027),"},{"line_number":55,"context_line":"            self.base_path,"},{"line_number":56,"context_line":"            self.id,"},{"line_number":57,"context_line":"            \u0027users\u0027,"},{"line_number":58,"context_line":"            user.id,"},{"line_number":59,"context_line":"            \u0027roles\u0027,"},{"line_number":60,"context_line":"            role.id,"},{"line_number":61,"context_line":"            (\u0027inherited_to_projects\u0027 if inherited else \u0027\u0027),"},{"line_number":62,"context_line":"        )"},{"line_number":63,"context_line":"        resp \u003d session.put("},{"line_number":64,"context_line":"            url,"},{"line_number":65,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":3,"id":"c42c9a45_58c52b3a","line":62,"range":{"start_line":53,"start_character":0,"end_line":62,"end_character":9},"in_reply_to":"182fbf57_46930b1a","updated":"2024-09-17 22:03:35.000000000","message":"Done","commit_id":"534c8278574d4621ae7421f4edb9071f5d836846"}],"openstack/tests/unit/cloud/test_role_assignment.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9e9bccdf392632f3c10b82dd555dc96c42afbeba","unresolved":true,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class TestRoleAssignment(base.TestCase):"},{"line_number":22,"context_line":"    def _is_inheritance_testcase(self):"},{"line_number":23,"context_line":"        return False"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    def _build_role_assignment_response("},{"line_number":26,"context_line":"        self, role_id, scope_type, scope_id, entity_type, entity_id"}],"source_content_type":"text/x-python","patch_set":19,"id":"a9b85408_47c05a8b","line":23,"range":{"start_line":22,"start_character":0,"end_line":23,"end_character":20},"updated":"2024-09-24 14:48:32.000000000","message":"This is a rather unusual way to do this. Why not have a simple class attribute and use that in the tests?\n\n```suggestion\n    _IS_INHERITED \u003d False\n```","commit_id":"10768d3d9ad26646a323abedc166c0135fa1eaad"},{"author":{"_account_id":36482,"name":"Oria Weng","display_name":"0weng","email":"oweng@osuosl.org","username":"0weng"},"change_message_id":"6c85ddbd451f7acf953a6df6e5dfad2cc1c29606","unresolved":false,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class TestRoleAssignment(base.TestCase):"},{"line_number":22,"context_line":"    def _is_inheritance_testcase(self):"},{"line_number":23,"context_line":"        return False"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    def _build_role_assignment_response("},{"line_number":26,"context_line":"        self, role_id, scope_type, scope_id, entity_type, entity_id"}],"source_content_type":"text/x-python","patch_set":19,"id":"1295ab19_511fe582","line":23,"range":{"start_line":22,"start_character":0,"end_line":23,"end_character":20},"in_reply_to":"a9b85408_47c05a8b","updated":"2024-09-24 15:32:51.000000000","message":"Done\n\nI was following the format that the unit tests for the role commands in SDK use; this looks much cleaner! :)","commit_id":"10768d3d9ad26646a323abedc166c0135fa1eaad"}],"openstack/tests/unit/identity/v3/test_proxy.py":[{"author":{"_account_id":36482,"name":"Oria Weng","display_name":"0weng","email":"oweng@osuosl.org","username":"0weng"},"change_message_id":"aaa2d93f581b79523ec48c3898073277ef8a5a77","unresolved":true,"context_lines":[{"line_number":485,"context_line":"            },"},{"line_number":486,"context_line":"        )"},{"line_number":487,"context_line":""},{"line_number":488,"context_line":"    def test_assign_domain_role_to_user(self):"},{"line_number":489,"context_line":"        self._verify("},{"line_number":490,"context_line":"            \"openstack.identity.v3.domain.Domain.assign_role_to_user\","},{"line_number":491,"context_line":"            self.proxy.assign_domain_role_to_user,"}],"source_content_type":"text/x-python","patch_set":14,"id":"b6769257_d5fd4e33","line":488,"updated":"2024-09-19 22:57:18.000000000","message":"Do I need to add any additional tests here? My understanding is that these tests should check that the functions in `_proxy.py` create the correct outputs when they differ from the inputs (e.g., the test below checks that `openstack.identity.v3._proxy.Proxy.assign_domain_role_to_user()` passes a `User` and a `Role` to `openstack.identity.v3.domain.Domain.assign_role_to_user()`, not the `uid` and `rid`) and that it\u0027s thus not a good place to test the `inherited` arg, but I wanted to double check that I\u0027m interpreting correctly. :)","commit_id":"55effaa4eafecfd6aa2019d058a6a57f2cf90b25"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c0324263b4e0d146e333252d11a19bbbd4656805","unresolved":false,"context_lines":[{"line_number":485,"context_line":"            },"},{"line_number":486,"context_line":"        )"},{"line_number":487,"context_line":""},{"line_number":488,"context_line":"    def test_assign_domain_role_to_user(self):"},{"line_number":489,"context_line":"        self._verify("},{"line_number":490,"context_line":"            \"openstack.identity.v3.domain.Domain.assign_role_to_user\","},{"line_number":491,"context_line":"            self.proxy.assign_domain_role_to_user,"}],"source_content_type":"text/x-python","patch_set":14,"id":"24b55c25_bc074886","line":488,"in_reply_to":"b6769257_d5fd4e33","updated":"2024-09-20 12:42:52.000000000","message":"That\u0027s correct. We check the call to the method in the second argument (the caller) results in a call to the method given by the first (the called). `method_(kw)args` are passed to the caller, while `expected_(kw)args` are what we expected to be given to the called method.\n\nI think what you\u0027ve done is a-okay 👌","commit_id":"55effaa4eafecfd6aa2019d058a6a57f2cf90b25"}]}