Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store failure reason for github event when marking it as errored #975

Merged
merged 1 commit into from
Sep 26, 2017

Conversation

begedin
Copy link
Contributor

@begedin begedin commented Sep 26, 2017

Closes #873

This is just a basic solution.

Things to discuss, how to enhance

  • maybe rename failure_reason -> error_reason/errored_reason
  • instead of just storing the atoms the processing code returns, we could map the atoms into "user friendly" actual messages. I feel the atoms are just a layer of abstraction that doesn't really help us.

@joshsmith
Copy link
Contributor

I agree we should probably have some more user-friendly messages.

Copy link
Contributor

@joshsmith joshsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should make the column error? Or errors that's an array of them a la changeset errors? Idk if there's ever a reason we'd have multiple errors that we've collected.

Did you want to do the error reason rewrite into user friendly messages here or elsewhere?

@begedin
Copy link
Contributor Author

begedin commented Sep 26, 2017

I would do that elsewhere, probably something we can work in as we figure out what the common reasons for errors are :)

@begedin
Copy link
Contributor Author

begedin commented Sep 26, 2017

I don't think we would have multiple errors, unless a changeset is a failure and we want to store the errors from the changeset.

Generally, an errored event happens when the handler module returns an error tuple, so we would not have more than one. The "friendly error" proposal would be mapping the atom in the second tuple element into a string.

@joshsmith joshsmith force-pushed the 873-track-github-event-failure-reason branch from de5300e to cfcc9c1 Compare September 26, 2017 21:57
@joshsmith joshsmith force-pushed the 873-track-github-event-failure-reason branch from cfcc9c1 to 602307e Compare September 26, 2017 21:59
@joshsmith joshsmith merged commit e0061c4 into develop Sep 26, 2017
@joshsmith joshsmith deleted the 873-track-github-event-failure-reason branch September 26, 2017 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants