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

Wait for db using healthchecks #17

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

squatica
Copy link

closes #16

@JannikStreek
Copy link
Member

thanks for the PR, I'll test it

codiflow added a commit to SWEETGOOD-DE/excalidraw-storage-backend that referenced this pull request May 21, 2024
@JannikStreek
Copy link
Member

@squatica sorry for the late reply. I would like to keep the entry point file, however, we can make the Postgres part optional depending on some ENV variable. We don't use docker compose in our prod setup. E.g. DATABASE_ADAPTER=postgres by default and you can set it to a different value.

@squatica
Copy link
Author

squatica commented Jan 5, 2025

@JannikStreek So you use the docker images, but not with health-checks? Or you don't use docker at all in prod? In general it's better to rely on health-checks, where a container defines its own healthy conditions, and other containers just wait for healthy status. Alternatively you could try to connect to the db normally in the .ts code, and when it fails just throw a fatal and rely on the service manager to restart the backend.

I could keep the entrypoint in repo but not use it in Dockerfile. But I'd recommend to avoid adding the postgresql-client dependency in Dockerfile, because that's essentially duplicated library (your @keyv/postgres already knows how to work with postgres), and it would be dead weight for anybody else who either uses redis or follows the best practices with health-checks.

@JannikStreek
Copy link
Member

@squatica we use the docker container, however, not together with docker compose but with Kubernetes. I will double check our configuration and get back to you.

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.

New entrypoint script depends on Postgres being used
2 participants