feat: serverless deploy directory - #97
Open
D1-3105 wants to merge 6 commits into
Open
Conversation
`deploy ./app.py` shipped exactly one file: packPythonFile stat'd a single path, rejected a directory and wrote one zip entry. An app that imported a helper module or read a data file of its own could not be deployed at all, even though the builder unpacks the archive, puts every top-level entry on PYTHONPATH and hands them to MLflow code_paths -- it has always been built for a project tree. packDirectory walks the whole source directory instead. --src-dir sets the root and defaults to the working directory; the entry file must live inside it, and a relative path is resolved there rather than against the working directory, because modelFile is a path *inside* the codebase -- that is what the field means on the wire and what the builder looks up in the zip. Exclusions are gitignore semantics via go-git's matcher, which the .gitignore fallback needs: a bare `*.pyc` has to match at any depth, and dockerignore matchers do not do that. .runwareignore wins outright when present rather than unioning with .gitignore, so the result stays possible to reason about. An excluded directory is pruned rather than walked, which both keeps a 300MB .venv from costing a stat per file and reproduces git's own rule that a negation cannot re-include a file whose parent directory is excluded. .env files and .git are excluded absolutely, ahead of the matcher, so no rule -- not even an explicit `!.env` -- can re-include them. The build pod unpacks whatever it is sent, and the builder keeping top-level dotfiles out of the *image* is a later step, not a promise about the build. Caps are per-file and total, and the total error names the largest files and points at .runwareignore: "your upload is too big" without saying what filled it leaves the caller hunting through a deep tree.
The public API has taken a `volumes` array on app create for a while and the deployer renders each entry as a hostPath mount, but the CLI had no way to say so -- which made it unusable for any app that downloads weights. Anything fetched at runtime belongs on a volume: the app runs in a sandbox whose filesystem is part of the checkpointed state, so an unmounted download is copied into every checkpoint AND re-fetched on every cold start. --volume takes an absolute path and repeats. The path is the volume's whole identity -- there is no name, because the path is what the app opens and what the node-local directory is keyed by -- so two entries resolving to the same place are a mistake rather than a merge. buildVolumes mirrors the server's checks locally: absolute, not root, within the length limits, allowed characters, no duplicates, no overlaps, at most 30. The duplication is deliberate and policy.go gives the reason for its own copy: a code deployment can build for up to ninety minutes, and learning only then that two mounts overlap wastes all of it. A shared prefix is not an overlap -- /data/weights-old sits beside /data/weights, not inside it.
An app's environment is frozen into the version snapshot that create mints, and
that snapshot is what the deployer renders the worker from. Nothing produces a
second version -- /versions and /builds are GET-only, and POST /deploy says
outright that it creates no version and re-applies the existing image -- so a
variable set through the /environment-variables endpoints after the app exists
is stored, listed back by `apps env set`/`env list`, and never reaches a pod.
Verified against a live app: version 1's snapshot held environmentVariables {}
one second before HF_TOKEN was set, a redeploy of that version logged "redeploy
completed", and the workload still had no HF_TOKEN on it.
So the create request is the only route into a worker, and the CLI had no way to
populate it. --env takes KEY=VALUE and repeats; only the first separator splits,
because a value may legitimately contain '=' (base64 padding, a DSN).
--env-file exists because the argv form cannot hold a secret: a value passed as
--env is visible in the process list to every other user on the machine for as
long as the call runs, and the shell records it in history. It reads KEY=VALUE
lines, skips blanks and comments, and absorbs a leading `export ` so a
shell-sourced file can be pasted in as-is. An inline --env wins over a file
entry of the same name, being the more specific statement.
Names are validated against the server's EnvironmentVariableName rule so a name
this accepts is one the API can store, rather than a 422 after the archive has
already been uploaded.
|
Important Review skippedAuto reviews are disabled on this repository. To trigger a review, include ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds directory-based serverless deployments with ignore rules, environment variables, and persistent volumes.
Changes:
- Packages complete source directories with
.runwareignoresupport and size limits. - Adds deploy-time environment-variable and volume configuration.
- Expands tests, documentation, and dependencies.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
internal/cmd/serverless/volume.go |
Validates volume mount paths. |
internal/cmd/serverless/volume_test.go |
Tests volume validation. |
internal/cmd/serverless/pack.go |
Implements directory packaging and ignore rules. |
internal/cmd/serverless/pack_test.go |
Tests directory archive behavior. |
internal/cmd/serverless/envvar.go |
Parses deploy environment variables. |
internal/cmd/serverless/envvar_test.go |
Tests environment-variable parsing. |
internal/cmd/serverless/deploy.go |
Adds deployment flags and request propagation. |
internal/api/serverless/client.go |
Exposes the generated volume type. |
go.mod |
Adds gitignore support dependencies. |
go.sum |
Updates dependency checksums. |
docs/runware_serverless_deploy.md |
Documents the new deployment workflow. |
Suppressed comments (4)
internal/cmd/serverless/pack.go:299
- The model-file exception does not work when an ignore rule matches one of its parent directories. For example, with
modelFileRel == "src/app.py"andsrc/in.runwareignore,WalkDirprunessrchere before it can visit the model file, so the resulting deployment cannot contain its required entry point. Keep descending when the ignored directory is an ancestor ofmodelFileRel, while still excluding its other contents.
if rel != modelFileRel && matcher.Match(segments, d.IsDir()) {
// Pruning the directory rather than descending is what keeps a
// .venv from costing a stat per file. It also reproduces git's own
// rule -- "it is not possible to re-include a file if a parent
// directory of that file is excluded" -- so a `!node_modules/keep.js`
internal/cmd/serverless/pack.go:313
- A symlink used as the model file passes
os.StatinrelativeModelFile, butWalkDirreports the entry as a symlink and this branch silently omits it. If any other file is packed, deployment proceeds with amodelFilepath absent from the archive and fails later in the builder. Either securely pack a resolved in-tree target or reject a symlink model file during local validation.
if !d.Type().IsRegular() {
return nil
internal/cmd/serverless/envvar.go:111
- The API's
maxLengthand this error message define this limit in characters, butlen(value)counts UTF-8 bytes. Valid non-ASCII values can therefore be rejected well below 4096 characters—for example, 4096écharacters are 8192 bytes. Count runes instead so local validation matches the API contract.
case len(value) > maxEnvValueLen:
return "", "", fmt.Errorf("value for %q exceeds %d characters", name, maxEnvValueLen)
internal/cmd/serverless/pack.go:404
- The aggregate size is checked only from the earlier
Statresults. Files can grow after the walk while remaining below the 10 MiB per-file cap; for example, three files measured at 8 MiB can each grow to 10 MiB and produce a 30 MiB payload despite the 25 MiB total limit. Track the bytes copied across entries and enforcemaxPackTotalByteswhile writing as well.
// Capped in case the file grew between the walk and here, so a file that
// changes underneath us cannot defeat the limit checked above.
written, err := io.Copy(w, io.LimitReader(src, maxPackEntryBytes+1))
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
danmrichards
requested changes
Aug 21, 2026
Renames and consolidation:
* envvar.go is gone. Its create-time helpers now live in env.go beside the
`apps env` command group, matching internal/api/serverless/env.go, with a
divider recording why both belong in one file: the commands change what the
API reports, and only the create request reaches a running worker.
Correctness:
* The model file survives an ignore rule that matches one of its ANCESTORS.
The exemption keyed on the file's own path, but WalkDir meets the directory
first and prunes it, so `dist/` plus a model file in dist/ produced an
archive without its entry point. Ancestor directories are now descended into
while everything else inside them stays excluded.
* A symlinked model file is rejected instead of silently dropped. It passed the
local stat and was then skipped by the walk as a non-regular file, uploading
an archive whose declared entry point was absent -- a 422 from the builder
after the upload rather than a message here.
* Ignore lines reach the parser verbatim, CR aside. Trimming rewrote valid
gitignore patterns: an escaped trailing space lost its escape and a leading
space is part of the pattern. Blanks and comments are ParsePattern's business.
* File modes are preserved with zip.FileInfoHeader + CreateHeader. zw.Create
writes mode 0, so an entrypoint script or helper binary in a codebase arrived
without its executable bit and failed at run time with nothing about the
archive to explain it.
* The archive total is enforced while writing, not only from the walk's stats.
Files can grow between being measured and being read, and several staying
under the per-file cap can still cross the total.
* --env-file no longer trims the assignment. TrimSpace ran on the whole line,
so it silently rewrote values; the trimmed copy now only decides whether a
line carries an assignment at all.
* One matching pair of surrounding quotes is stripped from a value. Files
routinely quote them and shells strip them when sourcing, so `HF_TOKEN="hf_x"`
was sending the quotes as part of the token -- a 401 in the pod with nothing
in the logs. Unbalanced or inner quotes are left alone.
* Name and value limits count runes. The API's maxLength is characters, so len()
rejected a valid non-ASCII value at half the documented limit.
Every fix has a test, including the reviewer's own case for the excluded-directory
model file across gitignore, runwareignore and built-in default rules.
Per review: what a project keeps out of version control is a different question from what it ships to a builder. A generated asset the app needs at run time is a routine .gitignore entry, and a file silently missing from a deployment because of a rule written for git is a surprise that cannot be debugged from the outside -- the archive is already uploaded by then. So exclusions are opt-in and local to .runwareignore. The built-in defaults stay: they cover output nobody means to ship (__pycache__, *.pyc, .venv, node_modules, the tool caches) and a project rule can still override them. .env and .git remain absolutely excluded ahead of the matcher. .gitignore itself now ships like any other file -- it is not special, just not obeyed -- which the replacement test asserts alongside the rules being ignored.
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.
Support directory deployment
Env vars propagation using deploy target
.runwareignorevolumes