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

Suppress logging of /ready handler errors #840

Merged
merged 2 commits into from
Jun 6, 2018
Merged

Conversation

juliusv
Copy link
Contributor

@juliusv juliusv commented Jun 6, 2018

This is achieved by returning Status Unavailable (which is exempt from
logging in the logging middleware) instead of an Internal Server Error.

Fixes #810

juliusv added 2 commits June 6, 2018 12:42
This is achieved by returning Status Unavailable (which is exempt from
logging in the logging middleware) instead of an Internal Server Error.

Fixes #810
Copy link
Contributor

@csmarchbanks csmarchbanks left a comment

Choose a reason for hiding this comment

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

Is the vendor update necessary for this? If not would it make more sense to do it as a separate PR?

Otherwise the actual fix looks good.

@juliusv
Copy link
Contributor Author

juliusv commented Jun 6, 2018

@csmarchbanks Yeah, the vendor update pulls in weaveworks/common#97

Copy link
Contributor

@csmarchbanks csmarchbanks left a comment

Choose a reason for hiding this comment

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

👍

@juliusv juliusv merged commit 88c86b7 into master Jun 6, 2018
@juliusv juliusv deleted the disable-readiness-logging branch June 6, 2018 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants