fix: mount the model registry from MODEL_CONFIG, not the git checkout#36
Open
JasonWildMe wants to merge 1 commit into
Open
fix: mount the model registry from MODEL_CONFIG, not the git checkout#36JasonWildMe wants to merge 1 commit into
JasonWildMe wants to merge 1 commit into
Conversation
docker-compose.prod.yml mounted the registry from inside the working tree:
- ../app/model_config.json:/app/app/model_config.json:ro
It was the only volume not sourced from .env, and it made `git` a delete button.
The repo's app/model_config.json is a 5-model development stub; a real deployment
serves a different, much larger registry. Keeping the live file at a tracked path
means it exists only as an uncommitted local edit, so `git checkout`, `reset
--hard`, `stash`, or a pull touching that path silently replaces it with the stub
-- and the service then fails to start on the next restart, taking down every
install that shares it.
This is not hypothetical. On 2026-07-16 a shared ml-service was reverted to main
to back out an unrelated code change; git restored its own 5-model stub over the
49-model production registry, and the service died on a stub entry whose weights
do not exist on that host. The revert was the right call for the original
problem; this line turned it into a second outage. The registry survived only
because a copy happened to exist on someone's laptop -- it is in no git history.
- "${MODEL_CONFIG:?Set MODEL_CONFIG in .env}:/app/app/model_config.json:ro"
Now consistent with MODELS_DIR and DATA_DB_DIR: sourced from .env, and `:?` fails
at compose time with an actionable message rather than silently mounting the stub.
Verified with docker compose: rejected when unset, resolves when set.
Deliberately required rather than defaulted to the old path. A default would keep
the landmine armed for anyone who does not read the release note, and the failure
it prevents is a multi-install outage. The cost is that each deployment must set
MODEL_CONFIG before its next restart -- a compose error naming the variable, which
is far better than a mysterious stub crash.
Also keeps the registry private: it never has to be committed to be deployed.
Documented in three places, because the old README actively taught the bad
practice ("Edit app/model_config.json"):
- docker/_env: MODEL_CONFIG as required, with the reason
- README: set it in .env; create the registry outside the checkout
- app/model_config.json: a _note marking it a dev stub and warning against
in-place server edits (main.py reads only "models", so the key is ignored)
Migration:
sudo mkdir -p /data/ml-service
sudo cp <checkout>/app/model_config.json /data/ml-service/model_config.json
echo 'MODEL_CONFIG=/data/ml-service/model_config.json' >> docker/.env
# restart; confirm /health reports the expected models_loaded
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
Stops
gitfrom being able to delete a deployment's model registry.The problem
docker-compose.prod.ymlmounted the registry from inside the working tree — the only volume not sourced from.env:The repo's
app/model_config.jsonis a 5-model development stub. A real deployment serves a different, much larger registry. Because the mount points at a tracked path, the live file exists only as an uncommitted local edit — in no git history at all.That makes routine git a delete button:
git checkout,reset --hard,stash, or a pull touching that path silently replaces the live registry with the stub. The service then fails to start on the next restart, taking down every install that shares it.This already happened
On 2026-07-16 a shared ml-service was reverted to
mainto back out an unrelated code change. Git restored its own 5-model stub over the 49-model production registry, and the service died on a stub entry (miewid-trout) whose weights don't exist on that host — after the revert had been made specifically to restore service.The registry survived only because a copy happened to sit on someone's laptop.
The fix
- "${MODEL_CONFIG:?Set MODEL_CONFIG in .env}:/app/app/model_config.json:ro"Now consistent with
MODELS_DIRandDATA_DB_DIR. Verified with realdocker compose: rejected when unset, resolves when set.:?errors at compose time naming the variable, instead of silently mounting the stubRequired, not defaulted — deliberately
Defaulting to the old path would keep the landmine armed for anyone who doesn't read the release note, and the failure it prevents is a multi-install outage.
Cost: every deployment must set
MODEL_CONFIGbefore its next restart, or compose refuses to start. That's a loud error naming the exact variable — strictly better than a mysterious 5-model stub crash, which is what happens today.Documentation
The old README actively taught the bad practice ("Edit
app/model_config.json"), so this is fixed in three places:docker/_env—MODEL_CONFIGas required, with the reasonREADME.md— set it in.env; create the registry outside the checkoutapp/model_config.json— a_notemarking it a dev stub and warning against in-place server edits (main.pyreads only"models", so the key is ignored)Migration
What this does NOT do
It isn't a backup. It stops git eating the registry; it doesn't protect against losing the host. A file that five installs depend on, exists in no version control, and has one off-host copy is still a single point of failure — worth solving separately, mindful that the registry is not something to make public.
Related: per-install model configuration would also bound the blast radius, since
main.pyeager-loads every entry regardless of which install references it.🤖 Generated with Claude Code