codeant-ai-for-open-source[bot] commented on code in PR #40837:
URL: https://github.com/apache/superset/pull/40837#discussion_r3370262286
##########
docker/nginx/templates/superset.conf.template:
##########
@@ -16,12 +16,12 @@
# under the License.
upstream superset_app {
- server host.docker.internal:8088;
+ server superset:8088;
keepalive 100;
}
upstream superset_websocket {
- server host.docker.internal:8080;
+ server superset-websocket:8080;
Review Comment:
**Suggestion:** Using Docker service DNS names in `upstream` without dynamic
re-resolution means nginx can keep stale backend IPs after container restarts,
leading to intermittent 502s even though a resolver was added globally. Add
dynamic DNS resolution for upstream servers (for example via `resolve`
semantics) so nginx refreshes backend addresses when containers are recreated.
[stale reference]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Superset HTTP requests can intermittently return 502 errors.
- ❌ Websocket connections can fail after backend restarts.
- ⚠️ Requires manual nginx restart to recover backend connectivity.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start the full dev stack with `docker compose up` using
`docker-compose.yml`, which
launches the `nginx` service (lines 48–76) and backend services `superset`
(lines 99–120)
and `superset-websocket` (lines 121–152) on a Docker network where service
names like
`superset` and `superset-websocket` resolve via Docker DNS.
2. Inspect `docker/nginx/nginx.conf` lines 29–32 and note that a global
`resolver
127.0.0.11 valid=10s; resolver_timeout 5s;` is configured, but
`docker/nginx/templates/superset.conf.template` defines upstreams at lines
18–25 as
`upstream superset_app { server superset:8088; ... }` and `upstream
superset_websocket {
server superset-websocket:8080; ... }` without any `resolve` parameter or
variable-based
`proxy_pass`.
3. After the stack is running, execute a partial restart such as `docker
compose restart
superset superset-websocket`, which recreates these containers with new IPs
while leaving
the `nginx` container and its worker processes (and their cached upstream
resolutions)
running.
4. Subsequent HTTP and websocket requests via nginx to
`${SUPERSET_APP_ROOT}` and `/ws`
(handled by `proxy_pass http://superset_app;` and `proxy_pass
http://superset_websocket;`
at `docker/nginx/templates/superset.conf.template` lines 32–48) can continue
targeting the
old IPs because the upstream `server superset:8088;` and `server
superset-websocket:8080;`
entries are resolved only once at nginx startup, leading to intermittent 502
errors until
nginx itself is restarted; the global `resolver` is not wired to re-resolve
these static
upstream hostnames.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9f140aef136442a9a37424a6bc502b0d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9f140aef136442a9a37424a6bc502b0d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** docker/nginx/templates/superset.conf.template
**Line:** 18:24
**Comment:**
*Stale Reference: Using Docker service DNS names in `upstream` without
dynamic re-resolution means nginx can keep stale backend IPs after container
restarts, leading to intermittent 502s even though a resolver was added
globally. Add dynamic DNS resolution for upstream servers (for example via
`resolve` semantics) so nginx refreshes backend addresses when containers are
recreated.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40837&comment_hash=bdab5aa69fddf70a9ed971abd3331c5e4efe22c31f1b20007d428bd5468f277d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40837&comment_hash=bdab5aa69fddf70a9ed971abd3331c5e4efe22c31f1b20007d428bd5468f277d&reaction=dislike'>👎</a>
##########
docker/nginx/templates/superset.conf.template:
##########
@@ -38,7 +38,7 @@ server {
}
location ${SUPERSET_APP_ROOT}/static {
- proxy_pass http://host.docker.internal:9000; # Proxy to superset-node
+ proxy_pass http://superset-node:9000;
Review Comment:
**Suggestion:** Routing static assets to `superset-node:9000` hard-couples
nginx to the in-container webpack service and breaks the documented host-based
frontend workflow (`BUILD_SUPERSET_FRONTEND_IN_DOCKER=false`), where the dev
server runs outside that container. This will cause static asset requests to
fail when the frontend is run on the host; keep a host-reachable fallback or
make the upstream target configurable by mode. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Host-based frontend workflow serves broken static assets.
- ⚠️ Devs cannot use documented faster host webpack setup.
- ⚠️ Nginx tightly couples to in-container frontend only.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Follow the documented host-based frontend workflow by setting
`BUILD_SUPERSET_FRONTEND_IN_DOCKER` to `false` and running `npm i && npm run
dev` on the
host as described in `docs/admin_docs/installation/docker-compose.mdx` lines
13–19; this
starts the webpack dev server on `http://localhost:9000` outside Docker.
2. Bring up the backend stack with `docker compose up` using
`docker-compose.yml`, which
starts the `nginx` service at lines 48–76, mounting `/etc/nginx/templates`
from
`./docker/nginx/templates` (line 63), so
`docker/nginx/templates/superset.conf.template`
is used to generate the virtual host.
3. In `docker/nginx/templates/superset.conf.template`, the static assets
location at lines
40–44 now reads `location ${SUPERSET_APP_ROOT}/static { proxy_pass
http://superset-node:9000; ... }`, meaning nginx always targets the
`superset-node`
service on port 9000 inside the Docker network rather than
`host.docker.internal` or any
host-configurable endpoint.
4. With the webpack dev server running only on the host (and `superset-node`
either not
running or exiting early when `BUILD_SUPERSET_FRONTEND_IN_DOCKER=false`, per
`docker/docker-frontend.sh` lines 26–46), requests to
`${SUPERSET_APP_ROOT}/static` (e.g.,
loading the Superset UI via `http://localhost:${NGINX_PORT}/`) are proxied to
`superset-node:9000` which has no active dev server, causing static asset
requests to fail
(502/404) instead of reaching the host-based frontend.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b9952192a6a94e37a5a7b325c64f6aa6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=b9952192a6a94e37a5a7b325c64f6aa6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** docker/nginx/templates/superset.conf.template
**Line:** 41:41
**Comment:**
*Api Mismatch: Routing static assets to `superset-node:9000`
hard-couples nginx to the in-container webpack service and breaks the
documented host-based frontend workflow
(`BUILD_SUPERSET_FRONTEND_IN_DOCKER=false`), where the dev server runs outside
that container. This will cause static asset requests to fail when the frontend
is run on the host; keep a host-reachable fallback or make the upstream target
configurable by mode.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40837&comment_hash=fcd7c74baf0da78475517eb4b5cd988e925c025289a0be5450e138960968ec06&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40837&comment_hash=fcd7c74baf0da78475517eb4b5cd988e925c025289a0be5450e138960968ec06&reaction=dislike'>👎</a>
##########
docker/.env:
##########
@@ -22,13 +22,13 @@ COMPOSE_PROJECT_NAME=superset
DEV_MODE=true
# Port configuration (override in .env-local for multiple instances)
-# NGINX_PORT=80
-# SUPERSET_PORT=8088
-# NODE_PORT=9000
-# WEBSOCKET_PORT=8080
-# CYPRESS_PORT=8081
-# DATABASE_PORT=5432
-# REDIS_PORT=6379
+NGINX_PORT=80
+SUPERSET_PORT=8088
+NODE_PORT=9000
+WEBSOCKET_PORT=8080
+CYPRESS_PORT=8081
+DATABASE_PORT=5432
+REDIS_PORT=6379
Review Comment:
**Suggestion:** `SUPERSET_PORT`, `DATABASE_PORT`, and `REDIS_PORT` are now
declared twice in the same `.env` file (once in the new "Port configuration"
block and again later), so the later declarations silently win. This makes
edits in the new block ineffective and causes unexpected port mappings; keep
each variable defined only once to avoid override-by-order behavior. [incorrect
variable usage]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Superset HTTP port overrides ignored for customized setups.
- ⚠️ Database port overrides ignored despite new config block.
- ⚠️ Redis port overrides ignored, confusing multi-instance configuration.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open `docker/.env` and note that `SUPERSET_PORT=8088` appears at line 26
and again at
line 77, `DATABASE_PORT=5432` appears at lines 30 and 49, and
`REDIS_PORT=6379` appears at
lines 31 and 63 (confirmed in `docker/.env` lines 21–79).
2. Edit only the new "Port configuration" block at `docker/.env` lines 24–31
(e.g., set
`SUPERSET_PORT=9090`, `DATABASE_PORT=55432`, `REDIS_PORT=6380`) and leave
the later
occurrences (lines 49, 63, 77) unchanged.
3. Start the stack with `docker compose up` using `docker-compose.yml`,
which loads
`docker/.env` as an env_file for `nginx`, `db`, `redis`, and `superset` (see
`docker-compose.yml` lines 48–55, 85–90, 99–104).
4. Observe that port bindings still use the later values: `superset` ports at
`docker-compose.yml:110–113` map `${SUPERSET_PORT:-8088}:8088`, `db` at line
94 maps
`${DATABASE_PORT:-5432}:5432`, and `redis` at line 81 maps
`${REDIS_PORT:-6379}:6379`;
because Docker Compose processes `.env` in order, the later duplicate
definitions at lines
49, 63, and 77 silently override the earlier block, making edits in the new
block
ineffective.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1a1839f125e54accb9bfe0532aa92a54&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=1a1839f125e54accb9bfe0532aa92a54&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** docker/.env
**Line:** 26:31
**Comment:**
*Incorrect Variable Usage: `SUPERSET_PORT`, `DATABASE_PORT`, and
`REDIS_PORT` are now declared twice in the same `.env` file (once in the new
"Port configuration" block and again later), so the later declarations silently
win. This makes edits in the new block ineffective and causes unexpected port
mappings; keep each variable defined only once to avoid override-by-order
behavior.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40837&comment_hash=e5a144ed743c32a033d876719ca44b1268a4c8992726d24349b507fc7d01dccb&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40837&comment_hash=e5a144ed743c32a033d876719ca44b1268a4c8992726d24349b507fc7d01dccb&reaction=dislike'>👎</a>
--
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]
