Copilot commented on code in PR #38161:
URL: https://github.com/apache/superset/pull/38161#discussion_r2885405703
##########
docker-compose.yml:
##########
@@ -186,6 +189,14 @@ services:
- path: docker/.env-local # optional override
required: false
volumes: *superset-volumes
+ healthcheck:
+ # Check if webpack dev server is responding on port 9000
+ # This prevents nginx from proxying before the frontend is ready
+ test: ["CMD-SHELL", "node -e \"const http = require('http');
http.get('http://localhost:9000', (r) => process.exit(r.statusCode === 200 ? 0
: 1)).on('error', () => process.exit(1))\""]
Review Comment:
The healthcheck probes `http://localhost:9000/` and requires a `200`
response. In this repo the webpack dev server proxies most paths (including
`/`) to the backend (see `superset-frontend/webpack.proxy-config.js`), so this
endpoint commonly returns a redirect (e.g. `302` to `/login`) or may be blocked
on backend readiness, causing the container to stay `unhealthy` and `nginx` to
never start. Probe an asset endpoint served directly by webpack (e.g.
`/static/assets/manifest.json`) and/or treat any 2xx/3xx as healthy.
```suggestion
# Check if webpack dev server is serving static assets on port 9000
# This prevents nginx from proxying before the frontend is ready
test: ["CMD-SHELL", "node -e \"const http = require('http');
http.get('http://localhost:9000/static/assets/manifest.json', (r) =>
process.exit(r.statusCode >= 200 && r.statusCode < 400 ? 0 : 1)).on('error', ()
=> process.exit(1))\""]
```
##########
docker-compose.yml:
##########
@@ -61,6 +61,9 @@ services:
volumes:
- ./docker/nginx/nginx.conf:/etc/nginx/nginx.conf:ro
- ./docker/nginx/templates:/etc/nginx/templates:ro
+ depends_on:
+ superset-node:
+ condition: service_healthy
Review Comment:
`nginx` now waits on `superset-node` being *healthy*. However,
`docker/docker-frontend.sh` exits immediately when
`BUILD_SUPERSET_FRONTEND_IN_DOCKER=false` (a supported workflow per the comment
in this service), which means `superset-node` will never become healthy and
`nginx` will never start. Consider either (a) keeping `superset-node` running
even in the "skip" mode and making its healthcheck validate the externally-run
dev server, or (b) moving the waiting logic into the `nginx` container
(entrypoint wait loop) so the setup still works when the dev server is run on
the host.
```suggestion
condition: service_started
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
