Skip to content

Quick Fix: Ensure Nginx Honors depends_on clause - #1346

Open
happycodemonkey wants to merge 1 commit into
mainfrom
task/nginx-honor-depends-on
Open

happycodemonkey wants to merge 1 commit into
mainfrom
task/nginx-honor-depends-on

Conversation

@happycodemonkey

Copy link
Copy Markdown
Member

Overview

No ticket for this one, I have noticed on my dev environment nginx will almost always fail to come up because it is not properly waiting on cms and django to be up. I have updated the depends_on clause as detailed in the docker documentation to tell it to wait until both services have started.

Related

No tickets involved

Changes

Updated depends_on to include service_started condition in the nginx block

Testing

  1. This should ideally just work like it always has, or more reliably bring everything up if it has been an issue.

@codecov

codecov Bot commented Aug 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.15%. Comparing base (43e1d9a) to head (18a3367).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1346   +/-   ##
=======================================
  Coverage   64.15%   64.15%           
=======================================
  Files         464      464           
  Lines       13853    13853           
  Branches     2767     2767           
=======================================
  Hits         8888     8888           
  Misses       4611     4611           
  Partials      354      354           
Flag Coverage Δ
javascript 67.16% <ø> (ø)
unittests 60.97% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@wesleyboar wesleyboar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(I reviewed this, wondering if Core-CMS docker-compose could benefit.)

I think this change, as is, has no effect (Docker Docs suggest this change is equivalent to the "short syntax"), but that this long format might yet achieve your goal, because docs also suggest service_healthy is a different value that could help if you define a successful "healthcheck" for cms and django.

@happycodemonkey

Copy link
Copy Markdown
Member Author

@wesleyboar thanks for taking a look and adding your feedback, this change does solve the issue I was having with my dev environment but I'm unclear why the short syntax doesn't work at all for me while the full one does, perhaps some weirdness with docker versions? I am curious if anyone else has had issues with this. I will look into defining the healthcheck as you suggested though, that seems like it will be more generally useful.

This branch has not been deployed

No deployments
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.

2 participants