)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"ecbdd770a752047483cdc71234c43f9c8f818500","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Clean up exception handling for CADF reason"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This change cleans up the exception handling for adding a reason"},{"line_number":10,"context_line":"object to CADF notifications for multiple PCI-DSS events."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Iab49986ec41028da5c0d45f83c0037c07434c744"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba5201f7_e18b9f1c","line":10,"range":{"start_line":9,"start_character":0,"end_line":10,"end_character":57},"updated":"2017-01-03 22:12:10.000000000","message":"this isn\u0027t the case any longer, update the message for why we\u0027re removing things","commit_id":"12b4b0e5d96d2fd3fc695a51bfbdb8ecb43b041e"},{"author":{"_account_id":18338,"name":"Ron De Rose","email":"ronald.de.rose@intel.com","username":"derosenet"},"change_message_id":"779e6792dc9896516fa9882815146e006b5671d1","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Clean up exception handling for CADF reason"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This change cleans up the exception handling for adding a reason"},{"line_number":10,"context_line":"object to CADF notifications for multiple PCI-DSS events."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Iab49986ec41028da5c0d45f83c0037c07434c744"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba5201f7_f6e33f31","line":10,"range":{"start_line":9,"start_character":0,"end_line":10,"end_character":57},"in_reply_to":"ba5201f7_e18b9f1c","updated":"2017-01-04 16:20:17.000000000","message":"++","commit_id":"12b4b0e5d96d2fd3fc695a51bfbdb8ecb43b041e"},{"author":{"_account_id":21420,"name":"Gage Hugo","email":"gagehugo@gmail.com","username":"ghugo"},"change_message_id":"e05336273e69ef44fcbe7e0588ee96926174a9a6","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Clean up exception handling for CADF reason"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This change cleans up the exception handling for adding a reason"},{"line_number":10,"context_line":"object to CADF notifications for multiple PCI-DSS events."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Iab49986ec41028da5c0d45f83c0037c07434c744"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba5201f7_d1dc8514","line":10,"range":{"start_line":9,"start_character":0,"end_line":10,"end_character":57},"in_reply_to":"ba5201f7_f6e33f31","updated":"2017-01-04 16:44:16.000000000","message":"Done","commit_id":"12b4b0e5d96d2fd3fc695a51bfbdb8ecb43b041e"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"bcb7c2e3376da3382809ec8a0591812af8438d72","unresolved":false,"context_lines":[{"line_number":10,"context_line":"authentication due to them being related to password validation"},{"line_number":11,"context_line":"and are caught/handled elsewhere from this change:"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"https://review.openstack.org/#/c/400882/"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Iab49986ec41028da5c0d45f83c0037c07434c744"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"ba5201f7_e0d9b223","line":13,"range":{"start_line":13,"start_character":0,"end_line":13,"end_character":40},"updated":"2017-01-04 18:08:13.000000000","message":"this change: https://review.openstack.org/#/c/396752/","commit_id":"679e12ac8d2fad63022ab08c096c103a69c86036"}],"keystone/notifications.py":[{"author":{"_account_id":21420,"name":"Gage Hugo","email":"gagehugo@gmail.com","username":"ghugo"},"change_message_id":"3b190d99046e1d1df0a7e33423eafd5a995bd2c3","unresolved":false,"context_lines":[{"line_number":493,"context_line":"                result \u003d f(wrapped_self, request, user_id, *args, **kwargs)"},{"line_number":494,"context_line":"            except (exception.AccountLocked,"},{"line_number":495,"context_line":"                    exception.PasswordExpired,"},{"line_number":496,"context_line":"                    exception.PasswordRequirementsValidationError,"},{"line_number":497,"context_line":"                    exception.PasswordHistoryValidationError,"},{"line_number":498,"context_line":"                    exception.PasswordAgeValidationError) as ex:"},{"line_number":499,"context_line":"                # Send a CADF event with a reason for PCI-DSS related"},{"line_number":500,"context_line":"                # authentication failures"},{"line_number":501,"context_line":"                audit_reason \u003d reason.Reason(str(ex), str(ex.code))"}],"source_content_type":"text/x-python","patch_set":1,"id":"da4df55a_909c2d6b","side":"PARENT","line":498,"range":{"start_line":496,"start_character":0,"end_line":498,"end_character":56},"updated":"2016-12-22 00:46:47.000000000","message":"These 3 exceptions are actually caught here:\nhttps://github.com/openstack/keystone/blob/master/keystone/identity/core.py#L1296\n\nThat is why I removed them from being caught here.","commit_id":"a5efb16256c848d998b1d119c533eeffda1ad481"},{"author":{"_account_id":21420,"name":"Gage Hugo","email":"gagehugo@gmail.com","username":"ghugo"},"change_message_id":"9a2c40a7fa2f1bbf9e49c4b028c4954df8b6187c","unresolved":false,"context_lines":[{"line_number":493,"context_line":"                result \u003d f(wrapped_self, request, user_id, *args, **kwargs)"},{"line_number":494,"context_line":"            except (exception.AccountLocked,"},{"line_number":495,"context_line":"                    exception.PasswordExpired,"},{"line_number":496,"context_line":"                    exception.PasswordRequirementsValidationError,"},{"line_number":497,"context_line":"                    exception.PasswordHistoryValidationError,"},{"line_number":498,"context_line":"                    exception.PasswordAgeValidationError) as ex:"},{"line_number":499,"context_line":"                # Send a CADF event with a reason for PCI-DSS related"},{"line_number":500,"context_line":"                # authentication failures"},{"line_number":501,"context_line":"                audit_reason \u003d reason.Reason(str(ex), str(ex.code))"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_2ed91984","side":"PARENT","line":498,"range":{"start_line":496,"start_character":0,"end_line":498,"end_character":56},"in_reply_to":"ba5201f7_cdbe1ed4","updated":"2017-01-03 19:56:16.000000000","message":"Ok","commit_id":"a5efb16256c848d998b1d119c533eeffda1ad481"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"b9e3edb3372eedbe54134085327e28ccd56ace81","unresolved":false,"context_lines":[{"line_number":493,"context_line":"                result \u003d f(wrapped_self, request, user_id, *args, **kwargs)"},{"line_number":494,"context_line":"            except (exception.AccountLocked,"},{"line_number":495,"context_line":"                    exception.PasswordExpired,"},{"line_number":496,"context_line":"                    exception.PasswordRequirementsValidationError,"},{"line_number":497,"context_line":"                    exception.PasswordHistoryValidationError,"},{"line_number":498,"context_line":"                    exception.PasswordAgeValidationError) as ex:"},{"line_number":499,"context_line":"                # Send a CADF event with a reason for PCI-DSS related"},{"line_number":500,"context_line":"                # authentication failures"},{"line_number":501,"context_line":"                audit_reason \u003d reason.Reason(str(ex), str(ex.code))"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_cdbe1ed4","side":"PARENT","line":498,"range":{"start_line":496,"start_character":0,"end_line":498,"end_character":56},"in_reply_to":"da4df55a_1fd34a05","updated":"2016-12-29 13:23:47.000000000","message":"++","commit_id":"a5efb16256c848d998b1d119c533eeffda1ad481"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"90dfce382090ae8a924815a38f4ba2ef02a5caf6","unresolved":false,"context_lines":[{"line_number":493,"context_line":"                result \u003d f(wrapped_self, request, user_id, *args, **kwargs)"},{"line_number":494,"context_line":"            except (exception.AccountLocked,"},{"line_number":495,"context_line":"                    exception.PasswordExpired,"},{"line_number":496,"context_line":"                    exception.PasswordRequirementsValidationError,"},{"line_number":497,"context_line":"                    exception.PasswordHistoryValidationError,"},{"line_number":498,"context_line":"                    exception.PasswordAgeValidationError) as ex:"},{"line_number":499,"context_line":"                # Send a CADF event with a reason for PCI-DSS related"},{"line_number":500,"context_line":"                # authentication failures"},{"line_number":501,"context_line":"                audit_reason \u003d reason.Reason(str(ex), str(ex.code))"}],"source_content_type":"text/x-python","patch_set":1,"id":"da4df55a_1fd34a05","side":"PARENT","line":498,"range":{"start_line":496,"start_character":0,"end_line":498,"end_character":56},"in_reply_to":"da4df55a_909c2d6b","updated":"2016-12-28 02:55:10.000000000","message":"just remove them instead?","commit_id":"a5efb16256c848d998b1d119c533eeffda1ad481"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"b9e3edb3372eedbe54134085327e28ccd56ace81","unresolved":false,"context_lines":[{"line_number":504,"context_line":"                                         target, self.event_type,"},{"line_number":505,"context_line":"                                         reason\u003daudit_reason)"},{"line_number":506,"context_line":"                raise"},{"line_number":507,"context_line":"            except Exception:"},{"line_number":508,"context_line":"                # For authentication failure send a CADF event as well"},{"line_number":509,"context_line":"                _send_audit_notification(self.action, request.audit_initiator,"},{"line_number":510,"context_line":"                                         taxonomy.OUTCOME_FAILURE,"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_4de4ee0c","side":"PARENT","line":507,"range":{"start_line":507,"start_character":18,"end_line":507,"end_character":28},"updated":"2016-12-29 13:23:47.000000000","message":"a possible refactoring is removing this. It is not needed.","commit_id":"a5efb16256c848d998b1d119c533eeffda1ad481"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"b9e3edb3372eedbe54134085327e28ccd56ace81","unresolved":false,"context_lines":[{"line_number":510,"context_line":"                                         taxonomy.OUTCOME_FAILURE,"},{"line_number":511,"context_line":"                                         target, self.event_type)"},{"line_number":512,"context_line":"                raise"},{"line_number":513,"context_line":"            else:"},{"line_number":514,"context_line":"                _send_audit_notification(self.action, request.audit_initiator,"},{"line_number":515,"context_line":"                                         taxonomy.OUTCOME_SUCCESS,"},{"line_number":516,"context_line":"                                         target, self.event_type)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_6dc45240","side":"PARENT","line":513,"range":{"start_line":513,"start_character":12,"end_line":513,"end_character":17},"updated":"2016-12-29 13:23:47.000000000","message":"another possible refactoring is to remove this \"else\" - don\u0027t think we want to send a notification for a notification error, right?","commit_id":"a5efb16256c848d998b1d119c533eeffda1ad481"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"e11f993b6e4c4f7d1981bd66aa8cc9e9e17a5014","unresolved":false,"context_lines":[{"line_number":510,"context_line":"                                         taxonomy.OUTCOME_FAILURE,"},{"line_number":511,"context_line":"                                         target, self.event_type)"},{"line_number":512,"context_line":"                raise"},{"line_number":513,"context_line":"            else:"},{"line_number":514,"context_line":"                _send_audit_notification(self.action, request.audit_initiator,"},{"line_number":515,"context_line":"                                         taxonomy.OUTCOME_SUCCESS,"},{"line_number":516,"context_line":"                                         target, self.event_type)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_af98a31c","side":"PARENT","line":513,"range":{"start_line":513,"start_character":12,"end_line":513,"end_character":17},"in_reply_to":"ba5201f7_0e4c5541","updated":"2017-01-03 21:49:54.000000000","message":"lol, I really think the \"else\" is rather unreadable. Think it is more a personal preference :)","commit_id":"a5efb16256c848d998b1d119c533eeffda1ad481"},{"author":{"_account_id":21420,"name":"Gage Hugo","email":"gagehugo@gmail.com","username":"ghugo"},"change_message_id":"9a2c40a7fa2f1bbf9e49c4b028c4954df8b6187c","unresolved":false,"context_lines":[{"line_number":510,"context_line":"                                         taxonomy.OUTCOME_FAILURE,"},{"line_number":511,"context_line":"                                         target, self.event_type)"},{"line_number":512,"context_line":"                raise"},{"line_number":513,"context_line":"            else:"},{"line_number":514,"context_line":"                _send_audit_notification(self.action, request.audit_initiator,"},{"line_number":515,"context_line":"                                         taxonomy.OUTCOME_SUCCESS,"},{"line_number":516,"context_line":"                                         target, self.event_type)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_0e4c5541","side":"PARENT","line":513,"range":{"start_line":513,"start_character":12,"end_line":513,"end_character":17},"in_reply_to":"ba5201f7_6dc45240","updated":"2017-01-03 19:56:16.000000000","message":"Yeah, we probably don\u0027t \"need\" the else here, it looks like it was added for more readability, but I\u0027m not sure if it needs to be removed.","commit_id":"a5efb16256c848d998b1d119c533eeffda1ad481"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"90dfce382090ae8a924815a38f4ba2ef02a5caf6","unresolved":false,"context_lines":[{"line_number":492,"context_line":"            try:"},{"line_number":493,"context_line":"                result \u003d f(wrapped_self, request, user_id, *args, **kwargs)"},{"line_number":494,"context_line":"            except Exception as ex:"},{"line_number":495,"context_line":"                # For authentication failure send a CADF event as well"},{"line_number":496,"context_line":"                # PCI-DSS events will have a reason associated with them"},{"line_number":497,"context_line":"                audit_reason \u003d self._get_audit_reason(ex)"},{"line_number":498,"context_line":"                _send_audit_notification(self.action, request.audit_initiator,"},{"line_number":499,"context_line":"                                         taxonomy.OUTCOME_FAILURE,"}],"source_content_type":"text/x-python","patch_set":1,"id":"da4df55a_df32a25d","line":496,"range":{"start_line":495,"start_character":18,"end_line":496,"end_character":72},"updated":"2016-12-28 02:55:10.000000000","message":"this doesn\u0027t explain the non-PCI case","commit_id":"970b8fd287f5c5fadf19a8d23351a721cec9593e"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"e261e565a952eac3dc5739c3f48943d6fd530392","unresolved":false,"context_lines":[{"line_number":493,"context_line":"                result \u003d f(wrapped_self, request, user_id, *args, **kwargs)"},{"line_number":494,"context_line":"            except Exception as ex:"},{"line_number":495,"context_line":"                # For authentication failure send a CADF event as well"},{"line_number":496,"context_line":"                # PCI-DSS events will have a reason associated with them"},{"line_number":497,"context_line":"                audit_reason \u003d self._get_audit_reason(ex)"},{"line_number":498,"context_line":"                _send_audit_notification(self.action, request.audit_initiator,"},{"line_number":499,"context_line":"                                         taxonomy.OUTCOME_FAILURE,"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_cbb9a6ee","line":496,"in_reply_to":"da4df55a_df32a25d","updated":"2016-12-29 15:04:40.000000000","message":"We are also only sending reasons for specific PCI-DSS events and not *every* PCI-DSS event, right? I agree that we should specify the non-PCI case in the comment.","commit_id":"970b8fd287f5c5fadf19a8d23351a721cec9593e"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"90dfce382090ae8a924815a38f4ba2ef02a5caf6","unresolved":false,"context_lines":[{"line_number":508,"context_line":""},{"line_number":509,"context_line":"        return wrapper"},{"line_number":510,"context_line":""},{"line_number":511,"context_line":"    def _get_audit_reason(self, ex):"},{"line_number":512,"context_line":"        if isinstance(ex, (exception.AccountLocked,"},{"line_number":513,"context_line":"                           exception.PasswordExpired)):"},{"line_number":514,"context_line":"            return reason.Reason(str(ex), str(ex.code))"},{"line_number":515,"context_line":"        return None"},{"line_number":516,"context_line":""},{"line_number":517,"context_line":""},{"line_number":518,"context_line":"class CadfRoleAssignmentNotificationWrapper(object):"}],"source_content_type":"text/x-python","patch_set":1,"id":"da4df55a_bf315e5c","line":515,"range":{"start_line":511,"start_character":0,"end_line":515,"end_character":19},"updated":"2016-12-28 02:55:10.000000000","message":"i think this change actually makes things harder to read/understand.\n\na single use private function ideally shouldn\u0027t need to exist; and without knowing the background it future maintainers may wonder why we\u0027re setting audit_reason to None.","commit_id":"970b8fd287f5c5fadf19a8d23351a721cec9593e"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"e261e565a952eac3dc5739c3f48943d6fd530392","unresolved":false,"context_lines":[{"line_number":508,"context_line":""},{"line_number":509,"context_line":"        return wrapper"},{"line_number":510,"context_line":""},{"line_number":511,"context_line":"    def _get_audit_reason(self, ex):"},{"line_number":512,"context_line":"        if isinstance(ex, (exception.AccountLocked,"},{"line_number":513,"context_line":"                           exception.PasswordExpired)):"},{"line_number":514,"context_line":"            return reason.Reason(str(ex), str(ex.code))"},{"line_number":515,"context_line":"        return None"},{"line_number":516,"context_line":""},{"line_number":517,"context_line":""},{"line_number":518,"context_line":"class CadfRoleAssignmentNotificationWrapper(object):"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_8bc4fe40","line":515,"range":{"start_line":511,"start_character":0,"end_line":515,"end_character":19},"in_reply_to":"ba5201f7_0d5d86e9","updated":"2016-12-29 15:04:40.000000000","message":"I\u0027d be in favor of keeping this logic in the try except above since we already have a comment there explaining PCI-DSS reasons. And since we aren\u0027t using this anywhere else yet - it would be fine to keep it isolated until we have a specific reason to move it out for re-use.","commit_id":"970b8fd287f5c5fadf19a8d23351a721cec9593e"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"b9e3edb3372eedbe54134085327e28ccd56ace81","unresolved":false,"context_lines":[{"line_number":508,"context_line":""},{"line_number":509,"context_line":"        return wrapper"},{"line_number":510,"context_line":""},{"line_number":511,"context_line":"    def _get_audit_reason(self, ex):"},{"line_number":512,"context_line":"        if isinstance(ex, (exception.AccountLocked,"},{"line_number":513,"context_line":"                           exception.PasswordExpired)):"},{"line_number":514,"context_line":"            return reason.Reason(str(ex), str(ex.code))"},{"line_number":515,"context_line":"        return None"},{"line_number":516,"context_line":""},{"line_number":517,"context_line":""},{"line_number":518,"context_line":"class CadfRoleAssignmentNotificationWrapper(object):"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_0d5d86e9","line":515,"range":{"start_line":511,"start_character":0,"end_line":515,"end_character":19},"in_reply_to":"da4df55a_bf315e5c","updated":"2016-12-29 13:23:47.000000000","message":"agree with most points, but the private function one. Code reuse is not the only case to create private functions/methods.\n\nanother option, that could look more readable is having something like:\n\n  except Exception as ex:\n    audit_reason \u003d None\n    if is_password_management_error(ex):\n      audit_reason \u003d reason.Reason(str(ex), str(ex.code))\n\n    ...","commit_id":"970b8fd287f5c5fadf19a8d23351a721cec9593e"}]}