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

server: update health check endpoint to query storage periodically #1397

Merged
merged 1 commit into from
Feb 4, 2019

Conversation

ericchiang
Copy link
Contributor

Instead of querying the storage every time a health check is performed
query it periodically and save the result.

Updates #1386

Copy link
Contributor

@srenatus srenatus left a comment

Choose a reason for hiding this comment

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

LGTM, just an imports nitpick. Also: would you think it's silly to add a test? 😉

server/handlers.go Outdated Show resolved Hide resolved
Instead of querying the storage every time a health check is performed
query it periodically and save the result.
@ericchiang ericchiang force-pushed the health-check-endpoint branch from d7a21b0 to 8935a14 Compare February 4, 2019 19:02
@ericchiang
Copy link
Contributor Author

We've already got test coverage but I added a negative test :)

@srenatus
Copy link
Contributor

srenatus commented Feb 4, 2019 via email

@srenatus srenatus merged commit 815311f into dexidp:master Feb 4, 2019
@ericchiang ericchiang deleted the health-check-endpoint branch February 4, 2019 21:02
@srenatus
Copy link
Contributor

srenatus commented Feb 5, 2019

💭 I guess making the 15s configurable would be a good first PR

@ericchiang
Copy link
Contributor Author

Instead of exposing a knob maybe we could choose a better value? Or lower the frequency when the health check isn't queried :)

mmrath pushed a commit to mmrath/dex that referenced this pull request Sep 2, 2019
server: update health check endpoint to query storage periodically
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