Dev - #250
Merged
Merged
Conversation
…so you cant do this
Need to set manual cuda devices and ray env fvar to prevent ray from blovking gpu access on the deployment
When a model fails to initalzie, error propagates to user api endpoints require a ray connection
…d development mode Updated AppConfig to include environment variables for minimum Nnsight version, minimum Python version, and development mode. Refactored dependencies to utilize these new configuration options for version validation and development checks.
… cleanly
Major uplift to the CLI so it's pleasant to use from pip (not just Docker):
start.py
* Rewrote as a table-driven SERVICES registry (broker / object-store /
ray / api / dashboard). `_determine_services_to_start`, the preflight
loop, and the start loop all iterate the registry. Cuts the file from
745 to ~590 lines and removes most of the per-service duplication.
* Click options for --api-url / --broker-url / --object-store-url /
--ray-address / --ray-dashboard-port now use `envvar=` + show_envvar,
and values flow through `SessionConfig.from_environment(**overrides)`
rather than mutating os.environ.
* Startup banner prints right after the logo: NDIF version, nnsight
version, Python, GPU count + model + driver (via nvidia-smi), port
summary, and dashboard enable status.
* Dashboard is now a first-class entry in SERVICES, included whenever
NDIF_DASHBOARD_PORT is set in the env. The explicit `ndif start
dashboard` fast-path is preserved.
session.py
* `SessionConfig.from_environment` gained keyword-only overrides for
broker_url / object_store_url / api_url / ray_address /
ray_dashboard_port. CLI flag values flow through directly.
* `Session.create` accepts an optional pre-built config so the
SessionConfig is built once per `ndif start`, not twice.
* `dashboard_port` field + `dashboard` entry in the services dict,
populated when NDIF_DASHBOARD_PORT is in the env, so the dashboard is
visible to `ndif info` / `ndif stop` / get_service_port.
* New `get_service_port(session, service)` helper used by stop.py too
(kills the duplicated port_map dict).
stop.py
* `ndif stop` knows about dashboard now; `ndif stop all` stops it
alongside api / ray / broker / object-store.
cli.py + config.py
* Group-level `--env-file PATH` flag (load_dotenv override=True).
* config.py also auto-loads `./.env` from the CWD — works for wheel
installs, not just editable. Precedence:
--env-file > ./.env > <repo>/.env > shell-export > .env.example
env.py
* Converted to a click group with the existing show-cluster-env as the
default behavior. New `ndif env example` subcommand prints the
bundled .env.example template (via importlib.resources). Wheel users:
`ndif env example > .env` to bootstrap config.
doctor.py (new)
* `ndif doctor` — comprehensive health check. Six sections: Environment
(Python / ndif / nnsight versions), System (ray + redis-server +
minio on PATH, with auto-install note), Compute (GPU detection +
driver), Filesystem (session + HF cache writability), Ports (free vs
in-use-by-this-session vs in-use-by-something-else), Session
(broker / API / Ray reachability if a session is active). Exits
non-zero on any failure — handy as a CI / pre-deploy gate.
util.py
* `spawn_service(service_dir, script, env_updates, log_file, verbose)`
and `terminate_process(proc, timeout)` extracted from start.py.
Consolidates the four near-identical subprocess.Popen sites the
refactor exposed.
Packaging
* `src/ndif/.env.example` symlink to ../../.env.example, added to
`[tool.setuptools.package-data]` as `"ndif" = [".env.example"]` so
the file ships in the wheel.
.env.example
* Dropped the default `NDIF_DASHBOARD_PORT=8081` line so its presence
in the env is a clean opt-in signal for the SERVICES registry. The
start.sh / compose / Dockerfile sites all default to 8081 themselves.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collapse the previous Dockerfile + Dockerfile.dashboard + (transient)
Dockerfile.standalone into a single `docker/Dockerfile` parameterized
by `--build-arg NAME=`. Same file builds four flavors:
NAME=api api:latest (compose)
NAME=ray ray:latest (compose)
NAME=dashboard dashboard:latest (compose)
NAME=all ndif/ndif:latest (Docker Hub standalone)
Single conditional in the install step:
* NAME=all: `uv pip install --system -e .` (full pyproject deps —
api+ray+monitor+dashboard requirements wired up via
tool.setuptools.dynamic).
* Per-service: install only that service's requirements.in plus
`uv pip install -e . --no-deps`.
Both paths use editable install so cli/config.py's `load_dotenv` and
the dashboard's source-tree `frontend_dist` default resolve correctly.
Other Dockerfile changes:
* Dropped the explicit `--index-url https://...cu128` pin — torch
comes in via the package deps, whichever CUDA build PyPI ships with
resolves at build time (smaller image, no version lock).
* Always installs cron + curl + ca-certificates (cron used by the
dashboard's monitor/reconcile crons; curl/ca-certs needed by
micromamba's installer).
* Pre-bootstrap step for NAME=all: runs
`ndif.cli.lib.deps.ensure_redis() + ensure_object_store()` so the
micromamba env is baked into the image. Runtime first-start has no
download delay.
* CMD branches on RAY_ADDRESS (worker mode) vs default; the standalone
case uses NDIF_SERVICE=all → `ndif start all --verbose`.
Frontend build moves to the host so both Dockerfiles can read a
pre-built dist:
* `make dashboard-frontend` runs `npm ci && npm run build` against
`src/ndif/services/dashboard/frontend/`. File-target with
source-tree dependencies, so it's a no-op when dist/ is already
fresh.
* `check-node` prereq fails fast with a clear error if npm < 20.
* Same dist will eventually bundle into the pip wheel; building on
the host keeps the docker + pip paths aligned.
Makefile additions:
* `build-standalone` — buildx with NAME=all, tags both `:latest` and
`:VERSION` (read straight from pyproject.toml via tomllib).
* `push-standalone` — pushes both tags; reminds you to paste
docker/DOCKERHUB.md into the Docker Hub Overview tab.
* `run-standalone` — `docker run --gpus all` with port mappings + HF
cache volume.
docker-compose.yml: dashboard port mapping now uses
`${NDIF_DASHBOARD_PORT:-8081}` fallback since the .env.example
default for that key was removed (it's now an opt-in trigger).
pyproject.toml: added `services/dashboard/requirements.in` to the
dynamic `dependencies` list so `pip install ndif` covers the
dashboard's deps too (consumed by the NAME=all install path).
docker/DOCKERHUB.md (new): comprehensive Docker Hub overview —
quick-start, ports/volumes tables, dashboard enable flow, full env
var reference grouped by category, four example invocations.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…file README.md (full rewrite, ~430 lines): * pip-first quickstart (then docker, then from-source). * "What's running" service table + default ports. * CLI reference — one line per subcommand. * Configuration section: .env discovery precedence table, CLI flag overrides table, and full env vars grouped by category (General / API / Broker / Object store / Ray / Controller / Dashboard / HF) — every NDIF_* and HF_* the code actually reads. * Common workflows: deploy, auto-deploy via models.yaml, dashboard enable (with auth), worker nodes, `ndif doctor`, stop. * Architecture summary (1 page) pointing at NDIF.md for the deep dive. * Contributing section: local dev + docker dev loop + pointers to NDIF.md / CLAUDE.md / DOCKERHUB.md / service READMEs. Removed: * Python 3.10 / conda dev guide (now 3.12+, pip install -e). * `compose/dev/.env` / `compose/prod/.env` references (those paths don't exist). CLAUDE.md: `make build` description updated to point at the unified Dockerfile + the new dashboard-frontend host-build dependency; adds `make build-standalone`. NDIF.md: * File-pointer table: collapsed the Dockerfile entries to one row describing the NAME= flavors. * Build section: rewrote to describe three flavors from one Dockerfile + the dashboard-frontend host build step + the new build-standalone target. * Dashboard service summary: now points at `docker/Dockerfile` (`--build-arg NAME=dashboard`); notes the env-presence trigger that makes `ndif start all` include the dashboard. * Directory tree: dropped the Dockerfile.dashboard line. services/dashboard/README.md: updated build instructions to point at the unified Dockerfile + the host-side frontend build. services/dashboard/start.sh: comment now says "CMD in docker/Dockerfile with --build-arg NAME=dashboard" since Dockerfile.dashboard is gone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…layers Pulls in PR #256 (Multi-replica support + per-Processor autoscaling). Big diff: 5087+/2838-, 56 files, adds replica/autoscaling support across Controller, Cluster, Node, Deployment, the API queue (new `Processor` → `Replica` pool with autoscaling loop), the CLI (`--replicas N`, `--replica RID` on deploy/evict/restart), the dashboard frontend (per-replica dots + per-replica controls), and a full pytest suite under `tests/new/`. Also incorporates their Dockerfile multi-stage caching split. ## Conflict resolutions * **docker/Dockerfile** — folded their 3-layer cache split (torch / service-requirements / source) into our unified-Dockerfile layout with the `NAME=all` conditional. Result: all four build flavors (api / ray / dashboard / all) share the torch layer; the service- requirements layer copies all four `requirements.in` files so the layer key reflects every flavor, and installs them conditionally by NAME. Kept our extras: cron + curl + ca-certificates apt deps, editable install (`-e .`) so cli/config.py's load_dotenv resolves to /ndif/.env.example + the dashboard's source-tree frontend_dist default works, and the redis/minio pre-bootstrap step for NAME=all. * **docker/Dockerfile.dashboard** — modify/delete conflict; stays deleted. We unified into one Dockerfile; their changes to the separate dashboard image are no longer needed. * **src/ndif/cli/lib/util.py** — both branches added imports. Combined: kept their logging/Iterable/logger + our os/subprocess for spawn_service. Both `notify_reconcile` (theirs) and `spawn_service`/`terminate_process` (ours) coexist. * **pyproject.toml** — accepted their removal of the `services/monitor/requirements.in` entry from the dynamic deps (monitor is gone). Kept our package-data entry for .env.example + the dashboard requirements.in addition. Also retitled the stale comment that mentioned Dockerfile.dashboard (now describes the source-tree dist + editable install path). * **.env.example** — auto-merged (orthogonal sections). Both their autoscaling block (NDIF_AUTOSCALING_INTERVAL_S / WAIT_THRESHOLD_S / BACKOFF_S) and our `NDIF_DASHBOARD_PORT` comment landed. * **docker/docker-compose.yml** — auto-merged. Both their api service autoscaling env additions and our dashboard port fallback (`${NDIF_DASHBOARD_PORT:-8081}`) landed. * **NDIF.md** — auto-merged. ## Verified post-merge - All Python files compile. - `ndif --version` works (0.0.1). - `ndif doctor` runs and all checks pass. - No callers of the removed `notify_dispatcher` or `wait_for_model_ready` (renamed to `notify_reconcile` and `wait_for_replica_ready(model_key, replica_id)`). ## Follow-up Doc updates to come in the next commit: - README env vars table: add the three NDIF_AUTOSCALING_* vars + the renamed NDIF_STATUS_CACHE_FREQ_S. - README CLI reference: surface `--replicas N` / `--replica RID` on deploy/evict/restart. - DOCKERHUB.md: mirror the env-var additions. - CLAUDE.md + NDIF.md: drop references to the now-deleted `services/monitor/` directory. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Post-merge updates for the features/replicas content: README.md: * Env vars: added the three NDIF_AUTOSCALING_* entries (new section between Controller / scheduler and Dashboard) and the renamed NDIF_STATUS_CACHE_FREQ_S. * CLI reference: deploy/evict/restart now show --replicas N / --replica RID. * New "Scale a model to multiple replicas" workflow with a short example + a pointer to the autoscaling env vars. * models.yaml example gained a `replicas: 2` line. docker/DOCKERHUB.md: mirrored — NDIF_AUTOSCALING_* section + the NDIF_STATUS_CACHE_FREQ_S row. CLAUDE.md: dropped the now-defunct `src/ndif/services/monitor/` "LEGACY" bullet — the directory is gone. NDIF.md: * "Where to read" + file-pointer tables: collapsed the two monitor/dashboard rows into one, marked the monitor as removed. * §13.5 + §15.9: kept as short "(removed)" stubs that point at the body's new home (`services/dashboard/jobs/monitor.py`) and note that the old INSTALL_DIR / MONITOR_CRON env vars no longer matter. Stub instead of full deletion so cross-references from §15.1 etc. don't dangle. * TOC entry retargeted to the new anchor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Describe how things are, not how they were. Removed every mention of the legacy uptime monitor — no "(removed)" stubs, no "replaces the now-deleted X", no "was services/monitor/jobs/monitor.py" annotations. NDIF.md: * Dropped the legacy-monitor row in "Where to read" * Dropped the parenthetical on the dashboard file-pointer row * Removed the §13.5 TOC entry * Deleted §13.5 entirely * Deleted §15.9 entirely, promoting §15.10 (CLI-only) to §15.9 * Dropped the directory-tree entry for `monitor/` * Cleaned three in-text references in §13.2 (the prose around the monitor cron) and the §13 intro CLAUDE.md: * Dropped the `monitor/` directory-tree entry * Removed the "Replaces the now-deleted services/monitor/" tail from the dashboard bullet src/ndif/services/dashboard/README.md: * Dropped "Replaces the standalone services/monitor/ dashboard." from the intro * Cleaned the "same format the previous services/monitor produced" reference in the monitor-cron description * Removed the "(was services/monitor/jobs/monitor.py)" tree annotation Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-from-dev CLI: align revision default semantics with NDIF null convention
…VICE
Every NDIF service now runs from the same `ndif/ndif` image. The
old `--build-arg NAME=` per-service-image story produced four
identical-up-to-imports artifacts (each ~6-8 GB because torch
dominates) for what was always the same install. Replaced with:
* One image — `ndif/ndif:latest` + `:VERSION`. Compose + ECS + Docker
Hub all pull the same tag.
* Runtime service selection via `NDIF_SERVICE` env var. Defaults to
`all` (broker + object-store + ray + api in one container — the
standalone Docker Hub experience). docker-compose sets it per
service (`NDIF_SERVICE=api`, `NDIF_SERVICE=ray`,
`NDIF_SERVICE=dashboard`).
* `ARG NAME=all` is still accepted at build time as a back-compat
alias for callers (existing CI workflow) that bake the service into
the image — the build-arg just sets the ENV default, doesn't change
what's installed.
Dockerfile layer order tuned for cache reuse:
1. apt deps (build-essential / git / cron / curl / ca-certificates)
2. torch + torchvision (largest, rarest — kept on its own layer)
3. COPY of just the four `requirements.in` files + uv pip install
4. COPY of the full source + editable install --no-deps
5. Pre-bootstrap of redis + minio via micromamba
Source edits only invalidate layer 4 (which is a cheap metadata
install). Editing a requirements.in invalidates layer 3 but not 1-2.
docker-compose.yml:
* All three NDIF services (ray, api, dashboard) reference
`${NDIF_IMAGE:-ndif/ndif:latest}` instead of separate
`api:VERSION` / `ray:VERSION` / `dashboard:VERSION` tags.
* Each sets `NDIF_SERVICE` in its `environment:` block.
* Makefile exports `NDIF_IMAGE` before invoking `docker compose`.
Makefile:
* `make build` builds one image (`ndif/ndif:latest` + `:VERSION`).
* Renamed: `build-standalone` → (gone, build does both),
`push-standalone` → `push`, `run-standalone` → `run`.
* `make up` / `down` / `ta` unchanged in behavior; they now export
`NDIF_IMAGE` for compose.
.env.example: dropped `NDIF_DASHBOARD_VERSION` (no per-service tags).
Docs: README, CLAUDE.md, NDIF.md updated to describe one image with
runtime `NDIF_SERVICE` selection. DOCKERHUB.md was already
single-image and needed no change.
## Verified
- `make build` succeeds and produces ndif/ndif:latest + :0.0.1
(8.68 GB; cu128 torch + redis + minio baked via micromamba).
- `docker run --rm ndif/ndif:latest ndif --version` → 0.0.1.
- `NDIF_SERVICE` defaults to `all` inside the image.
- Pre-bootstrap landed redis-server + minio in
/root/.ndif/micromamba/envs/ndif-deps/bin/.
- `docker compose -f docker/docker-compose.yml config` resolves all
three NDIF services to `ndif/ndif:latest` with the right
`NDIF_SERVICE` env per service.
## Reviewer note
The existing CI workflow (`.github/workflows/build_images.yml`)
passes `--build-arg NAME=api` and `NAME=ray` and pushes to two
separate ECR repos. With this change those produce identical
content under different tags. Still functional (ECS pulls each tag
and the baked-in NDIF_SERVICE makes the entrypoint do the right
thing), but the workflow could be simplified to a single push in a
follow-up.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Prevents the per-model autoscaling loop from growing the replica pool without bound under sustained queue pressure. Defaults to 3. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Multi-replica support + per-Processor autoscaling
…r tables Final reconciliation sweep across .env.example, README, and DOCKERHUB after the dev merge. Two real gaps: * `NDIF_AUTOSCALING_MAX_REPLICAS` (default 3, introduced in 768beab) was in .env.example but missing from both README and DOCKERHUB. Added to the Autoscaling section in both. * `NDIF_SESSION_ROOT` was in README's General section but missing from DOCKERHUB. Added there for parity. Verified after edits: * Every NDIF_* the code reads (via os.environ / get_env / pydantic-settings) appears in at least one of .env.example / README / DOCKERHUB. * README and DOCKERHUB now cover the same set of NDIF_* names. * No stale `COORDINATOR_*`, `NDIF_DASHBOARD_VERSION`, `API_VERSION`, or `RAY_VERSION` references anywhere. * All documented defaults still match the code (broker URL, status cache freq, autoscaling intervals, etc). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CLAUDE.md: mention `ndif env example`, `ndif doctor`, and the group-level `--env-file PATH` flag in the CLI overview line that lists available commands. NDIF.md: compose service-image table now shows `ndif/ndif:latest (NDIF_SERVICE=ray|api|dashboard)` instead of the dead `ray:latest`/`api:latest`/`dashboard:latest` per-service tags. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The two NAME=api / NAME=ray builds were producing identical content
under different tags now that the Dockerfile is unified. Replaces
them with one build pushed to <env>/ndif. The ECS task defs (in
ndif-aws) decide what each container runs via the NDIF_SERVICE env
var, so one image is enough.
Workflow: 110 → 83 lines.
## Required ndif-aws change (separate PR)
ECS task defs need updating before the next merge to dev / main:
* Both api and ray services: change image source from
`<account>.dkr.ecr.us-east-1.amazonaws.com/{env}/api:latest` (and
`/ray:latest`) to `<account>.dkr.ecr.us-east-1.amazonaws.com/{env}/ndif:latest`.
* Add a container `environment` entry: `NDIF_SERVICE=api` for the api
task def, `NDIF_SERVICE=ray` for the ray task def. Overrides the
image's baked-in default of `NDIF_SERVICE=all`.
Without that update, the existing api / ray ECR repos stop receiving
new pushes from this workflow — production keeps running on the
current image, so nothing breaks, but updates won't ship.
The `ARG NAME=all` line in `docker/Dockerfile` is left in place as a
defensive back-compat hatch for any other external caller that still
passes `--build-arg NAME=...` to set a per-image NDIF_SERVICE default.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Standalone Docker image + CLI uplift (doctor, env discovery, banner)
replace jaeger backend with grafana tempo
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.