)]}'
{"keystone/assignment/core.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"c49c588f7da454eca17271b26bef5922ba88b65c","unresolved":false,"context_lines":[{"line_number":470,"context_line":"                users \u003d self.identity_api.list_users_in_group(ref[\u0027group_id\u0027])"},{"line_number":471,"context_line":"            except exception.GroupNotFound:"},{"line_number":472,"context_line":"                msg \u003d (\u0027Group %(group)s not found in the\u0027"},{"line_number":473,"context_line":"                       \u0027 backend but still has role assignments.\u0027)"},{"line_number":474,"context_line":"                LOG.warning(msg, {\u0027group\u0027: ref[\u0027group_id\u0027]})"},{"line_number":475,"context_line":"                users \u003d []"},{"line_number":476,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ff0f0b1f_1a54f253","line":473,"updated":"2017-05-25 21:35:43.000000000","message":"nit: We could make this a bit more descriptive so that the reader knows which backend we\u0027re referring to.","commit_id":"f15727c8487ed47d665dc63a7919f4a2f5e35093"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"5e089b4cf59100dcdc171b4a2dc4403138e4a841","unresolved":false,"context_lines":[{"line_number":470,"context_line":"                users \u003d self.identity_api.list_users_in_group(ref[\u0027group_id\u0027])"},{"line_number":471,"context_line":"            except exception.GroupNotFound:"},{"line_number":472,"context_line":"                msg \u003d (\u0027Group %(group)s not found in the\u0027"},{"line_number":473,"context_line":"                       \u0027 backend but still has role assignments.\u0027)"},{"line_number":474,"context_line":"                LOG.warning(msg, {\u0027group\u0027: ref[\u0027group_id\u0027]})"},{"line_number":475,"context_line":"                users \u003d []"},{"line_number":476,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ff0f0b1f_70f1cd01","line":473,"in_reply_to":"ff0f0b1f_1a54f253","updated":"2017-05-25 22:33:34.000000000","message":"we probably just shouldn\u0027t mention the backend at all. I don\u0027t think that\u0027s terminology users are instantly familiar with, and doesn\u0027t really seem important here... just \"Group %(group)s not found but still has role assignments\" should work.","commit_id":"f15727c8487ed47d665dc63a7919f4a2f5e35093"}],"keystone/tests/unit/assignment/test_backends.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"c49c588f7da454eca17271b26bef5922ba88b65c","unresolved":false,"context_lines":[{"line_number":1447,"context_line":"        # role assignments."},{"line_number":1448,"context_line":"        with mock.patch.object(self.identity_api, \u0027list_users_in_group\u0027,"},{"line_number":1449,"context_line":"                               _group_not_found):"},{"line_number":1450,"context_line":"            a_proj_list \u003d self.assignment_api.get_roles_for_user_and_project("},{"line_number":1451,"context_line":"                              user1[\u0027id\u0027], project1[\u0027id\u0027])"},{"line_number":1452,"context_line":"        self.assertEqual(2, len(a_proj_list))"},{"line_number":1453,"context_line":"        self.assertIn(role_list[4][\u0027id\u0027], a_proj_list)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff0f0b1f_9a612215","line":1450,"updated":"2017-05-25 21:35:43.000000000","message":"The indentation here is causing pep8 to fail. We can mitigate this by doing something like:\n\n  project_list \u003d self.assignment_api.get_roles_for_user_and_project(\n      user1[\u0027id\u0027], project1[\u0027id\u0027]\n  )","commit_id":"f15727c8487ed47d665dc63a7919f4a2f5e35093"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"c49c588f7da454eca17271b26bef5922ba88b65c","unresolved":false,"context_lines":[{"line_number":1448,"context_line":"        with mock.patch.object(self.identity_api, \u0027list_users_in_group\u0027,"},{"line_number":1449,"context_line":"                               _group_not_found):"},{"line_number":1450,"context_line":"            a_proj_list \u003d self.assignment_api.get_roles_for_user_and_project("},{"line_number":1451,"context_line":"                              user1[\u0027id\u0027], project1[\u0027id\u0027])"},{"line_number":1452,"context_line":"        self.assertEqual(2, len(a_proj_list))"},{"line_number":1453,"context_line":"        self.assertIn(role_list[4][\u0027id\u0027], a_proj_list)"},{"line_number":1454,"context_line":"        self.assertIn(role_list[5][\u0027id\u0027], a_proj_list)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff0f0b1f_7ac1aee4","line":1451,"updated":"2017-05-25 21:35:43.000000000","message":"I wonder if the mock isn\u0027t working properly here because the assertion fails with 4 instead of 2.","commit_id":"f15727c8487ed47d665dc63a7919f4a2f5e35093"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"5e089b4cf59100dcdc171b4a2dc4403138e4a841","unresolved":false,"context_lines":[{"line_number":1448,"context_line":"        with mock.patch.object(self.identity_api, \u0027list_users_in_group\u0027,"},{"line_number":1449,"context_line":"                               _group_not_found):"},{"line_number":1450,"context_line":"            a_proj_list \u003d self.assignment_api.get_roles_for_user_and_project("},{"line_number":1451,"context_line":"                              user1[\u0027id\u0027], project1[\u0027id\u0027])"},{"line_number":1452,"context_line":"        self.assertEqual(2, len(a_proj_list))"},{"line_number":1453,"context_line":"        self.assertIn(role_list[4][\u0027id\u0027], a_proj_list)"},{"line_number":1454,"context_line":"        self.assertIn(role_list[5][\u0027id\u0027], a_proj_list)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff0f0b1f_505da9e7","line":1451,"in_reply_to":"ff0f0b1f_7ac1aee4","updated":"2017-05-25 22:33:34.000000000","message":"I think that\u0027s actually correct... the 2 user assignments still come back, but the 2 group assignments no longer show for the user because the group is not found (and a user can\u0027t be in a group that doesn\u0027t exist).","commit_id":"f15727c8487ed47d665dc63a7919f4a2f5e35093"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"c49c588f7da454eca17271b26bef5922ba88b65c","unresolved":false,"context_lines":[{"line_number":1458,"context_line":"        with mock.patch.object(self.identity_api, \u0027list_users_in_group\u0027,"},{"line_number":1459,"context_line":"                               _group_not_found):"},{"line_number":1460,"context_line":"            a_domain_list \u003d self.assignment_api.get_roles_for_user_and_project("},{"line_number":1461,"context_line":"                                user1[\u0027id\u0027], domain1[\u0027id\u0027])"},{"line_number":1462,"context_line":"        self.assertEqual(2, len(a_domain_list))"},{"line_number":1463,"context_line":"        self.assertIn(role_list[0][\u0027id\u0027], a_domain_list)"},{"line_number":1464,"context_line":"        self.assertIn(role_list[1][\u0027id\u0027], a_domain_list)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff0f0b1f_3a70d6dc","line":1461,"updated":"2017-05-25 21:35:43.000000000","message":"Same comment here as above.","commit_id":"f15727c8487ed47d665dc63a7919f4a2f5e35093"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"3c7270ec2fd4a2c13caa1625e7497c0b4c3c2209","unresolved":false,"context_lines":[{"line_number":652,"context_line":""},{"line_number":653,"context_line":"        num_effective \u003d len(self.assignment_api.list_role_assignments("},{"line_number":654,"context_line":"            effective\u003dTrue))"},{"line_number":655,"context_line":"        self.assertGreater(num_effective, len(assignment_list))"},{"line_number":656,"context_line":""},{"line_number":657,"context_line":"        # Patch list_users_in_group to return GroupNotFound allowing us to"},{"line_number":658,"context_line":"        # confirm that the exception is handled properly when effective"}],"source_content_type":"text/x-python","patch_set":7,"id":"df140735_57057ac1","line":655,"updated":"2017-05-30 15:50:49.000000000","message":"Don\u0027t we know the values of these? Can\u0027t these be more explicit assertions that help readers understand exactly what the values are?","commit_id":"d09c337619fed8664272848abb3a1351dd5e4c85"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"9343651f06070276f9dfbb8483f15b9deb7ebbd3","unresolved":false,"context_lines":[{"line_number":652,"context_line":""},{"line_number":653,"context_line":"        num_effective \u003d len(self.assignment_api.list_role_assignments("},{"line_number":654,"context_line":"            effective\u003dTrue))"},{"line_number":655,"context_line":"        self.assertGreater(num_effective, len(assignment_list))"},{"line_number":656,"context_line":""},{"line_number":657,"context_line":"        # Patch list_users_in_group to return GroupNotFound allowing us to"},{"line_number":658,"context_line":"        # confirm that the exception is handled properly when effective"}],"source_content_type":"text/x-python","patch_set":7,"id":"df140735_22363d05","line":655,"in_reply_to":"df140735_57057ac1","updated":"2017-05-30 19:10:16.000000000","message":"we could give exact numbers, yes, but I thought it was actually better this way. num_effective being \u003e len(assignment_list) shows that there was expansion. If we did a check for a number, it\u0027s a lot less clear to a reader where that number came from and easier to mess things up in future edits.","commit_id":"d09c337619fed8664272848abb3a1351dd5e4c85"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"3c7270ec2fd4a2c13caa1625e7497c0b4c3c2209","unresolved":false,"context_lines":[{"line_number":664,"context_line":"                effective\u003dTrue"},{"line_number":665,"context_line":"            )"},{"line_number":666,"context_line":""},{"line_number":667,"context_line":"        self.assertGreater(num_effective, len(assignment_list))"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        # cleanup"},{"line_number":670,"context_line":"        self.assignment_api.delete_grant("}],"source_content_type":"text/x-python","patch_set":7,"id":"df140735_97c9d279","line":667,"updated":"2017-05-30 15:50:49.000000000","message":"Same comment here. I think the readability of the test would be improved significantly. \n\nThis is also something we can do in a follow-on patch.","commit_id":"d09c337619fed8664272848abb3a1351dd5e4c85"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"9343651f06070276f9dfbb8483f15b9deb7ebbd3","unresolved":false,"context_lines":[{"line_number":664,"context_line":"                effective\u003dTrue"},{"line_number":665,"context_line":"            )"},{"line_number":666,"context_line":""},{"line_number":667,"context_line":"        self.assertGreater(num_effective, len(assignment_list))"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        # cleanup"},{"line_number":670,"context_line":"        self.assignment_api.delete_grant("}],"source_content_type":"text/x-python","patch_set":7,"id":"df140735_a2212db7","line":667,"in_reply_to":"df140735_97c9d279","updated":"2017-05-30 19:10:16.000000000","message":"same response as above.","commit_id":"d09c337619fed8664272848abb3a1351dd5e4c85"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"3c7270ec2fd4a2c13caa1625e7497c0b4c3c2209","unresolved":false,"context_lines":[{"line_number":672,"context_line":"            domain_id\u003ddomain_id,"},{"line_number":673,"context_line":"            role_id\u003ddefault_fixtures.MEMBER_ROLE_ID)"},{"line_number":674,"context_line":"        # TODO(edmondsw) should cleanup users/groups as well, but that raises"},{"line_number":675,"context_line":"        # LDAP read-only issues"},{"line_number":676,"context_line":""},{"line_number":677,"context_line":"    def test_add_duplicate_role_grant(self):"},{"line_number":678,"context_line":"        roles_ref \u003d self.assignment_api.get_roles_for_user_and_project("}],"source_content_type":"text/x-python","patch_set":7,"id":"df140735_17b262e7","line":675,"updated":"2017-05-30 15:50:49.000000000","message":"How does this not raise read-only errors when we setup the original groups and users at the beginning of the test then?","commit_id":"d09c337619fed8664272848abb3a1351dd5e4c85"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"9343651f06070276f9dfbb8483f15b9deb7ebbd3","unresolved":false,"context_lines":[{"line_number":672,"context_line":"            domain_id\u003ddomain_id,"},{"line_number":673,"context_line":"            role_id\u003ddefault_fixtures.MEMBER_ROLE_ID)"},{"line_number":674,"context_line":"        # TODO(edmondsw) should cleanup users/groups as well, but that raises"},{"line_number":675,"context_line":"        # LDAP read-only issues"},{"line_number":676,"context_line":""},{"line_number":677,"context_line":"    def test_add_duplicate_role_grant(self):"},{"line_number":678,"context_line":"        roles_ref \u003d self.assignment_api.get_roles_for_user_and_project("}],"source_content_type":"text/x-python","patch_set":7,"id":"df140735_45e93b18","line":675,"in_reply_to":"df140735_17b262e7","updated":"2017-05-30 19:10:16.000000000","message":"because things have been hacked to still allow group/user creation for LDAP for UT, but not deletion. See https://github.com/openstack/keystone/blob/master/keystone/identity/backends/ldap/core.py#L154-L168 . I chatted with knikolla about this on Friday, and he\u0027s in the process of working on improving LDAP tests but was ok with what I\u0027d done here for now.","commit_id":"d09c337619fed8664272848abb3a1351dd5e4c85"}]}