)]}'
{"keystone/credential/backends/sql.py":[{"author":{"_account_id":2903,"name":"Morgan Fainberg","email":"morgan.fainberg@gmail.com","username":"mdrnstm"},"change_message_id":"032e95c38c9422dbd0e0c00c232cb29ed2bfcb03","unresolved":false,"context_lines":[{"line_number":92,"context_line":"            ref \u003d self._get_credential(session, credential_id)"},{"line_number":93,"context_line":"            session.delete(ref)"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    def delete_credentials_for_project(self, project_id):"},{"line_number":96,"context_line":"        with sql.session_for_write() as session:"},{"line_number":97,"context_line":"            query \u003d session.query(CredentialModel)"},{"line_number":98,"context_line":"            query \u003d query.filter_by(project_id\u003dproject_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_dee597a5","line":95,"range":{"start_line":95,"start_character":3,"end_line":95,"end_character":38},"updated":"2019-08-15 17:06:27.000000000","message":"Is this needed here as well and/or anywhere else?\n\nWhat is the context where this is needed, e.g. how widespread is the need to add this decorator.\n\nThanks for the fix and once you answer this it\u0027ll be easy to score/review the fix.\n\nThis also should have a release note added to it.","commit_id":"478f0a08a9069d6d64dd21aa06eefa60d2b9a7d4"},{"author":{"_account_id":8833,"name":"Rabi Mishra","email":"ramishra@redhat.com","username":"rabi"},"change_message_id":"95fa9c186ce1a14b5b4f7cd4cfb68334c21b5c51","unresolved":false,"context_lines":[{"line_number":92,"context_line":"            ref \u003d self._get_credential(session, credential_id)"},{"line_number":93,"context_line":"            session.delete(ref)"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    def delete_credentials_for_project(self, project_id):"},{"line_number":96,"context_line":"        with sql.session_for_write() as session:"},{"line_number":97,"context_line":"            query \u003d session.query(CredentialModel)"},{"line_number":98,"context_line":"            query \u003d query.filter_by(project_id\u003dproject_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_3a7950fb","line":95,"range":{"start_line":95,"start_character":3,"end_line":95,"end_character":38},"in_reply_to":"7faddb67_6452cd15","updated":"2019-08-16 00:48:45.000000000","message":"\u003e Yes, we try to have a release note for each bug fixed\n\nSorry, that does not seem true looking at some recent bug fix patches i.e https://review.opendev.org/#/c/674122/\n\nThe example you\u0027ve given has at least a change to the api behavior. This fix is neither a feature nor has any user visible impact (user won\u0027t know about it).\n\nI\u0027ve been working on OpenStack for last 7 years and have never seen someone -1 a patch for not having a release note for a bug fix that has no user visible impact. I hope the keystone team follows https://docs.openstack.org/project-team-guide/review-the-openstack-way.html\n\n\n\u003e  but is this needed in more places, look at how deletion is done for projects, could you end up with a deadlock here as well as for users.\n\nI don\u0027t know if this is needed at other places and have no way to know that without seeing an issue. In this case, I\u0027ve seen an issue, reported and fixed it.\n\n\u003e Trying to avoid having the same bug over and over and over again.\n\nThat way you would end up adding retry for every db operation (expecting a potential issue) and that wont be good thing to do.","commit_id":"478f0a08a9069d6d64dd21aa06eefa60d2b9a7d4"},{"author":{"_account_id":2903,"name":"Morgan Fainberg","email":"morgan.fainberg@gmail.com","username":"mdrnstm"},"change_message_id":"96f1cfbdca62d6eb2b5e04e58f6a3a50f0f983fb","unresolved":false,"context_lines":[{"line_number":92,"context_line":"            ref \u003d self._get_credential(session, credential_id)"},{"line_number":93,"context_line":"            session.delete(ref)"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    def delete_credentials_for_project(self, project_id):"},{"line_number":96,"context_line":"        with sql.session_for_write() as session:"},{"line_number":97,"context_line":"            query \u003d session.query(CredentialModel)"},{"line_number":98,"context_line":"            query \u003d query.filter_by(project_id\u003dproject_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_6452cd15","line":95,"range":{"start_line":95,"start_character":3,"end_line":95,"end_character":38},"in_reply_to":"7faddb67_993bb9cf","updated":"2019-08-15 21:06:07.000000000","message":"Yes, we try to have a release note for each bug fixed. Example from a past bug: https://github.com/openstack/keystone/blob/master/releasenotes/notes/bug-1594482-52a5dd1d8477b694.yaml\n\nThe bug does show context, but is this needed in more places, look at how deletion is done for projects, could you end up with a deadlock here as well as for users. Is this a wide-spread issue or very isolated to the user case?\n\nTrying to avoid having the same bug over and over and over again.","commit_id":"478f0a08a9069d6d64dd21aa06eefa60d2b9a7d4"},{"author":{"_account_id":8833,"name":"Rabi Mishra","email":"ramishra@redhat.com","username":"rabi"},"change_message_id":"c0b62592238745b2e1df47377099830618042a05","unresolved":false,"context_lines":[{"line_number":92,"context_line":"            ref \u003d self._get_credential(session, credential_id)"},{"line_number":93,"context_line":"            session.delete(ref)"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    def delete_credentials_for_project(self, project_id):"},{"line_number":96,"context_line":"        with sql.session_for_write() as session:"},{"line_number":97,"context_line":"            query \u003d session.query(CredentialModel)"},{"line_number":98,"context_line":"            query \u003d query.filter_by(project_id\u003dproject_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_993bb9cf","line":95,"range":{"start_line":95,"start_character":3,"end_line":95,"end_character":38},"in_reply_to":"7faddb67_dee597a5","updated":"2019-08-15 17:15:32.000000000","message":"\u003e What is the context where this is needed\n\nHmm.. Doesn\u0027t the bug provide the context? I\u0027ve seen this when deleting a bunch of domain users cocurrently from heat when deleting resources in a large stack. Traceback is there in the bug.\n\n\u003e This also should have a release note added to it.\n\nI think this fixes a bug. Do you always add releasenotes for bug fixes?","commit_id":"478f0a08a9069d6d64dd21aa06eefa60d2b9a7d4"}]}
