Skip to content

[INFRA] feat: respect pre-set port env vars when initializing worktree ports#6558

Open
ntilwalli wants to merge 2 commits into
comet-ml:mainfrom
ntilwalli:feat/respect-port-env-vars
Open

[INFRA] feat: respect pre-set port env vars when initializing worktree ports#6558
ntilwalli wants to merge 2 commits into
comet-ml:mainfrom
ntilwalli:feat/respect-port-env-vars

Conversation

@ntilwalli
Copy link
Copy Markdown

Details

init_worktree_ports() in scripts/worktree-utils.sh unconditionally overwrites NGINX_PORT (and every sibling port) with BASE_* + PORT_OFFSET, so callers can't pin a single port via env (e.g. when 5173 is already taken on the host by another local app that can't move). The compose file already plumbs NGINX_PORT through to the frontend container's port mapping, healthcheck, and nginx config — only the unconditional overwrite blocks it from working.

This PR switches each port assignment in init_worktree_ports to the ${VAR:-default} idiom so pre-set env vars win while preserving the worktree-offset behavior for everything else. It also:

  • Fixes OPIK_REVERSE_PROXY_URL's default in docker-compose.yaml to follow ${NGINX_PORT:-5173} instead of hardcoding 5173, so the python-backend can still reach the frontend on a non-default port.
  • Adds a short "Changing the Frontend Port" section to deployment/docker-compose/README.md documenting NGINX_PORT (and pointing at OPIK_PORT_OFFSET for the all-ports-shifted case).

Change checklist

  • User facing
  • Documentation update

Issues

  • Resolves #
  • OPIK-

AI-WATERMARK

AI-WATERMARK: yes

  • Tools: Claude Code
  • Model(s): Claude Opus 4.7
  • Scope: Code edits in scripts/worktree-utils.sh and deployment/docker-compose/docker-compose.yaml, README addition in deployment/docker-compose/README.md, commit message, and this PR description.
  • Human verification: Author dogfooded the change end-to-end with NGINX_PORT=5293 ./opik.sh against this branch — confirmed boot banner shows http://localhost:5293, frontend container binds 0.0.0.0:5293->5293/tcp, UI returns 200, /api/is-alive/ver and /api/v1/private/projects return 200 through the reverse proxy, and python-backend's rendered OPIK_REVERSE_PROXY_URL resolves to http://frontend:5293/api.

Testing

  • bash -n scripts/worktree-utils.sh — syntax check.
  • Local docker-compose run with NGINX_PORT=5293 ./opik.sh: confirmed docker ps shows 0.0.0.0:5293->5293/tcp on frontend-1, the boot banner prints http://localhost:5293, the UI loads, /api/is-alive/ver returns 200 through the proxy, and python-backend's OPIK_REVERSE_PROXY_URL env var resolves to http://frontend:5293/api (verified via docker inspect).
  • docker compose --profile opik config | grep OPIK_REVERSE_PROXY_URL — confirmed compose substitution renders correctly with both NGINX_PORT set and unset.
  • Default invocation (./opik.sh with no env overrides) is unchanged — ${VAR:-…} falls through to the existing BASE_* + PORT_OFFSET expression, and OPIK_REVERSE_PROXY_URL resolves to http://frontend:5173/api as before.
  • Multi-worktree use case is preserved: when no port var is pre-set, the offset still applies as today.

Documentation

Added a "Changing the Frontend Port" subsection to deployment/docker-compose/README.md covering NGINX_PORT and pointing at OPIK_PORT_OFFSET as the all-ports alternative.

…e ports

init_worktree_ports() unconditionally overwrote NGINX_PORT (and every sibling
port) with BASE + PORT_OFFSET, so callers couldn't pin a single port via env
without also disabling the worktree offset for the rest. Switch each assignment
to the ${VAR:-default} idiom so pre-set values win, fix the docker-compose
default for OPIK_REVERSE_PROXY_URL to follow NGINX_PORT, and document the knob
in the docker-compose README.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@ntilwalli ntilwalli requested review from a team as code owners April 30, 2026 02:01
@github-actions github-actions Bot added documentation Improvements or additions to documentation Infrastructure labels Apr 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation Infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant