-
-
Notifications
You must be signed in to change notification settings - Fork 50
Added an uptime-check
page that the automated uptime checkers can use
#3610
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
Conversation
owner: "SwiftPackageIndex", | ||
repository: "SemanticVersion") | ||
return "Success" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our smoke test is already exercising the db via package page and search requests. I'm wondering why we'd need this extra route?
If we do this, I should call it /healthcheck
.
Edit: If we do this, I think we should call it /healthcheck
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the proposal for these changes I offered you two alternatives for this piece and you expressed no opinion between them.
You had previously expressed an opinion to not get misleading data into our analytics, which I agree with, and this scenario avoids that completely as it allows us to keep the automated traffic detection protection enabled for the home page.
But of course it’s wrong. Why would I expect anything different?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, and you know this already, the smoke test only happens at deploy time, the uptime check is a constant monitor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not quite, the smoke tests also run every four hours. As to the Azure probes, they can probe a package page in addition if we feel that's important. I'm simply asking it wouldn't be simpler and more realistic to do just that.
I'll tick the PR either way, it was just a question if you've considered this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd meant to tick the PR earlier but forgot after commenting.
FWIW, we know that the CF cache isn't preventing the uptime checker from alerting. I don't think we need a special route with special caching. |
9599702
to
cb6c188
Compare
Merge after #3609
Adds a simple uptime check route that exercises the database that we should monitor with the Azure and StatusPage checkers.
I'll set this page to cache this for 5 minutes which will prevent it being abused, but it will be a much more accurate test than the home page, which is cached for many hours.