)]}'
{"keystone/resource/core.py":[{"author":{"_account_id":5707,"name":"Henry Nash","email":"henryn@linux.vnet.ibm.com","username":"henry-nash"},"change_message_id":"b93244652fa331a8eed8562b40eaef9ac3e7b37e","unresolved":false,"context_lines":[{"line_number":428,"context_line":"        return ret"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"    def _pre_delete_cleanup_project(self, project_id):"},{"line_number":431,"context_line":"        PROVIDERS.unified_limit_api.delete_limits_for_project(project_id)"},{"line_number":432,"context_line":"        project_user_ids \u003d ("},{"line_number":433,"context_line":"            PROVIDERS.assignment_api.list_user_ids_for_project(project_id))"},{"line_number":434,"context_line":"        for user_id in project_user_ids:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f93b717_10cb231e","line":431,"updated":"2018-01-29 18:42:58.000000000","message":"should this be in the pre or post cleanup? Can the delete project fail after the pre, resulting in the limits being removed but the project remaining?","commit_id":"8298e0c3c5d17a67a448de58d0f91bdad3976014"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"7957497734183b8979f482538b47775ad4c903db","unresolved":false,"context_lines":[{"line_number":428,"context_line":"        return ret"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"    def _pre_delete_cleanup_project(self, project_id):"},{"line_number":431,"context_line":"        PROVIDERS.unified_limit_api.delete_limits_for_project(project_id)"},{"line_number":432,"context_line":"        project_user_ids \u003d ("},{"line_number":433,"context_line":"            PROVIDERS.assignment_api.list_user_ids_for_project(project_id))"},{"line_number":434,"context_line":"        for user_id in project_user_ids:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f93b717_d69ccac1","line":431,"in_reply_to":"5f93b717_10cb231e","updated":"2018-01-30 03:59:51.000000000","message":"project limits table has a FK on project_id, so it can\u0027t be deleted in post cleanup.\nBut your concern is a problem indeed. How about remove the FK since we already have a check here: https://github.com/openstack/keystone/blob/master/keystone/limit/core.py#L47","commit_id":"8298e0c3c5d17a67a448de58d0f91bdad3976014"},{"author":{"_account_id":5707,"name":"Henry Nash","email":"henryn@linux.vnet.ibm.com","username":"henry-nash"},"change_message_id":"464d7bad2423f29693dbffd8d80b954aaf184553","unresolved":false,"context_lines":[{"line_number":428,"context_line":"        return ret"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"    def _pre_delete_cleanup_project(self, project_id):"},{"line_number":431,"context_line":"        PROVIDERS.unified_limit_api.delete_limits_for_project(project_id)"},{"line_number":432,"context_line":"        project_user_ids \u003d ("},{"line_number":433,"context_line":"            PROVIDERS.assignment_api.list_user_ids_for_project(project_id))"},{"line_number":434,"context_line":"        for user_id in project_user_ids:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f93b717_5fe07826","line":431,"in_reply_to":"5f93b717_5a4739bd","updated":"2018-02-01 17:27:33.000000000","message":"So in cases when you can\u0027t us FK cascading (and even in the all-SQL case, are we sure that the cascade support across all flavours of DB we might support is sufficient?), then perhaps an approach is something like:\n1) Check permission to delete\n2) If OK, mark the entity as deleted in some atomic way (e.g. a \"permenantly-diabled\" flag\". This flag would have to be checked wherever we might use this entity (e.g. looking up a user would fail if the domain/project was \"permanently disabled\")\n3) Then the actual delete becomes a cleanup task, we try and do it at the point of request, but if it screws up, we have some background cleanup process that keeps trying, alerts operator to manual intervention if required etc.","commit_id":"8298e0c3c5d17a67a448de58d0f91bdad3976014"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"1aed0be21af7a8584d144cfd046925b3d0ab0b19","unresolved":false,"context_lines":[{"line_number":428,"context_line":"        return ret"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"    def _pre_delete_cleanup_project(self, project_id):"},{"line_number":431,"context_line":"        PROVIDERS.unified_limit_api.delete_limits_for_project(project_id)"},{"line_number":432,"context_line":"        project_user_ids \u003d ("},{"line_number":433,"context_line":"            PROVIDERS.assignment_api.list_user_ids_for_project(project_id))"},{"line_number":434,"context_line":"        for user_id in project_user_ids:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f93b717_128bbe67","line":431,"in_reply_to":"5f93b717_5fe07826","updated":"2018-02-02 01:53:37.000000000","message":"Cool. Seems Colleen\u0027s new patch that using the soft-delete way may useful. I\u0027ll update this patch one hers down. Thanks for your suggestion.","commit_id":"8298e0c3c5d17a67a448de58d0f91bdad3976014"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"b0ea9ab0266bd2135698fbb500dd340194c25358","unresolved":false,"context_lines":[{"line_number":428,"context_line":"        return ret"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"    def _pre_delete_cleanup_project(self, project_id):"},{"line_number":431,"context_line":"        PROVIDERS.unified_limit_api.delete_limits_for_project(project_id)"},{"line_number":432,"context_line":"        project_user_ids \u003d ("},{"line_number":433,"context_line":"            PROVIDERS.assignment_api.list_user_ids_for_project(project_id))"},{"line_number":434,"context_line":"        for user_id in project_user_ids:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f93b717_5a4739bd","line":431,"in_reply_to":"5f93b717_d69ccac1","updated":"2018-02-01 06:26:15.000000000","message":"A similiar problem raised here: https://review.openstack.org/#/c/539347/1/keystone/resource/core.py\nSeems that to solve this problem we can either remove the FK or add cascade delete action in db.\nBut both are seems not very good way. any suggestion?","commit_id":"8298e0c3c5d17a67a448de58d0f91bdad3976014"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"1b99eb9859790dbe622d1cede35435639817386f","unresolved":false,"context_lines":[{"line_number":498,"context_line":"            for prj in project_list:"},{"line_number":499,"context_line":"                self._post_delete_cleanup_project(prj[\u0027id\u0027], prj, initiator)"},{"line_number":500,"context_line":"        else:"},{"line_number":501,"context_line":"            # FixMe(wxy): If the delete project fail after the pre limits"},{"line_number":502,"context_line":"            # delete, it\u0027ll result in the limits being removed but the project"},{"line_number":503,"context_line":"            # remaining. It\u0027s not easy to solve it with a small change. Should"},{"line_number":504,"context_line":"            # find a way in the future."}],"source_content_type":"text/x-python","patch_set":4,"id":"bf659307_6a72fc7e","line":501,"range":{"start_line":501,"start_character":14,"end_line":501,"end_character":19},"updated":"2018-04-06 12:09:03.000000000","message":"nit - this should be all caps, like FIXME","commit_id":"2b74389edf631157ccd15992fecf3eb19605e16b"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"fdfc93c0774d3e1a4bc847b3377d943a1acd87e6","unresolved":false,"context_lines":[{"line_number":498,"context_line":"            for prj in project_list:"},{"line_number":499,"context_line":"                self._post_delete_cleanup_project(prj[\u0027id\u0027], prj, initiator)"},{"line_number":500,"context_line":"        else:"},{"line_number":501,"context_line":"            # FixMe(wxy): If the delete project fail after the pre limits"},{"line_number":502,"context_line":"            # delete, it\u0027ll result in the limits being removed but the project"},{"line_number":503,"context_line":"            # remaining. It\u0027s not easy to solve it with a small change. Should"},{"line_number":504,"context_line":"            # find a way in the future."}],"source_content_type":"text/x-python","patch_set":4,"id":"bf659307_439270a1","line":501,"range":{"start_line":501,"start_character":14,"end_line":501,"end_character":19},"in_reply_to":"bf659307_6a72fc7e","updated":"2018-04-08 06:31:29.000000000","message":"Done","commit_id":"2b74389edf631157ccd15992fecf3eb19605e16b"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"7b62d051d5aa052cc39130f9e411e1e2cc0faccc","unresolved":false,"context_lines":[{"line_number":501,"context_line":"            # FIXME(wxy): If the delete project fail after the pre limits"},{"line_number":502,"context_line":"            # delete, it\u0027ll result in the limits being removed but the project"},{"line_number":503,"context_line":"            # remaining. It\u0027s not easy to solve it with a small change. Should"},{"line_number":504,"context_line":"            # find a way in the future."},{"line_number":505,"context_line":"            PROVIDERS.unified_limit_api.delete_limits_for_project(project_id)"},{"line_number":506,"context_line":"            ret \u003d self.driver.delete_project(project_id)"},{"line_number":507,"context_line":"            self._post_delete_cleanup_project(project_id, project, initiator)"}],"source_content_type":"text/x-python","patch_set":5,"id":"bf659307_c3f2e39a","line":504,"updated":"2018-04-09 19:53:16.000000000","message":"One option would be to create a FK relationship between the limit and the project, but that\u0027s always been problematic with pluggable backends. Is there any reason why this can\u0027t be done after the project is successfully deleted on line 506?","commit_id":"326acda8782594bddc59e031a7fd5da0790dc83d"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"fa75d90ea078212098d5e7acadc563d340735b07","unresolved":false,"context_lines":[{"line_number":501,"context_line":"            # FIXME(wxy): If the delete project fail after the pre limits"},{"line_number":502,"context_line":"            # delete, it\u0027ll result in the limits being removed but the project"},{"line_number":503,"context_line":"            # remaining. It\u0027s not easy to solve it with a small change. Should"},{"line_number":504,"context_line":"            # find a way in the future."},{"line_number":505,"context_line":"            PROVIDERS.unified_limit_api.delete_limits_for_project(project_id)"},{"line_number":506,"context_line":"            ret \u003d self.driver.delete_project(project_id)"},{"line_number":507,"context_line":"            self._post_delete_cleanup_project(project_id, project, initiator)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f7c97a3_c999bcd0","line":504,"in_reply_to":"9f6a8fd7_58317459","updated":"2018-06-06 15:13:41.000000000","message":"So would you be inclined to do option #1 then?","commit_id":"326acda8782594bddc59e031a7fd5da0790dc83d"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"70f49c7892b96a47debb5d70b60a09fe025ddb7a","unresolved":false,"context_lines":[{"line_number":501,"context_line":"            # FIXME(wxy): If the delete project fail after the pre limits"},{"line_number":502,"context_line":"            # delete, it\u0027ll result in the limits being removed but the project"},{"line_number":503,"context_line":"            # remaining. It\u0027s not easy to solve it with a small change. Should"},{"line_number":504,"context_line":"            # find a way in the future."},{"line_number":505,"context_line":"            PROVIDERS.unified_limit_api.delete_limits_for_project(project_id)"},{"line_number":506,"context_line":"            ret \u003d self.driver.delete_project(project_id)"},{"line_number":507,"context_line":"            self._post_delete_cleanup_project(project_id, project, initiator)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f6a8fd7_58317459","line":504,"in_reply_to":"bf659307_2991249a","updated":"2018-04-24 08:47:20.000000000","message":"If use CASCADE, we\u0027ll add a new db migration script for it. I\u0027m not sure whether it\u0027s worth to do that base on the case that limit and project are in different resource backend.","commit_id":"326acda8782594bddc59e031a7fd5da0790dc83d"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"b88886b7109220217cce47685fee4c4ed031b6f9","unresolved":false,"context_lines":[{"line_number":501,"context_line":"            # FIXME(wxy): If the delete project fail after the pre limits"},{"line_number":502,"context_line":"            # delete, it\u0027ll result in the limits being removed but the project"},{"line_number":503,"context_line":"            # remaining. It\u0027s not easy to solve it with a small change. Should"},{"line_number":504,"context_line":"            # find a way in the future."},{"line_number":505,"context_line":"            PROVIDERS.unified_limit_api.delete_limits_for_project(project_id)"},{"line_number":506,"context_line":"            ret \u003d self.driver.delete_project(project_id)"},{"line_number":507,"context_line":"            self._post_delete_cleanup_project(project_id, project, initiator)"}],"source_content_type":"text/x-python","patch_set":5,"id":"bf659307_2991249a","line":504,"in_reply_to":"bf659307_3070bcdc","updated":"2018-04-10 19:04:26.000000000","message":"I wouldn\u0027t be opposed to just pushing this into the database with CASCADE. Especially since we only support SQL for both the limit backend and resource backend.","commit_id":"326acda8782594bddc59e031a7fd5da0790dc83d"},{"author":{"_account_id":15054,"name":"wangxiyuan","email":"wangxiyuan1007@gmail.com","username":"wangxiyuan"},"change_message_id":"f3ef936875e29567bc5eb876435c7bab5577e65a","unresolved":false,"context_lines":[{"line_number":501,"context_line":"            # FIXME(wxy): If the delete project fail after the pre limits"},{"line_number":502,"context_line":"            # delete, it\u0027ll result in the limits being removed but the project"},{"line_number":503,"context_line":"            # remaining. It\u0027s not easy to solve it with a small change. Should"},{"line_number":504,"context_line":"            # find a way in the future."},{"line_number":505,"context_line":"            PROVIDERS.unified_limit_api.delete_limits_for_project(project_id)"},{"line_number":506,"context_line":"            ret \u003d self.driver.delete_project(project_id)"},{"line_number":507,"context_line":"            self._post_delete_cleanup_project(project_id, project, initiator)"}],"source_content_type":"text/x-python","patch_set":5,"id":"bf659307_3070bcdc","line":504,"in_reply_to":"bf659307_c3f2e39a","updated":"2018-04-10 02:12:50.000000000","message":"There is a FK already:\nhttps://github.com/openstack/keystone/blob/master/keystone/limit/backends/sql.py#L65\nSo it\u0027s the reason why we should delete the limit first.\n\nAnother two ways to solve the problem are:\n1. remove the FK, then we can delete the limit after L506\n2. add cascade delete support in db.\n\nthe first one can be considers as well. Adding a FIXME here so that we can think more about the FK in Keystone next.","commit_id":"326acda8782594bddc59e031a7fd5da0790dc83d"}]}
