feat: resolve oci:// model references via llmman serve - #1769
ericcurtin wants to merge 2 commits into
Conversation
098a28a to
8784876
Compare
AlpinDale
left a comment
There was a problem hiding this comment.
Thanks for the PR! Please resolve the merge conflicts and take a look at the review comments below.
Also as an aside, I took a look through the llmman repo, and it seems to download through the daemon selected by LLMMAN_HOST, then runs a local CLI to find the files, which seems to read its own local store instead of the remote daemon's...? Please let me know if that's intentional because that'd imply remote daemons (even even local ones running under another user) won't work without shared storage.
| # extracted to a local directory, which the default HuggingFace-format | ||
| # loading then sees. Handled before the object-storage path since the | ||
| # two schemes are disjoint. | ||
| if is_oci_uri(model) or is_oci_uri(tokenizer): |
There was a problem hiding this comment.
Maybe a bit of a nit, but this seems to break when used together with S3, for example OCI model + S3 tokenizer, and S3 model + OCI tokenizer both fail.
There was a problem hiding this comment.
Thanks, fixed: model and tokenizer are now resolved independently, so oci:// and s3:// can be mixed. Rebased, with a test added in tests/test_config.py.
Lets --model and --tokenizer point at a model published as a CNCF
ModelPack OCI artifact:
aphrodite run 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)
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.
maybe_pull_model_tokenizer_for_runai is the existing rewrite hook that
runs before anything else touches model/tokenizer, so the oci:// branch
goes there. The two schemes are disjoint, so it returns early and the
object-storage path is untouched. A tokenizer naming the same reference
reuses the pull rather than fetching twice.
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>
Signed-off-by: Eric Curtin <eric.curtin@docker.com>
8784876 to
076e5c8
Compare
|
@AlpinDale Thanks, good catch. Not intentional for remote daemons: |
|
@AlpinDale Rebased and addressed your comments, PTAL when you can. Thank you! |
Summary
Adds an
oci://scheme so a model published as a CNCF ModelPack OCI artifact can be used anywhere a HuggingFace 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.
How it works
ModelConfig.maybe_pull_model_tokenizer_for_runaiis already the one rewrite hook that runs before anything else touchesmodel=/tokenizer=, so theoci://branch goes there. The two schemes are disjoint, so it dispatches and returns early -- the object-storage path is reached only when nooci://reference is present, and is otherwise bit-for-bit unchanged.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
aphrodite/transformers_utils/llmman.pyis the 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. The daemon deliberately exposes no local path, so the CLI is the documented interface;--no-pullguarantees it only reports on what/api/pullalready 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.aphrodite/transformers_utils/oci_utils.pyhandles the scheme, deliberately mirroring the shape of the neighbouringrunai_utils.py(including thestr()cast sopathlib.Pathinputs work).A pull needs both the daemon reachable and the binary on
PATH(orAPHRODITE_LLMMAN_BIN); each missing piece has its own actionable error. Neither is required unless anoci://reference is actually used.Design notes
registry/name:tagis indistinguishable from a HuggingFace repo id (org/model); guessing would silently hijack existing--model org/modeldeployments.model_weightskeeps the reference the user typed, matching what the runai path does with its URL.oci://ands3://can be combined.--tokenizernaming the same image reuses the resolved directory -- a ModelPack image is pulled whole, so the tokenizer is already there.Limitations
llmman resolvereads the local store, not the daemon's, so the daemon must share the store (same host and user, or sharedLLMMAN_MODELS).Testing
Two new files under
tests/transformers_utils/.test_llmman.py(8 cases) runs against a real HTTP server on a loopback port, not mocks, so the NDJSON streaming contract is genuinely exercised:/api/versionaccepted, a non-llmman server rejected, nothing-listening reported actionably; pull success with forwarded byte progress and the exact request body asserted; in-band error at HTTP 200; a stream ending withoutsuccess; non-OK status; a non-JSON diagnostic tolerated.test_oci_utils.py: scheme detection incl. case-insensitivity andpathlib.Path; that a HF repo id, a local path, ands3:///gs:///az://are not claimed (the regression that matters most);strip_oci_schemeround-trips; the resolve contract plus eight malformed-output cases; everyLLMMAN_HOSTform incl. wildcard-to-loopback; binary default/override and the missing-binary error; empty reference rejected without touching the daemon; and the scheme stripped before hand-off with progress wired.Verified honestly:
/api/version, pull success + progress + request body, in-band error, stream without success, resolve contract) -- all passruff formatandruff checkclean on all touched filesaphrodite.config.modelneeds torch and the full stack, unavailable here. Flagging rather than implying coverage I do not have.llmman servebacked by a real registry.