Docker Compose: Remove wait-for-dependencies recipe
#1320
Conversation
Where applicable, replace with Compose' native `--wait` option. docker compose up --detach --wait
WalkthroughRemoved external "wait-for-dependencies" service blocks from multiple docker-compose files and replaced manual wait orchestration by adding Docker Compose's native --wait flag to startup commands and Makefile targets; some services gained or retained healthchecks. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| test: curl --max-time 25 http://localhost:4200 || exit 1 | ||
| interval: 30s | ||
| timeout: 30s | ||
|
|
||
| # Wait for all defined services to be fully available by probing their health | ||
| # status, even when using `docker compose up --detach`. | ||
| # https://marcopeg.com/2019/docker-compose-healthcheck/ | ||
| wait: | ||
| image: dadarek/wait-for-dependencies | ||
| depends_on: | ||
| metabase: | ||
| condition: service_healthy | ||
| cratedb: | ||
| condition: service_healthy |
There was a problem hiding this comment.
Problem
We need to restore this recipe with Metabase, because it uses pytest-docker-compose-v2, which doesn't consider Compose' built-in health checks yet?
References
There was a problem hiding this comment.
Could you please comment this so we know in future (and can change this) why we still keep it?
Btw. looks like the pytest-docker-composer functionality could achieved quite simple when reading https://tpwo.github.io/blog/2024/07/15/python-integration-tests-with-pytest-and-docker-compose/? jfyi, probably there is more which I did not grasp.
There was a problem hiding this comment.
It looks like the recipe uses shellouts to run the Docker CLI. Sure that is a viable option. In this case, the test uses a Python package, so we took the chance to create an upstream ticket.
There was a problem hiding this comment.
Could you please comment this so we know in future (and can change this) why we still keep it?
Done with 425fb89.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
application/ingestr/kafka-compose.yml(0 hunks)application/ingestr/kafka-demo.xsh(1 hunks)application/roapi/compose.yml(0 hunks)application/roapi/test.sh(1 hunks)framework/flink/kafka-jdbcsink-java/Makefile(1 hunks)framework/flink/kafka-jdbcsink-java/docker-compose.yml(0 hunks)framework/flink/kafka-jdbcsink-java/test.sh(1 hunks)operation/compose/ssl/Makefile(1 hunks)operation/compose/ssl/compose.yml(0 hunks)topic/machine-learning/open-webui/compose.yml(0 hunks)
💤 Files with no reviewable changes (5)
- application/roapi/compose.yml
- operation/compose/ssl/compose.yml
- framework/flink/kafka-jdbcsink-java/docker-compose.yml
- application/ingestr/kafka-compose.yml
- topic/machine-learning/open-webui/compose.yml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-23T22:00:51.593Z
Learnt from: amotl
Repo: crate/cratedb-examples PR: 1038
File: application/open-webui/compose.yml:44-48
Timestamp: 2025-07-23T22:00:51.593Z
Learning: In the cratedb-examples repository, hard-coded credentials like "crate:crate" in Docker Compose files are acceptable for demonstration purposes to maintain simplicity and avoid unnecessary layers of indirection, even when flagged by security tools like Checkov.
Applied to files:
application/roapi/test.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: OS ubuntu-latest
🔇 Additional comments (3)
operation/compose/ssl/Makefile (1)
6-6: LGTM! Confirm health checks for SSL compose setup.The
--waitaddition ensures services are healthy before client tests execute. Health checks are properly defined in the compose file with a test command verifying the SSL endpoint athttps://localhost:4200, appropriate retry/timeout settings, and a 3-second start period suitable for SSL initialization.application/ingestr/kafka-demo.xsh (1)
36-36: Health checks are properly configured for--waitflag to function correctly.The kafka-compose.yml file includes health check definitions for both primary services: Kafka uses a netcat probe on the internal broker port, and CrateDB uses a curl probe on the HTTP port. Both checks have appropriate timeout and retry configurations, enabling the
--waitflag to correctly determine service readiness during startup.framework/flink/kafka-jdbcsink-java/Makefile (1)
7-7: LGTM! The behavioral change from blocking to detached mode with health check monitoring is correct.The command now runs services in detached mode (
--detach) while waiting for readiness checks (--wait), which requires healthcheck entries defined in docker-compose.yml. This waits until containers are healthy and only then exits while containers run in the background. The docker-compose.yml already includes healthchecks for zookeeper, kafka-broker, flink-jobmanager, and cratedb, so the synchronization guarantee is maintained.
Otherwise, the downstream application will connect to CrateDB or other services before they are ready to serve requests, resulting in connection errors.
amotl
force-pushed
the
compose-rm-wait-for-dependencies
branch
from
e0a8262 to
929b055
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
application/metabase/docker-compose.ymlapplication/roapi/compose.ymlapplication/roapi/roapi.Dockerfileframework/flink/kafka-jdbcsink-java/docker-compose.yml
✅ Files skipped from review due to trivial changes (1)
- application/metabase/docker-compose.yml
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: amotl
Repo: crate/cratedb-examples PR: 1038
File: application/open-webui/README.md:68-72
Timestamp: 2025-07-23T20:40:47.146Z
Learning: In Docker Compose documentation, when describing service counts, distinguish between main daemon services that users interact with versus auxiliary helper jobs/setup containers. Focus documentation on the user-facing services rather than technical infrastructure services.
📚 Learning: 2025-07-27T22:22:57.501Z
Learnt from: amotl
Repo: crate/cratedb-examples PR: 1038
File: application/open-webui/compose.yml:103-108
Timestamp: 2025-07-27T22:22:57.501Z
Learning: The ghcr.io/open-webui/open-webui:0.6.18 Docker image includes curl as an available HTTP client tool, as verified by amotl through real-world testing. The health check using `test: [ "CMD", "curl", "--fail", "http://localhost:8080" ]` works correctly with this image.
Applied to files:
application/roapi/compose.yml
📚 Learning: 2025-07-24T12:30:34.402Z
Learnt from: amotl
Repo: crate/cratedb-examples PR: 0
File: :0-0
Timestamp: 2025-07-24T12:30:34.402Z
Learning: The official crate/crate:6.0.0 Docker image includes curl (version 8.12.1 as demonstrated by amotl), contrary to test results that may vary depending on execution environment, platform architecture, or image registry differences.
Applied to files:
application/roapi/roapi.Dockerfile
📚 Learning: 2025-07-24T12:30:34.402Z
Learnt from: amotl
Repo: crate/cratedb-examples PR: 0
File: :0-0
Timestamp: 2025-07-24T12:30:34.402Z
Learning: The official CrateDB Docker image crate/crate:6.0.0 includes curl (version 8.12.1), as demonstrated by amotl. Note that Docker is not available in the CodeRabbit sandbox environment, so Docker image contents cannot be directly tested through shell scripts.
Applied to files:
application/roapi/roapi.Dockerfile
📚 Learning: 2025-07-24T12:30:01.944Z
Learnt from: amotl
Repo: crate/cratedb-examples PR: 0
File: :0-0
Timestamp: 2025-07-24T12:30:01.944Z
Learning: The official crate/crate:6.0.0 Docker image includes curl as an available HTTP client tool. The image name should be specified as "crate/crate:6.0.0" not just "crate:6.0.0".
Applied to files:
application/roapi/roapi.Dockerfile
📚 Learning: 2025-07-24T12:30:01.944Z
Learnt from: amotl
Repo: crate/cratedb-examples PR: 0
File: :0-0
Timestamp: 2025-07-24T12:30:01.944Z
Learning: The official crate/crate:6.0.0 Docker image includes curl as an available HTTP client tool. This has been verified by direct testing with the command `docker run --rm -it crate/crate:6.0.0 curl --help`.
Applied to files:
application/roapi/roapi.Dockerfile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: OS ubuntu-latest
- GitHub Check: OS ubuntu-latest
🔇 Additional comments (2)
application/roapi/roapi.Dockerfile (1)
31-31: LGTM!Adding
curlto support the healthcheck incompose.ymlis the right approach. The change is minimal and provides the necessary tooling for service readiness verification.framework/flink/kafka-jdbcsink-java/docker-compose.yml (1)
102-107: The process-based healthcheck is appropriate for TaskManager.The Flink REST API monitoring is backed by a web server that runs as part of the JobManager, not the TaskManager. Unlike the JobManager which exposes an HTTP endpoint, TaskManager health is monitored via RPC heartbeats where JobManager actively sends heartbeat requests to detect whether TaskExecutor is alive. The process-based healthcheck using
psand grep is a valid approach for verifying the TaskManager process is running in a Docker container.
| healthcheck: | ||
| test: ["CMD", "curl", "--fail", "http://localhost:8080/health"] | ||
| start_period: 3s | ||
| interval: 1.5s | ||
| retries: 30 | ||
| timeout: 30s |
There was a problem hiding this comment.
ROAPI does not expose a /health endpoint for health checks.
The healthcheck is configured to call http://localhost:8080/health, but ROAPI's documented API only includes data query endpoints (/api/sql, /api/graphql, /api/tables, /api/schema). This healthcheck will fail immediately, causing the container to restart repeatedly. Either use a simpler health check (e.g., curl http://localhost:8080/api/schema), or remove the healthcheck if the deployment doesn't require one.
🤖 Prompt for AI Agents
In application/roapi/compose.yml around lines 45 to 50, the healthcheck targets
/health which ROAPI doesn't expose; update the healthcheck to use a valid
endpoint such as "http://localhost:8080/api/schema" (or another documented API
path like /api/sql, /api/graphql, or /api/tables) or remove the healthcheck
entirely if not required by your deployment; ensure the test command uses --fail
and appropriate timeout/retry settings so the container won't repeatedly restart
on a permanent absence of a health endpoint.
There was a problem hiding this comment.
It looks like it works well. Maybe documentation can be improved? /cc @houqp, @coderabbitai
$ http localhost:8080/health
HTTP/1.1 200 OK
access-control-allow-origin: *
content-length: 2
content-type: text/plain
date: Mon, 22 Dec 2025 23:16:57 GMT
vary: origin, access-control-request-method, access-control-request-headers
OK
## About The existing healtcheck endpoint http://localhost:8080/health can now be used without much ado within Docker Compose service definitions, and it works well. ```yaml healthcheck: test: ["CMD", "curl", "--fail", "http://localhost:8080/health"] start_period: 3s interval: 1.5s retries: 30 timeout: 30s ``` ## References - Resolves: #418 - See also: crate/cratedb-examples#1320 (review)
About
Let's remove redundant details, here about synchronizing CI workflows.
Details
Where applicable, replace manual recipe using the
wait-for-dependenciescontainer with the Compose' native--waitoption.References