From 9275c6dd3c33417666fee683b82ca9c325fe62ac Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 23 May 2026 22:46:13 +0000 Subject: [PATCH] fix(docker): address Greptile review (depends_on health + exact port tests) Two issues raised in Greptile 4/5 review: 1. gateway depends_on used the list form which starts the gateway before Postgres is ready. Switch to condition: service_healthy so the gateway waits for pg_isready to pass on cold boots. 2. Port tests used substring matching ("3000" in str(p)) which could pass spurious mappings like "13000:3000". Change to exact string equality "3000:3000" / "4000:4000". Also assert service_healthy in test_gateway_depends_on_db. Resolves LIT-2815 --- docker-compose.yml | 3 ++- tests/test_litellm/test_docker_compose.py | 23 +++++++++++++++-------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 09e8f1bc944..7659c210519 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -38,7 +38,8 @@ services: env_file: - .env # Load local .env file depends_on: - - db # Indicates that this service depends on the 'db' service, ensuring 'db' starts first + db: + condition: service_healthy # Wait for Postgres to pass pg_isready before starting gateway healthcheck: # Defines the health check configuration for the container test: - CMD-SHELL diff --git a/tests/test_litellm/test_docker_compose.py b/tests/test_litellm/test_docker_compose.py index 7f86a7e12f3..a5a81d8f3a2 100644 --- a/tests/test_litellm/test_docker_compose.py +++ b/tests/test_litellm/test_docker_compose.py @@ -91,16 +91,16 @@ def test_gateway_builds_from_gateway_dockerfile(services): def test_ui_exposes_port_3000(services): - ports = services["ui"].get("ports", []) - assert any("3000" in str(p) for p in ports), ( - f"ui service should expose port 3000, got ports={ports}" + ports = [str(p) for p in services["ui"].get("ports", [])] + assert "3000:3000" in ports, ( + f"ui service should expose port mapping 3000:3000, got ports={ports}" ) def test_gateway_exposes_port_4000(services): - ports = services["gateway"].get("ports", []) - assert any("4000" in str(p) for p in ports), ( - f"gateway service should expose port 4000, got ports={ports}" + ports = [str(p) for p in services["gateway"].get("ports", [])] + assert "4000:4000" in ports, ( + f"gateway service should expose port mapping 4000:4000, got ports={ports}" ) @@ -142,8 +142,15 @@ def test_ui_healthcheck_configured(services): def test_gateway_depends_on_db(services): - depends_on = services["gateway"].get("depends_on", []) - assert "db" in depends_on, "gateway service must depend on the 'db' service" + depends_on = services["gateway"].get("depends_on", {}) + # depends_on can be a list or a dict (service_healthy condition form) + if isinstance(depends_on, dict): + assert "db" in depends_on, "gateway service must depend on the 'db' service" + assert depends_on["db"].get("condition") == "service_healthy", ( + "gateway should wait for db to be healthy before starting" + ) + else: + assert "db" in depends_on, "gateway service must depend on the 'db' service" # ---------------------------------------------------------------------------