这是indexloc提供的服务，不要输入任何密码
Skip to content

check db connection in healthz endpoint (close #2645) #3440

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Dec 3, 2019
Merged

check db connection in healthz endpoint (close #2645) #3440

merged 5 commits into from
Dec 3, 2019

Conversation

@tirumaraiselvan
Copy link
Contributor

@tirumaraiselvan tirumaraiselvan commented Nov 28, 2019 •
edited
Loading

Description

Currently, /healthz endpoint does not check for db connectivity. It seems vital to check for db connectivity in the health check.

Also, /healthz throws 500 when inconsistent objects are detected. This seems not really correct because hasura starts and functions even with inconsistent objects. We should throw a WARN here instead.

Affected components

  • Server
  • Docs

Related issues

Not exactly sure what this issue means but I think we should close this with this PR
#2645

Limitations, known bugs & workarounds

Should we add some check here which checks whether graphql API is working?

@netlify
Copy link

netlify bot commented Nov 28, 2019 •
edited
Loading

Deploy preview for hasura-docs ready!

Built with commit 2056101

https://deploy-preview-3440--hasura-docs.netlify.com

@hasura-bot
Copy link
Contributor

hasura-bot commented Nov 28, 2019

Review app for commit 54c265e deployed to Heroku: https://hge-ci-pull-3440.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3440-54c265e0

lexi-lambda
lexi-lambda previously approved these changes Dec 2, 2019
View reviewed changes
@hasura-bot
Copy link
Contributor

hasura-bot commented Dec 2, 2019

Review app for commit 9e1e999 deployed to Heroku: https://hge-ci-pull-3440.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3440-9e1e9998

tirumaraiselvan
tirumaraiselvan commented Dec 3, 2019
View reviewed changes
server/src-lib/Hasura/Server/App.hs
then Spock.setStatus HTTP.status200 >> Spock.lazyBytes "OK"
else Spock.setStatus HTTP.status500 >> Spock.lazyBytes "ERROR"
dbOk <- checkDbConnection
if dbOk
Copy link
Contributor Author

@tirumaraiselvan tirumaraiselvan Dec 3, 2019

Choose a reason for hiding this comment

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

There was no content-type set when using lazyBytes, also no point using lazyBytes for such small strings.

@hasura-bot
Copy link
Contributor

hasura-bot commented Dec 3, 2019

Review app for commit 1a223d3 deployed to Heroku: https://hge-ci-pull-3440.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3440-1a223d39

@tirumaraiselvan tirumaraiselvan changed the title check db connection in healthz endpoint check db connection in healthz endpoint (close #2645) Dec 3, 2019
@hasura-bot
Copy link
Contributor

hasura-bot commented Dec 3, 2019

Review app for commit 2056101 deployed to Heroku: https://hge-ci-pull-3440.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3440-2056101c

@lexi-lambda lexi-lambda merged commit 3cd656f into hasura:master Dec 3, 2019
@hasura-bot
Copy link
Contributor

hasura-bot commented Dec 3, 2019

Review app https://hge-ci-pull-3440.herokuapp.com is deleted

polRk pushed a commit to polRk/graphql-engine that referenced this pull request Feb 12, 2020
@tirumaraiselvan @polRk
check db connection in healthz endpoint (close hasura#2645) (hasura#3440
0424819 
)

* check db connection in healthz endpoint
* use Spock.text to automatically set Content-Type
* update docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shahidhk shahidhk Awaiting requested review from shahidhk

2 more reviewers

@lexi-lambda lexi-lambda lexi-lambda approved these changes

@marionschleifer marionschleifer marionschleifer approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tirumaraiselvan @hasura-bot @lexi-lambda @marionschleifer