)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"f61bd03558cc1222cc085b7675fd3b89a1109f86","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Kristi Nikolla \u003cknikolla@bu.edu\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-03-08 12:16:35 -0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Remove LDAP delete logic and associated tests"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I9f183492fd318c755ba026e9e402615fe66f100c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"9a30ddce_69d2f75d","line":7,"updated":"2017-03-09 17:31:17.000000000","message":"This was deprecated and staged for removal wasn\u0027t it? If so, we should tag it as removed-as-of-pike and add a release note.","commit_id":"dfb57c681ebfd8ef07dc41512ae46ad7e907a7a2"},{"author":{"_account_id":16465,"name":"Kristi Nikolla","email":"knikolla@bu.edu","username":"knikolla"},"change_message_id":"aaa4b0ff55019e6b698720d086ed1d31c9419c79","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Kristi Nikolla \u003cknikolla@bu.edu\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-03-08 12:16:35 -0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Remove LDAP delete logic and associated tests"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I9f183492fd318c755ba026e9e402615fe66f100c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"9a30ddce_22770803","line":7,"in_reply_to":"9a30ddce_240d5de8","updated":"2017-03-10 16:20:15.000000000","message":"Will do in the next patchset. I can understand why this may be confusing to some.","commit_id":"dfb57c681ebfd8ef07dc41512ae46ad7e907a7a2"},{"author":{"_account_id":16465,"name":"Kristi Nikolla","email":"knikolla@bu.edu","username":"knikolla"},"change_message_id":"ae11bdea33b24ec11fb498a999926d06caedbb90","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Kristi Nikolla \u003cknikolla@bu.edu\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-03-08 12:16:35 -0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Remove LDAP delete logic and associated tests"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I9f183492fd318c755ba026e9e402615fe66f100c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"9a30ddce_fe6773e6","line":7,"in_reply_to":"9a30ddce_69d2f75d","updated":"2017-03-09 20:07:54.000000000","message":"The possibility to do any change to LDAP has been already removed in Ocata, this is just cleanup for the unit tests and remaining logic.\n\nI can change the topic, but the release note already exists as I linked you in IRC.","commit_id":"dfb57c681ebfd8ef07dc41512ae46ad7e907a7a2"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"72d29838c4bcbd93828808523f122f8682d81c68","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Kristi Nikolla \u003cknikolla@bu.edu\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-03-08 12:16:35 -0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Remove LDAP delete logic and associated tests"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I9f183492fd318c755ba026e9e402615fe66f100c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"9a30ddce_240d5de8","line":7,"in_reply_to":"9a30ddce_fe6773e6","updated":"2017-03-10 14:36:27.000000000","message":"Maybe the commit message could be expanded to mention that this is just cleanup.","commit_id":"dfb57c681ebfd8ef07dc41512ae46ad7e907a7a2"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"6b7193bff4105aa4914928045cd5818e35766b34","unresolved":false,"context_lines":[{"line_number":16,"context_line":"those unit tests don\u0027t depend anymore on being able to do deletes"},{"line_number":17,"context_line":"to LDAP, the associated delete logic in the driver is removed."},{"line_number":18,"context_line":"None of this is user facing or changes anything in the Keystone"},{"line_number":19,"context_line":"behavior."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"[0] I13eada3d5c3a166223c3e3ce70b7054eaed1003a"},{"line_number":22,"context_line":"[1] I225daf0e00742c54d5d009e456d4a3ad864356a0"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"bff0334d_f9de303c","line":19,"updated":"2017-04-11 19:44:25.000000000","message":"Is a 403 still returned if a user attempts to write to an ldap-backed keystone? What catches that logic if it\u0027s no longer being caught by the logic you\u0027re removing?","commit_id":"1845afd239a9ddfa16b134ac1cd60bd059acf8dc"},{"author":{"_account_id":16465,"name":"Kristi Nikolla","email":"knikolla@bu.edu","username":"knikolla"},"change_message_id":"9e7392b301958f0cb59bdbc9b24fa560e16f476b","unresolved":false,"context_lines":[{"line_number":16,"context_line":"those unit tests don\u0027t depend anymore on being able to do deletes"},{"line_number":17,"context_line":"to LDAP, the associated delete logic in the driver is removed."},{"line_number":18,"context_line":"None of this is user facing or changes anything in the Keystone"},{"line_number":19,"context_line":"behavior."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"[0] I13eada3d5c3a166223c3e3ce70b7054eaed1003a"},{"line_number":22,"context_line":"[1] I225daf0e00742c54d5d009e456d4a3ad864356a0"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"bff0334d_d497b147","line":19,"in_reply_to":"bff0334d_f9de303c","updated":"2017-04-11 20:09:18.000000000","message":"As answered in IRC, [0][1] already did that. More specifically: \n\nhttps://github.com/openstack/keystone/blob/af4e98c770d771144463e6dd49cb4b559d48c403/keystone/identity/backends/ldap/core.py#L155-L161\n\nhttps://github.com/openstack/keystone/blob/fa63f893d487d54fe932e42ad9b53eea7a24932f/keystone/tests/unit/ksfixtures/ldapdb.py#L29","commit_id":"1845afd239a9ddfa16b134ac1cd60bd059acf8dc"}],"keystone/identity/backends/ldap/common.py":[{"author":{"_account_id":17860,"name":"Samuel de Medeiros Queiroz","email":"samueldmq@gmail.com","username":"samueldmq"},"change_message_id":"571f88749602765801c4305c6c289534a9d9ec6f","unresolved":false,"context_lines":[{"line_number":1547,"context_line":""},{"line_number":1548,"context_line":"            if v is None:"},{"line_number":1549,"context_line":"                if old_obj.get(k) is not None:"},{"line_number":1550,"context_line":"                    modlist.append((ldap.MOD_DELETE,"},{"line_number":1551,"context_line":"                                    self.attribute_mapping.get(k, k),"},{"line_number":1552,"context_line":"                                    None))"},{"line_number":1553,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":8,"id":"bff0334d_95b7a487","side":"PARENT","line":1550,"range":{"start_line":1550,"start_character":36,"end_line":1550,"end_character":51},"updated":"2017-04-05 19:08:15.000000000","message":"do you have an idea of what this is ?","commit_id":"2481ceb5438023891c0d15b5d274a90aef3e5c00"},{"author":{"_account_id":16465,"name":"Kristi Nikolla","email":"knikolla@bu.edu","username":"knikolla"},"change_message_id":"a460e089dfa2ddbbd2a5f98abcecdaed4253eb24","unresolved":false,"context_lines":[{"line_number":1547,"context_line":""},{"line_number":1548,"context_line":"            if v is None:"},{"line_number":1549,"context_line":"                if old_obj.get(k) is not None:"},{"line_number":1550,"context_line":"                    modlist.append((ldap.MOD_DELETE,"},{"line_number":1551,"context_line":"                                    self.attribute_mapping.get(k, k),"},{"line_number":1552,"context_line":"                                    None))"},{"line_number":1553,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":8,"id":"bff0334d_6788f21e","side":"PARENT","line":1550,"range":{"start_line":1550,"start_character":36,"end_line":1550,"end_character":51},"in_reply_to":"bff0334d_95b7a487","updated":"2017-04-05 21:39:21.000000000","message":"It\u0027s deleting an attribute of a user when it\u0027s updated to a None value. My next patch will be similar to this one, but remove all update operations (hopefully).","commit_id":"2481ceb5438023891c0d15b5d274a90aef3e5c00"},{"author":{"_account_id":17860,"name":"Samuel de Medeiros Queiroz","email":"samueldmq@gmail.com","username":"samueldmq"},"change_message_id":"571f88749602765801c4305c6c289534a9d9ec6f","unresolved":false,"context_lines":[{"line_number":1825,"context_line":"                    conn.add_s(self.enabled_emulation_dn, attr_list)"},{"line_number":1826,"context_line":""},{"line_number":1827,"context_line":"    def _remove_enabled(self, object_id):"},{"line_number":1828,"context_line":"        modlist \u003d [(ldap.MOD_DELETE,"},{"line_number":1829,"context_line":"                    self.member_attribute,"},{"line_number":1830,"context_line":"                    [self._id_to_dn(object_id)])]"},{"line_number":1831,"context_line":"        with self.get_connection() as conn:"}],"source_content_type":"text/x-python","patch_set":8,"id":"bff0334d_55b11c8f","side":"PARENT","line":1828,"range":{"start_line":1828,"start_character":20,"end_line":1828,"end_character":35},"updated":"2017-04-05 19:08:15.000000000","message":"here again","commit_id":"2481ceb5438023891c0d15b5d274a90aef3e5c00"},{"author":{"_account_id":16465,"name":"Kristi Nikolla","email":"knikolla@bu.edu","username":"knikolla"},"change_message_id":"a460e089dfa2ddbbd2a5f98abcecdaed4253eb24","unresolved":false,"context_lines":[{"line_number":1825,"context_line":"                    conn.add_s(self.enabled_emulation_dn, attr_list)"},{"line_number":1826,"context_line":""},{"line_number":1827,"context_line":"    def _remove_enabled(self, object_id):"},{"line_number":1828,"context_line":"        modlist \u003d [(ldap.MOD_DELETE,"},{"line_number":1829,"context_line":"                    self.member_attribute,"},{"line_number":1830,"context_line":"                    [self._id_to_dn(object_id)])]"},{"line_number":1831,"context_line":"        with self.get_connection() as conn:"}],"source_content_type":"text/x-python","patch_set":8,"id":"bff0334d_87b1b6e4","side":"PARENT","line":1828,"range":{"start_line":1828,"start_character":20,"end_line":1828,"end_character":35},"in_reply_to":"bff0334d_55b11c8f","updated":"2017-04-05 21:39:21.000000000","message":"This is deleting the \u0027enabled\u0027 attribute.","commit_id":"2481ceb5438023891c0d15b5d274a90aef3e5c00"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"e61e3bf5b3cd7a8037db5e5a709cfc75f80633f4","unresolved":false,"context_lines":[{"line_number":36,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"LDAP_VALUES \u003d {\u0027TRUE\u0027: True, \u0027FALSE\u0027: False}"},{"line_number":39,"context_line":"CONTROL_TREEDELETE \u003d \u00271.2.840.113556.1.4.805\u0027"},{"line_number":40,"context_line":"LDAP_SCOPES \u003d {\u0027one\u0027: ldap.SCOPE_ONELEVEL,"},{"line_number":41,"context_line":"               \u0027sub\u0027: ldap.SCOPE_SUBTREE}"},{"line_number":42,"context_line":"LDAP_DEREF \u003d {\u0027always\u0027: ldap.DEREF_ALWAYS,"}],"source_content_type":"text/x-python","patch_set":10,"id":"7ffa3b31_69de3512","side":"PARENT","line":39,"updated":"2017-04-18 20:11:15.000000000","message":"Can probably remove this too? http://git.openstack.org/cgit/openstack/keystone/tree/keystone/tests/unit/fakeldap.py#n45","commit_id":"c45912533620b7dc2d6bd2dac293b7dcb39759ff"}],"keystone/identity/backends/ldap/core.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"6b7193bff4105aa4914928045cd5818e35766b34","unresolved":false,"context_lines":[{"line_number":446,"context_line":"        group_ref \u003d self.get(group_id)"},{"line_number":447,"context_line":"        group_dn \u003d group_ref[\u0027dn\u0027]"},{"line_number":448,"context_line":"        try:"},{"line_number":449,"context_line":"            super(GroupApi, self).remove_member(user_dn, group_dn)"},{"line_number":450,"context_line":"        except ldap.NO_SUCH_ATTRIBUTE:"},{"line_number":451,"context_line":"            raise exception.UserNotFound(user_id\u003duser_id)"},{"line_number":452,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"bff0334d_19b1d4e1","side":"PARENT","line":449,"updated":"2017-04-11 19:44:25.000000000","message":"This looks be the only call to remove_member left in keystone. Can this be removed too? \n\n  \u003e grep -Rn --exclude-dir tests remove_member keystone/\n  keystone/identity/backends/ldap/core.py:412:            super(GroupApi, self).remove_member(user_dn, group_dn)","commit_id":"c45912533620b7dc2d6bd2dac293b7dcb39759ff"},{"author":{"_account_id":16465,"name":"Kristi Nikolla","email":"knikolla@bu.edu","username":"knikolla"},"change_message_id":"9e7392b301958f0cb59bdbc9b24fa560e16f476b","unresolved":false,"context_lines":[{"line_number":446,"context_line":"        group_ref \u003d self.get(group_id)"},{"line_number":447,"context_line":"        group_dn \u003d group_ref[\u0027dn\u0027]"},{"line_number":448,"context_line":"        try:"},{"line_number":449,"context_line":"            super(GroupApi, self).remove_member(user_dn, group_dn)"},{"line_number":450,"context_line":"        except ldap.NO_SUCH_ATTRIBUTE:"},{"line_number":451,"context_line":"            raise exception.UserNotFound(user_id\u003duser_id)"},{"line_number":452,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"bff0334d_94c80974","side":"PARENT","line":449,"in_reply_to":"bff0334d_19b1d4e1","updated":"2017-04-11 20:09:18.000000000","message":"Nice catch! Will do in the next patchset.","commit_id":"c45912533620b7dc2d6bd2dac293b7dcb39759ff"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"6b7193bff4105aa4914928045cd5818e35766b34","unresolved":false,"context_lines":[{"line_number":165,"context_line":"        return self._update_user(user_id, user)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"    def delete_user(self, user_id):"},{"line_number":168,"context_line":"        raise exception.Forbidden(READ_ONLY_LDAP_ERROR_MESSAGE)"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def change_password(self, user_id, new_password):"},{"line_number":171,"context_line":"        raise exception.Forbidden(READ_ONLY_LDAP_ERROR_MESSAGE)"}],"source_content_type":"text/x-python","patch_set":9,"id":"bff0334d_f9e870b3","line":168,"updated":"2017-04-11 19:44:25.000000000","message":"Aha - nice. This essentially answers the question I had in the commit message.","commit_id":"1845afd239a9ddfa16b134ac1cd60bd059acf8dc"}],"keystone/tests/unit/filtering.py":[{"author":{"_account_id":16465,"name":"Kristi Nikolla","email":"knikolla@bu.edu","username":"knikolla"},"change_message_id":"4a93aa6e861a8d688a7f26f7c97a0cd96b822457","unresolved":false,"context_lines":[{"line_number":122,"context_line":"        return entity_list"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def _delete_test_data(self, entity_type, entity_list):"},{"line_number":125,"context_line":"        if __name__ \u003d\u003d \u0027__main__\u0027:"},{"line_number":126,"context_line":"            for entity in entity_list:"},{"line_number":127,"context_line":"                try:"},{"line_number":128,"context_line":"                    self._delete_entity(entity_type)(entity[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a30ddce_ce807749","line":125,"updated":"2017-03-08 17:14:22.000000000","message":"Not sure how this got here.","commit_id":"293b73d0a6f370ed0caaa3f2370e7bc5d6c455d8"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"6b7193bff4105aa4914928045cd5818e35766b34","unresolved":false,"context_lines":[{"line_number":126,"context_line":"            try:"},{"line_number":127,"context_line":"                self._delete_entity(entity_type)(entity[\u0027id\u0027])"},{"line_number":128,"context_line":"            except exception.Forbidden:"},{"line_number":129,"context_line":"                # Note(knikolla): Some identity backends such as LDAP are"},{"line_number":130,"context_line":"                # read only"},{"line_number":131,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":9,"id":"bff0334d_19ff9467","line":129,"updated":"2017-04-11 19:44:25.000000000","message":"We could log something here so that operators have some insight, just saying \"hey, someone is trying to write to LDAP but its not supported\"","commit_id":"1845afd239a9ddfa16b134ac1cd60bd059acf8dc"},{"author":{"_account_id":16465,"name":"Kristi Nikolla","email":"knikolla@bu.edu","username":"knikolla"},"change_message_id":"9e7392b301958f0cb59bdbc9b24fa560e16f476b","unresolved":false,"context_lines":[{"line_number":126,"context_line":"            try:"},{"line_number":127,"context_line":"                self._delete_entity(entity_type)(entity[\u0027id\u0027])"},{"line_number":128,"context_line":"            except exception.Forbidden:"},{"line_number":129,"context_line":"                # Note(knikolla): Some identity backends such as LDAP are"},{"line_number":130,"context_line":"                # read only"},{"line_number":131,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":9,"id":"bff0334d_54bf21d0","line":129,"in_reply_to":"bff0334d_19ff9467","updated":"2017-04-11 20:09:18.000000000","message":"As answered in IRC, that has already been done in [0][1] (references in commit message)","commit_id":"1845afd239a9ddfa16b134ac1cd60bd059acf8dc"}],"keystone/tests/unit/identity/test_backends.py":[{"author":{"_account_id":16465,"name":"Kristi Nikolla","email":"knikolla@bu.edu","username":"knikolla"},"change_message_id":"679438c6615e338e25fa3d818d3261476921ab46","unresolved":false,"context_lines":[{"line_number":702,"context_line":"                negative_user[\u0027id\u0027])"},{"line_number":703,"context_line":"            self.assertEqual(0, len(group_refs))"},{"line_number":704,"context_line":""},{"line_number":705,"context_line":"        # remove the user from each group and ensure that"},{"line_number":706,"context_line":"        # the group count reduces by one for each"},{"line_number":707,"context_line":"        for x in range(0, 3):"},{"line_number":708,"context_line":"            before_count \u003d GROUP_COUNT - x"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a3905b3_e8ea2673","side":"PARENT","line":705,"updated":"2017-01-23 21:54:08.000000000","message":"Thoughts on this? Why would test_list_groups_for_user also test the delete functionality?","commit_id":"fa63f893d487d54fe932e42ad9b53eea7a24932f"},{"author":{"_account_id":16465,"name":"Kristi Nikolla","email":"knikolla@bu.edu","username":"knikolla"},"change_message_id":"be1827b3756b80b96f58c38abbeb1b510da2f065","unresolved":false,"context_lines":[{"line_number":702,"context_line":"                negative_user[\u0027id\u0027])"},{"line_number":703,"context_line":"            self.assertEqual(0, len(group_refs))"},{"line_number":704,"context_line":""},{"line_number":705,"context_line":"        # remove the user from each group and ensure that"},{"line_number":706,"context_line":"        # the group count reduces by one for each"},{"line_number":707,"context_line":"        for x in range(0, 3):"},{"line_number":708,"context_line":"            before_count \u003d GROUP_COUNT - x"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a3905b3_21dfc43a","side":"PARENT","line":705,"in_reply_to":"5a3905b3_2218c174","updated":"2017-01-24 15:00:41.000000000","message":"Yeah, I meant remove. Sorry. \n\nHowever R/W backends also subclass their tests from this, so removing this impacts tests for R/W backends too.","commit_id":"fa63f893d487d54fe932e42ad9b53eea7a24932f"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"0f4b83cf0917520de80b113d1b12af51c96b0ee7","unresolved":false,"context_lines":[{"line_number":702,"context_line":"                negative_user[\u0027id\u0027])"},{"line_number":703,"context_line":"            self.assertEqual(0, len(group_refs))"},{"line_number":704,"context_line":""},{"line_number":705,"context_line":"        # remove the user from each group and ensure that"},{"line_number":706,"context_line":"        # the group count reduces by one for each"},{"line_number":707,"context_line":"        for x in range(0, 3):"},{"line_number":708,"context_line":"            before_count \u003d GROUP_COUNT - x"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a3905b3_2218c174","side":"PARENT","line":705,"in_reply_to":"5a3905b3_e8ea2673","updated":"2017-01-24 04:17:27.000000000","message":"it\u0027s not testing delete, it was testing remove_user_from_group -- so the number of users goes down as well.\n\nsafe to remove since remove/add_user_from_group will not be supported in R/O mode","commit_id":"fa63f893d487d54fe932e42ad9b53eea7a24932f"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"f61bd03558cc1222cc085b7675fd3b89a1109f86","unresolved":false,"context_lines":[{"line_number":761,"context_line":"            self.assertEqual(0, len(group_refs))"},{"line_number":762,"context_line":""},{"line_number":763,"context_line":"        # remove the user from each group and ensure that"},{"line_number":764,"context_line":"        # the group count reduces by one for each"},{"line_number":765,"context_line":"        for x in range(0, 3):"},{"line_number":766,"context_line":"            before_count \u003d GROUP_COUNT - x"},{"line_number":767,"context_line":"            after_count \u003d GROUP_COUNT - x - 1"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a30ddce_856c71d1","side":"PARENT","line":764,"updated":"2017-03-09 17:31:17.000000000","message":"Is this not still a valid part of the test?","commit_id":"41b0d28bc33256f5fae75f73905eedf08cc1d760"},{"author":{"_account_id":16465,"name":"Kristi Nikolla","email":"knikolla@bu.edu","username":"knikolla"},"change_message_id":"aaa4b0ff55019e6b698720d086ed1d31c9419c79","unresolved":false,"context_lines":[{"line_number":761,"context_line":"            self.assertEqual(0, len(group_refs))"},{"line_number":762,"context_line":""},{"line_number":763,"context_line":"        # remove the user from each group and ensure that"},{"line_number":764,"context_line":"        # the group count reduces by one for each"},{"line_number":765,"context_line":"        for x in range(0, 3):"},{"line_number":766,"context_line":"            before_count \u003d GROUP_COUNT - x"},{"line_number":767,"context_line":"            after_count \u003d GROUP_COUNT - x - 1"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a30ddce_0225a407","side":"PARENT","line":764,"in_reply_to":"9a30ddce_647915c8","updated":"2017-03-10 16:20:15.000000000","message":"That\u0027s true. It\u0027s inherited by SQL also, however the list_groups functionality is pretty extensively tested without this bit. The other approach would be for me to copy paste this test into the LDAP tests so that it overwrites this inherited test and remove this part there. I was trying to avoid extensive copy-pastes.","commit_id":"41b0d28bc33256f5fae75f73905eedf08cc1d760"},{"author":{"_account_id":16465,"name":"Kristi Nikolla","email":"knikolla@bu.edu","username":"knikolla"},"change_message_id":"ae11bdea33b24ec11fb498a999926d06caedbb90","unresolved":false,"context_lines":[{"line_number":761,"context_line":"            self.assertEqual(0, len(group_refs))"},{"line_number":762,"context_line":""},{"line_number":763,"context_line":"        # remove the user from each group and ensure that"},{"line_number":764,"context_line":"        # the group count reduces by one for each"},{"line_number":765,"context_line":"        for x in range(0, 3):"},{"line_number":766,"context_line":"            before_count \u003d GROUP_COUNT - x"},{"line_number":767,"context_line":"            after_count \u003d GROUP_COUNT - x - 1"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a30ddce_e327e0c7","side":"PARENT","line":764,"in_reply_to":"9a30ddce_856c71d1","updated":"2017-03-09 20:07:54.000000000","message":"Removing a user from a group means deleting stuff in LDAP, and I\u0027m removing the logic for that. So this part of the test is unnecessary. https://github.com/openstack/keystone/blob/master/keystone/identity/backends/ldap/common.py#L1615","commit_id":"41b0d28bc33256f5fae75f73905eedf08cc1d760"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"72d29838c4bcbd93828808523f122f8682d81c68","unresolved":false,"context_lines":[{"line_number":761,"context_line":"            self.assertEqual(0, len(group_refs))"},{"line_number":762,"context_line":""},{"line_number":763,"context_line":"        # remove the user from each group and ensure that"},{"line_number":764,"context_line":"        # the group count reduces by one for each"},{"line_number":765,"context_line":"        for x in range(0, 3):"},{"line_number":766,"context_line":"            before_count \u003d GROUP_COUNT - x"},{"line_number":767,"context_line":"            after_count \u003d GROUP_COUNT - x - 1"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a30ddce_647915c8","side":"PARENT","line":764,"in_reply_to":"9a30ddce_e327e0c7","updated":"2017-03-10 14:36:27.000000000","message":"What about for sql backends? Isn\u0027t this test inherited by both? I see Steve\u0027s comment that it\u0027s safe to remove but I\u0027m not clear on why.","commit_id":"41b0d28bc33256f5fae75f73905eedf08cc1d760"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"6b7193bff4105aa4914928045cd5818e35766b34","unresolved":false,"context_lines":[{"line_number":761,"context_line":"            self.assertEqual(0, len(group_refs))"},{"line_number":762,"context_line":""},{"line_number":763,"context_line":"        # remove the user from each group and ensure that"},{"line_number":764,"context_line":"        # the group count reduces by one for each"},{"line_number":765,"context_line":"        for x in range(0, 3):"},{"line_number":766,"context_line":"            before_count \u003d GROUP_COUNT - x"},{"line_number":767,"context_line":"            after_count \u003d GROUP_COUNT - x - 1"}],"source_content_type":"text/x-python","patch_set":9,"id":"bff0334d_19343449","side":"PARENT","line":764,"updated":"2017-04-11 19:44:25.000000000","message":"This bit could be replaced with an assertion that a 403 Forbidden is raised when trying to remove a given user.","commit_id":"c45912533620b7dc2d6bd2dac293b7dcb39759ff"},{"author":{"_account_id":16465,"name":"Kristi Nikolla","email":"knikolla@bu.edu","username":"knikolla"},"change_message_id":"b1cac487b2dd59521a1b6fe6224c46fbf2abf086","unresolved":false,"context_lines":[{"line_number":761,"context_line":"            self.assertEqual(0, len(group_refs))"},{"line_number":762,"context_line":""},{"line_number":763,"context_line":"        # remove the user from each group and ensure that"},{"line_number":764,"context_line":"        # the group count reduces by one for each"},{"line_number":765,"context_line":"        for x in range(0, 3):"},{"line_number":766,"context_line":"            before_count \u003d GROUP_COUNT - x"},{"line_number":767,"context_line":"            after_count \u003d GROUP_COUNT - x - 1"}],"source_content_type":"text/x-python","patch_set":9,"id":"7ffa3b31_83fe00ac","side":"PARENT","line":764,"in_reply_to":"bff0334d_14dcf943","updated":"2017-04-18 18:13:26.000000000","message":"Actually this cannot be done here since this test is shared by all backends, not just ldap.","commit_id":"c45912533620b7dc2d6bd2dac293b7dcb39759ff"},{"author":{"_account_id":16465,"name":"Kristi Nikolla","email":"knikolla@bu.edu","username":"knikolla"},"change_message_id":"9e7392b301958f0cb59bdbc9b24fa560e16f476b","unresolved":false,"context_lines":[{"line_number":761,"context_line":"            self.assertEqual(0, len(group_refs))"},{"line_number":762,"context_line":""},{"line_number":763,"context_line":"        # remove the user from each group and ensure that"},{"line_number":764,"context_line":"        # the group count reduces by one for each"},{"line_number":765,"context_line":"        for x in range(0, 3):"},{"line_number":766,"context_line":"            before_count \u003d GROUP_COUNT - x"},{"line_number":767,"context_line":"            after_count \u003d GROUP_COUNT - x - 1"}],"source_content_type":"text/x-python","patch_set":9,"id":"bff0334d_14dcf943","side":"PARENT","line":764,"in_reply_to":"bff0334d_19343449","updated":"2017-04-11 20:09:18.000000000","message":"Will do in the next patchset.","commit_id":"c45912533620b7dc2d6bd2dac293b7dcb39759ff"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"e61e3bf5b3cd7a8037db5e5a709cfc75f80633f4","unresolved":false,"context_lines":[{"line_number":779,"context_line":"            group_refs \u003d self.identity_api.list_groups_for_user("},{"line_number":780,"context_line":"                negative_user[\u0027id\u0027])"},{"line_number":781,"context_line":"            self.assertEqual(0, len(group_refs))"},{"line_number":782,"context_line":""},{"line_number":783,"context_line":"    def test_remove_user_from_group(self):"},{"line_number":784,"context_line":"        domain \u003d self._get_domain_fixture()"},{"line_number":785,"context_line":"        new_group \u003d unit.new_group_ref(domain_id\u003ddomain[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":10,"id":"7ffa3b31_6964f522","side":"PARENT","line":782,"updated":"2017-04-18 20:11:15.000000000","message":"Could this get moved over to keystone.tests.unit.test_backends_sql?","commit_id":"c45912533620b7dc2d6bd2dac293b7dcb39759ff"}],"keystone/tests/unit/test_backend_ldap.py":[{"author":{"_account_id":21420,"name":"Gage Hugo","email":"gagehugo@gmail.com","username":"ghugo"},"change_message_id":"4b602fda150fbac6a8ec626a3a4e8f83d7ed82ea","unresolved":false,"context_lines":[{"line_number":390,"context_line":"        self.assertTrue(user_api)"},{"line_number":391,"context_line":"        self.assertEqual(\"ou\u003dUsers,%s\" % CONF.ldap.suffix, user_api.tree_dn)"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"    def test_configurable_allowed_user_actions(self):"},{"line_number":394,"context_line":"        user \u003d self.new_user_ref(domain_id\u003dCONF.identity.default_domain_id)"},{"line_number":395,"context_line":"        user \u003d self.identity_api.create_user(user)"},{"line_number":396,"context_line":"        self.identity_api.get_user(user[\u0027id\u0027])"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"        user[\u0027password\u0027] \u003d u\u0027fäképass2\u0027"},{"line_number":399,"context_line":"        self.identity_api.update_user(user[\u0027id\u0027], user)"},{"line_number":400,"context_line":""},{"line_number":401,"context_line":"    def test_user_filter(self):"},{"line_number":402,"context_line":"        user_ref \u003d self.identity_api.get_user(self.user_foo[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"da36d5c6_e3d6e4ef","line":399,"range":{"start_line":393,"start_character":0,"end_line":399,"end_character":55},"updated":"2017-02-23 19:57:39.000000000","message":"Would we want to just get rid of this test since it doesn\u0027t assert anything now?","commit_id":"eeb9c3ab20a34f417e94ba25a52699620936191c"},{"author":{"_account_id":16465,"name":"Kristi Nikolla","email":"knikolla@bu.edu","username":"knikolla"},"change_message_id":"1d694f07342f9393bfb50ea9a4882b1401fce499","unresolved":false,"context_lines":[{"line_number":390,"context_line":"        self.assertTrue(user_api)"},{"line_number":391,"context_line":"        self.assertEqual(\"ou\u003dUsers,%s\" % CONF.ldap.suffix, user_api.tree_dn)"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"    def test_configurable_allowed_user_actions(self):"},{"line_number":394,"context_line":"        user \u003d self.new_user_ref(domain_id\u003dCONF.identity.default_domain_id)"},{"line_number":395,"context_line":"        user \u003d self.identity_api.create_user(user)"},{"line_number":396,"context_line":"        self.identity_api.get_user(user[\u0027id\u0027])"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"        user[\u0027password\u0027] \u003d u\u0027fäképass2\u0027"},{"line_number":399,"context_line":"        self.identity_api.update_user(user[\u0027id\u0027], user)"},{"line_number":400,"context_line":""},{"line_number":401,"context_line":"    def test_user_filter(self):"},{"line_number":402,"context_line":"        user_ref \u003d self.identity_api.get_user(self.user_foo[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"da36d5c6_ec67b1b4","line":399,"range":{"start_line":393,"start_character":0,"end_line":399,"end_character":55},"in_reply_to":"da36d5c6_e3d6e4ef","updated":"2017-02-23 20:50:22.000000000","message":"Not sure. It still sort of does something and I\u0027m trying to restrict this review to only removing \u0027delete\u0027 bits.","commit_id":"eeb9c3ab20a34f417e94ba25a52699620936191c"},{"author":{"_account_id":16465,"name":"Kristi Nikolla","email":"knikolla@bu.edu","username":"knikolla"},"change_message_id":"fd29f490fe5c5127650a2990a4ecbe5dc79da466","unresolved":false,"context_lines":[{"line_number":3168,"context_line":"        self.assertDictEqual(self.group, group_ref)"},{"line_number":3169,"context_line":""},{"line_number":3170,"context_line":"    def test_list_user_groups(self):"},{"line_number":3171,"context_line":"        # tests indirectly by calling delete user"},{"line_number":3172,"context_line":"        self.identity_api.delete_user(self.user[\u0027id\u0027])"},{"line_number":3173,"context_line":""},{"line_number":3174,"context_line":"    def test_list_groups_for_user(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"da36d5c6_2c80e9bf","side":"PARENT","line":3171,"updated":"2017-02-23 20:55:27.000000000","message":"This is the only difference between patchset 4 and 5, but since I rebased diff between 4 and 5 got ugly.","commit_id":"1d4c72cd6fdad2c75e6b16ecf089852c604b8cd5"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"f61bd03558cc1222cc085b7675fd3b89a1109f86","unresolved":false,"context_lines":[{"line_number":368,"context_line":"        user[\u0027password\u0027] \u003d u\u0027fäképass2\u0027"},{"line_number":369,"context_line":"        self.identity_api.update_user(user[\u0027id\u0027], user)"},{"line_number":370,"context_line":""},{"line_number":371,"context_line":"        self.identity_api.delete_user(user[\u0027id\u0027])"},{"line_number":372,"context_line":"        self.assertRaises(exception.UserNotFound,"},{"line_number":373,"context_line":"                          self.identity_api.get_user,"},{"line_number":374,"context_line":"                          user[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a30ddce_058fa175","side":"PARENT","line":371,"updated":"2017-03-09 17:31:17.000000000","message":"Instead of deleting this assertion - we should update it to expect a read-only warning, which is more descriptive of the ldap behavior we\u0027re expecting.","commit_id":"41b0d28bc33256f5fae75f73905eedf08cc1d760"},{"author":{"_account_id":21420,"name":"Gage Hugo","email":"gagehugo@gmail.com","username":"ghugo"},"change_message_id":"96de2f0c1c630762b78b4712ec9d93050525cf28","unresolved":false,"context_lines":[{"line_number":368,"context_line":"        user[\u0027password\u0027] \u003d u\u0027fäképass2\u0027"},{"line_number":369,"context_line":"        self.identity_api.update_user(user[\u0027id\u0027], user)"},{"line_number":370,"context_line":""},{"line_number":371,"context_line":"        self.identity_api.delete_user(user[\u0027id\u0027])"},{"line_number":372,"context_line":"        self.assertRaises(exception.UserNotFound,"},{"line_number":373,"context_line":"                          self.identity_api.get_user,"},{"line_number":374,"context_line":"                          user[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a30ddce_ecfb55bf","side":"PARENT","line":371,"in_reply_to":"9a30ddce_058fa175","updated":"2017-03-09 18:04:21.000000000","message":"++, otherwise this test doesn\u0027t really assert anything","commit_id":"41b0d28bc33256f5fae75f73905eedf08cc1d760"},{"author":{"_account_id":16465,"name":"Kristi Nikolla","email":"knikolla@bu.edu","username":"knikolla"},"change_message_id":"ae11bdea33b24ec11fb498a999926d06caedbb90","unresolved":false,"context_lines":[{"line_number":368,"context_line":"        user[\u0027password\u0027] \u003d u\u0027fäképass2\u0027"},{"line_number":369,"context_line":"        self.identity_api.update_user(user[\u0027id\u0027], user)"},{"line_number":370,"context_line":""},{"line_number":371,"context_line":"        self.identity_api.delete_user(user[\u0027id\u0027])"},{"line_number":372,"context_line":"        self.assertRaises(exception.UserNotFound,"},{"line_number":373,"context_line":"                          self.identity_api.get_user,"},{"line_number":374,"context_line":"                          user[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a30ddce_7e962340","side":"PARENT","line":371,"in_reply_to":"9a30ddce_058fa175","updated":"2017-03-09 20:07:54.000000000","message":"Is it okay if I do this more nicely in a follow-up patch? Because in a lot of the tests here I have removed the delete part completely instead of asserting for that exception. I would prefer to be consistent and add a few tests specifically for this kind of forbidden behavior.","commit_id":"41b0d28bc33256f5fae75f73905eedf08cc1d760"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"6b7193bff4105aa4914928045cd5818e35766b34","unresolved":false,"context_lines":[{"line_number":692,"context_line":"        # updating the group."},{"line_number":693,"context_line":"        unused, driver, entity_id \u003d ("},{"line_number":694,"context_line":"            self.identity_api._get_domain_driver_and_entity_id(user_2_id))"},{"line_number":695,"context_line":"        driver.user.delete(entity_id)"},{"line_number":696,"context_line":""},{"line_number":697,"context_line":"        # List group users and verify only user 1."},{"line_number":698,"context_line":"        res \u003d self.identity_api.list_users_in_group(group_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"bff0334d_b93c282f","side":"PARENT","line":695,"updated":"2017-04-11 19:44:25.000000000","message":"Is this whole test being removed because this bit is no longer relevant?","commit_id":"c45912533620b7dc2d6bd2dac293b7dcb39759ff"},{"author":{"_account_id":16465,"name":"Kristi Nikolla","email":"knikolla@bu.edu","username":"knikolla"},"change_message_id":"9e7392b301958f0cb59bdbc9b24fa560e16f476b","unresolved":false,"context_lines":[{"line_number":692,"context_line":"        # updating the group."},{"line_number":693,"context_line":"        unused, driver, entity_id \u003d ("},{"line_number":694,"context_line":"            self.identity_api._get_domain_driver_and_entity_id(user_2_id))"},{"line_number":695,"context_line":"        driver.user.delete(entity_id)"},{"line_number":696,"context_line":""},{"line_number":697,"context_line":"        # List group users and verify only user 1."},{"line_number":698,"context_line":"        res \u003d self.identity_api.list_users_in_group(group_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"bff0334d_543161a2","side":"PARENT","line":695,"in_reply_to":"bff0334d_b93c282f","updated":"2017-04-11 20:09:18.000000000","message":"Yes. The tests purpose is checking that bypassing the user interface and using directly the driver to delete a user results in the user not showing up in the group list anymore. We already have tests for creating a group and adding it to a user, so this test would basically be a duplicate of those without the delete bit.","commit_id":"c45912533620b7dc2d6bd2dac293b7dcb39759ff"}]}