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

Mark some BuildEnvironmentError exceptions as Warning and do not log them #4495

Merged
merged 7 commits into from Aug 22, 2018

Conversation

Projects
None yet
3 participants
@humitos
Member

humitos commented Aug 8, 2018

There are some exceptions risen while building documentation that are considered an ERROR from the Build perspective, but for our application are just a WARNING and shouldn't be logged as ERROR (sent to Sentry).

It's not an ERROR in our application but a WARNING that the user's documentation couldn't be built.

This will help us to find Sentry more useful than it's right now as we are getting close to have logged only things that need human/dev attention.

NOTE: this is the most important commit 1dd850a, the first one is a minimum refactor and the last one introduces some style changes

Closes: #4109

@humitos humitos requested a review from rtfd/core Aug 8, 2018

@ericholscher

Looks good. Will definitely be nice not to have so much spam in sentry.

@ericholscher

This comment has been minimized.

Show comment
Hide comment
@ericholscher

ericholscher Aug 22, 2018

Member

@humitos we just hit our limit again, we should fix the conflict here and ship it.

Member

ericholscher commented Aug 22, 2018

@humitos we just hit our limit again, we should fix the conflict here and ship it.

humitos added some commits Aug 8, 2018

Mark BuildEnvironmentError exceptions as Warning and do not log them
There are some exceptions risen while building documentation that are
considered an ERROR from the Build perspective, but for our
application are just a WARNING and shouldn't be logged as ERROR (sent
to Sentry).

It's not an ERROR in our application but a WARNING that the user's
documentation couldn't be built.
Style by `pre-commit run`
Note: the changes were selected manually since there were many that
break the style that we want :(
@humitos

This comment has been minimized.

Show comment
Hide comment
@humitos

humitos Aug 22, 2018

Member

Rebased the branch. Once the tests passed, I will merge it.

Member

humitos commented Aug 22, 2018

Rebased the branch. Once the tests passed, I will merge it.

@humitos humitos merged commit 883b383 into master Aug 22, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@humitos humitos deleted the humitos/build/log-exceptions branch Aug 22, 2018

@stsewd

This comment has been minimized.

Show comment
Hide comment
@stsewd

stsewd Aug 22, 2018

Member

This solves #3856? or we still need less logs?

Member

stsewd commented Aug 22, 2018

This solves #3856? or we still need less logs?

@humitos

This comment has been minimized.

Show comment
Hide comment
@humitos

humitos Aug 22, 2018

Member

I don't think it 100% solves it, but at least reduce the noise.

I'd like to get to the point to anything logged in Sentry requires manual/human attention. Currently, there are still lot of garbage there.

Member

humitos commented Aug 22, 2018

I don't think it 100% solves it, but at least reduce the noise.

I'd like to get to the point to anything logged in Sentry requires manual/human attention. Currently, there are still lot of garbage there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment