feat: resolve oci:// model references via llmman serve - #672
Conversation
Lets --model-id point at a model published as a CNCF ModelPack OCI
artifact:
infinity_emb v2 --model-id oci://ghcr.io/org/model:tag
Model distribution is increasingly moving to OCI registries, which lets
a deployment reuse the registry, credentials, mirroring and air-gap
tooling it already has for container images.
Acquisition is delegated to a running `llmman serve`, which already
implements the ModelPack media types, registry auth, resumable blob
download and a content-addressed store. The daemon does the pull (POST
/api/pull, streamed as NDJSON so a multi-gigabyte fetch is not silent,
and an error arriving in-band at HTTP 200 is caught) but deliberately
exposes no local path, so `llmman resolve --no-pull` reports where the
bytes landed. The client is stdlib-only, so no new dependency.
EngineArgs.__post_init__ is the single dispatch point, resolved first so
the rest of that method and the loading strategy only ever see a local
path -- every engine then loads it exactly as it would a local
directory. served_model_name keeps the reference the user typed rather
than the store path, unless one was given explicitly.
An explicit oci:// scheme is required rather than sniffing a bare
registry/name:tag: that shape is indistinguishable from a HuggingFace
repo id, so guessing would silently hijack existing deployments.
Signed-off-by: Eric Curtin <eric.curtin@docker.com>
Greptile SummaryAdds transparent resolution of
Confidence Score: 2/5The PR should not merge until HTTPS endpoint handling and bounded deadlines for both daemon pulls and CLI resolution prevent startup failures and indefinite hangs. OCI acquisition runs synchronously during engine construction, while configured transport schemes are discarded and both long-running acquisition stages can block forever when their external counterpart stalls. Files Needing Attention: libs/infinity_emb/infinity_emb/llmman.py
|
| Filename | Overview |
|---|---|
| libs/infinity_emb/infinity_emb/args.py | Adds synchronous OCI resolution at the start of EngineArgs initialization while preserving the typed reference as the served name. |
| libs/infinity_emb/infinity_emb/llmman.py | Implements daemon and CLI acquisition, but discards configured HTTPS schemes and leaves both pull streaming and resolver execution unbounded. |
| libs/infinity_emb/infinity_emb/oci.py | Detects and strips the explicit OCI scheme, validates nonempty references, and delegates acquisition with progress logging. |
| libs/infinity_emb/tests/unit_test/test_llmman.py | Covers daemon identity and NDJSON outcomes but does not exercise stalled operations or deadlines. |
| libs/infinity_emb/tests/unit_test/test_oci.py | Covers scheme routing and common host forms but omits scheme-bearing LLMMAN_HOST values such as HTTPS. |
Sequence Diagram
sequenceDiagram
participant CLI as CLI / Engine caller
participant Args as EngineArgs
participant OCI as oci.resolve
participant Daemon as llmman daemon
participant Bin as llmman CLI
CLI->>Args: Construct with oci:// reference
Args->>OCI: Resolve bare registry reference
OCI->>Daemon: GET /api/version
OCI->>Daemon: POST /api/pull (NDJSON)
Daemon-->>OCI: progress and success
OCI->>Bin: resolve --no-pull reference
Bin-->>OCI: local model path
OCI-->>Args: resolved path
Args-->>CLI: configured local model
Reviews (1): Last reviewed commit: "feat: resolve oci:// model references vi..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if "://" in raw: | ||
| raw = raw.split("://", 1)[1] |
There was a problem hiding this comment.
If LLMMAN_HOST specifies an HTTPS daemon, endpoint() removes that scheme and unconditionally produces an http:// URL, causing OCI model initialization to fail against a TLS-only endpoint or silently dropping the configured transport protection.
|
|
||
| succeeded = False | ||
| try: | ||
| with urllib.request.urlopen(req) as resp: |
There was a problem hiding this comment.
If the daemon accepts /api/pull but stalls its response or leaves the NDJSON stream open without a terminal status, this timeout-free urlopen and response loop block synchronous EngineArgs construction indefinitely, preventing the application server from completing startup.
| completed = subprocess.run( | ||
| [binary, "resolve", "--no-pull", reference], | ||
| capture_output=True, | ||
| stdin=subprocess.DEVNULL, | ||
| text=True, | ||
| check=False, | ||
| ) | ||
| if completed.returncode != 0: |
There was a problem hiding this comment.
If llmman resolve --no-pull hangs because of daemon, lock, or filesystem contention, this subprocess.run has no timeout and blocks synchronous model initialization indefinitely, so the application server never becomes ready or reports a resolver failure.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54737be9db
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| base = endpoint() | ||
| check_daemon(base) | ||
| logger.info("Pulling %s via llmman daemon at %s", reference, base) | ||
| pull(base, reference, progress) |
There was a problem hiding this comment.
When the daemon is reachable but llmman is absent or INFINITY_LLMMAN_BIN is misconfigured, this starts and may complete a multi-gigabyte registry pull before resolve() performs the binary check and inevitably fails startup. Preflight the executable after probing the daemon but before beginning the expensive pull.
Useful? React with 👍 / 👎.
1 participant
What
Adds an
oci://scheme so a model published as a CNCF ModelPack artifact can be used anywhere a Hugging Face repo id can:Model distribution is increasingly moving to OCI registries -- the same registries, credentials, mirroring and air-gap tooling a deployment already uses for container images. Usually easier to run air-gapped than reaching the Hub.
How
EngineArgs.__post_init__is the single dispatch point. Resolution runs first, so the rest of that method --served_model_namederivation,vector_disk_cache_path, the loading strategy, and every engine downstream -- only ever sees a local path and needs no changes. Each engine then loads it exactly as it would a local directory.One deliberate detail:
served_model_namekeeps the reference the user typed (oci://ghcr.io/org/model:tag) rather than the resolved store path, since the path is an implementation detail of llmman's cache. An explicitly-passed--served-model-namestill wins.Acquisition is delegated to a running
llmman serverather than hand-rolled: llmman already implements the ModelPack media types, registry auth, resumable blob download and a content-addressed store.New
infinity_emb/llmman.pyis the daemon client, stdlib-only (urllib), no new dependency:GET /api/versionprobes reachability and identity -- a server answering without aversionfield is reported as "not an llmman daemon", worth distinguishing from nothing listening.POST /api/pullstreams NDJSON so a multi-gigabyte fetch is not silent. An error arrives in-band at HTTP 200, and a stream that ends withoutsuccessis also a failure -- both are errors, not a completed pull.llmman resolve --no-pullreports where the bytes landed;--no-pullguarantees it only reports on what the pull already fetched, keeping the daemon the only thing that touches the network.LLMMAN_HOSTis honoured with llmman's own parsing, including rewriting a wildcard bind (0.0.0.0,[::]) to loopback.A pull needs both the daemon reachable and the binary on
PATH(orINFINITY_LLMMAN_BIN); each missing piece has its own actionable error, and neither is required unless anoci://id is used.Design notes
registry/name:tagis indistinguishable from a Hugging Face repo id (michaelfeil/bge-small-en-v1.5); guessing would silently hijack existing--model-id org/modeldeployments. Every other id shape reaches exactly the branch it did before.resolvestdout is used; unknown JSON fields are ignored so the contract can grow.Testing
Two new files under
libs/infinity_emb/tests/unit_test/.test_llmman.pyruns against a real HTTP server on a loopback port, not mocks, so the NDJSON contract is genuinely exercised.24 passed, executed here. Coverage: scheme detection incl. case-insensitivity; that a HF repo id, a local path and
s3://are not claimed;strip_schemeround-trips; empty reference rejected; the bare reference handed to the daemon with progress wired; everyLLMMAN_HOSTform incl. wildcard-to-loopback;/api/versionaccepted / non-llmman rejected / nothing-listening actionable; pull success with byte progress and the exact request body; in-band error at HTTP 200; stream ending withoutsuccess; non-OK status; non-JSON diagnostic tolerated.3 failed here for an environment reason, not a defect -- the
TestEngineArgsIntegrationcases construct a realEngineArgs, which imports torch (ImportError: torch.nn is not available), unavailable in this environment. They assert thatmodel_name_or_pathis rewritten, thatserved_model_namekeeps the typed reference, that an explicit served name wins, and that a plain HF repo id never reaches the resolver. They should pass in CI; flagging rather than quietly deleting them.ruff formatandruff checkclean on all four new/changed files.args.pyreports 4 ruff findings both before and after this change (verified against a stashed clean tree), so none are introduced here. An earlier format run also touchedtests/unit_test/inference/test_batch_handler.py; I reverted it so the diff stays scoped.llmman servebacked by a real registry.