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

Make webserver resilient to database not starting up #649

Merged
merged 12 commits into from
May 19, 2022

Conversation

allisonking
Copy link
Contributor

@allisonking allisonking commented May 13, 2022

Closes #562

Code Changes

  • Instead of failing, surface an issue if the server can't reach the database
  • Fix health endpoint to only return healthy if the db is up too

Steps to Confirm

  • change the database host in the docker-compose file
  • nox -s api
  • confirm in the logs that the database is unreachable
  • go to localhost:8080/api/v1/health and confirm that the database is unhealthy

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation Updated:
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md

Description Of Changes

This PR updates how the webserver initializes so that even if the database is unreachable, the webserver is still able to spin up. The health command has also been updated to return various health statuses to provide better debugging for users.

@allisonking allisonking changed the title Update changelog Make webserver resilient to database not starting up May 13, 2022
@allisonking
Copy link
Contributor Author

Would love some eyes on this to make sure I'm going in the right direction with how I interpreted the ticket. I'm doing some except Exception as error which pylint is complaining about because it's too broad, but it seems like that's what we want? want to make sure though!

Also I am getting destroyed by this unit test. I'm trying to mock a call and have not been succeeding, and then I discovered FastAPI mocks are kind of weird but I still haven't successfully mocked the call.

tests/core/test_api.py Outdated Show resolved Hide resolved
@allisonking
Copy link
Contributor Author

I'm having trouble figuring out the failing test. It does fail locally for me too, but I'm having trouble seeing what the Internal Server Error is. I'm running nox -s api and when I navigate to http://localhost:8080/api/v1/data_use/visualize/graphs I see "Internal Server Error" but nothing in the logs about what it could be. Any tips?

@ThomasLaPiana
Copy link
Contributor

@allisonking I'll take a look and get this over the line, I have a hunch of what it is but its hard to articulate (its a weird endpoint)

@ThomasLaPiana ThomasLaPiana marked this pull request as ready for review May 19, 2022 14:48
@ThomasLaPiana ThomasLaPiana requested a review from a team May 19, 2022 14:48
@ThomasLaPiana ThomasLaPiana removed their assignment May 19, 2022
Copy link
Contributor

@ThomasLaPiana ThomasLaPiana left a comment

Choose a reason for hiding this comment

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

LGTM! @allisonking thanks!

@ThomasLaPiana ThomasLaPiana merged commit 6b3f4c7 into main May 19, 2022
@ThomasLaPiana ThomasLaPiana deleted the aking-562-webserver-db-resilience branch May 19, 2022 14:54
@allisonking allisonking mentioned this pull request Jun 3, 2022
8 tasks
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.

Update the webserver so that it won't fail to start if the database is inaccessible
3 participants