nickjj / docker-django-example

A production ready example Django app that's using Docker and Docker Compose.
MIT License
1.22k stars 265 forks source link

Make sure postgres is healthy and not simply started #48

Open Winnie-Fred opened 5 months ago

Winnie-Fred commented 5 months ago

I have added healthchecks for postgres, redis and celery worker. The depends_on condition for postgres and redis has been changed from service_started to service_healthy to ensure the services are healthy and ready to accept connections. This would fix the Django Operational error that is sometimes raised when running the network of containers, especially in development. This solution does not use polling to check health status and implements only a few tweaks to the existing docker compose configuration.

I have also added timeout config to the gunicorn config to fix gunicorn worker exiting due to [CRITICAL] WORKER TIMEOUT Error handling request (no URI read).

nickjj commented 5 months ago

Hi, thanks. I am traveling internationally and will be back later next week. I will check this then.

On Sat, Jun 15, 2024, 4:18 AM Igboama Winifred @.***> wrote:

I have added healthchecks for postgres, redis and celery worker. The depends_on condition for postgres and redis has been changed from service_started to service_healthy to ensure the services are healthy and ready to accept connections. This would fix the Django Operational error that is sometimes raised when running the network of containers, especially in development. This solution does not use polling to check health status and implements only a few tweaks to the existing docker compose configuration.

I have also added timeout config to the gunicorn config to fix gunicorn worker exiting due to [CRITICAL] WORKER TIMEOUT Error handling request (no URI read).

You can view, comment on, or merge this pull request online at:

https://github.com/nickjj/docker-django-example/pull/48 Commit Summary

File Changes

(2 files https://github.com/nickjj/docker-django-example/pull/48/files)

Patch Links:

— Reply to this email directly, view it on GitHub https://github.com/nickjj/docker-django-example/pull/48, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGGRI74LQF5BUVGZ372OW3ZHOXA3AVCNFSM6AAAAABJLKDMTOVHI2DSMVQWIX3LMV43ASLTON2WKOZSGM2TINBUGM4DOOA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

Winnie-Fred commented 5 months ago

Sure, no problem :+1: .

nickjj commented 5 months ago

I do like that pg_isready exists, I never used it before. Normally I've always connected and ran a SELECT 1 which demonstrates you can login as a specific user and the DB server can accept queries. Sounds like pg_isready does something comparable.

I think there's a few topics to tackle:

The 2nd one is easy, for example ["CMD-SHELL", "pg_isready -U $$POSTGRES_USER -d $$POSTGRES_DB"] can be changed to ["CMD-SHELL", "pg_isready", "-U", "$$POSTGRES_USER", "-d", "$$POSTGRES_DB"] to use the array syntax for all of the flags. When it comes to using $$foo, does ${$foo} also work? I haven't tested that. The same changes could be applied to the other health checks.

Also, maybe we can remove setting the 120s timeout on gunicorn from this PR as it's a different type of change.

Winnie-Fred commented 5 months ago

Alright. I am looking into $$foo vs ${$foo} now. I am also adjusting the health check commands to use the array syntax.

You are right, the gunicorn timeout should be in a different PR and commit.

Also, can you explain what you mean when you asked if we want to perform the health checks here?

nickjj commented 5 months ago

Oh, I mean if we should merge the PR in general.