Skip to content

Commit 4548715

Browse files
authored
feat(comfy-cli): push prints its save's warnings and holds a release on a model link warning (BE-15875) (#901)
1 parent c08aded commit 4548715

11 files changed

Lines changed: 280 additions & 12 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ history.
1717

1818
### Added
1919

20+
- `comfy build push` prints every warning a save returns, and `--release` cuts no
21+
release while one says a deployment could not download a model link
22+
(`build_release_held`); `--release-despite-warnings` cuts anyway.
2023
- `comfy build release delete RELEASE` deletes the named release, freeing the slot
2124
it held against the workspace's release limit. It confirms first (`--yes` skips
2225
the prompt, `build_release_delete_needs_confirm` refuses a caller that cannot

comfy_cli/builder_api.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,13 @@ def upload_blob(self, upload_url: str, path: Path) -> None:
118118
)
119119
resp.raise_for_status()
120120

121-
def create_build(self, name: str, definition: dict, description: str | None = None) -> str:
122-
"""Create a build from a definition. Returns its id."""
121+
def create_build_response(self, name: str, definition: dict, description: str | None = None) -> dict:
122+
"""Create a build from a definition. Returns the created build, carrying
123+
the ``warnings`` the save earned, which a read never returns."""
123124
body: dict = {"name": name, "definition": definition}
124125
if description:
125126
body["description"] = description
126-
return self._post(("builds",), body)["id"]
127+
return self._post(("builds",), body)
127128

128129
def create_release(self, build_id: str, targets: list[dict] | None = None) -> tuple[str, str]:
129130
"""POST /v1/builds/{id}/releases: freeze the definition and enqueue a

comfy_cli/command/build.py

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1855,6 +1855,13 @@ def push_cmd(
18551855
bool,
18561856
typer.Option("--force", help="Overwrite remote changes, retrying a bounded GET-then-PATCH."),
18571857
] = False,
1858+
release_despite_warnings: Annotated[
1859+
bool,
1860+
typer.Option(
1861+
"--release-despite-warnings",
1862+
help="With --release, cut the release even though the save warned about a model link.",
1863+
),
1864+
] = False,
18581865
dry_run: Annotated[bool, typer.Option("--dry-run", help="Compute uploads locally; send no HTTP requests.")] = False,
18591866
models_dir: Annotated[
18601867
str | None,
@@ -1875,6 +1882,14 @@ def push_cmd(
18751882
details={"conflict": ["--release", "--dry-run"]},
18761883
)
18771884
raise typer.Exit(code=1)
1885+
if release_despite_warnings and not release:
1886+
renderer.error(
1887+
code="build_missing_input",
1888+
message="--release-despite-warnings applies only to the release --release cuts.",
1889+
hint="pass --release to cut a release despite the save's model link warnings",
1890+
details={"missing": ["--release"]},
1891+
)
1892+
raise typer.Exit(code=1)
18781893
targets = _parse_release_targets(renderer, target or ())
18791894
if targets and not release:
18801895
renderer.error(
@@ -1991,8 +2006,14 @@ def push_cmd(
19912006
name = str(spec["name"])
19922007
description = str(spec["description"])
19932008
if target_id is None:
1994-
target_id = _builder_call(renderer, lambda: client.create_build(name, wire_definition, description))
1995-
saved = _builder_call(renderer, lambda: client.get_build(target_id))
2009+
created_build = _builder_call(
2010+
renderer, lambda: client.create_build_response(name, wire_definition, description)
2011+
)
2012+
target_id = created_build["id"]
2013+
saved = {
2014+
**_builder_call(renderer, lambda: client.get_build(target_id)),
2015+
"warnings": created_build.get("warnings"),
2016+
}
19962017
created = True
19972018
elif force:
19982019
saved = _force_update(renderer, client, target_id, wire_definition, name, description)
@@ -2028,6 +2049,12 @@ def push_cmd(
20282049
"deduped": len(uploads) - uploaded,
20292050
}
20302051
)
2052+
warnings = _save_warnings(saved)
2053+
if warnings:
2054+
payload["warnings"] = warnings
2055+
_hold_release_on_link_warnings(
2056+
renderer, warnings, target_id, saved["updatedAt"], release and not release_despite_warnings
2057+
)
20312058
release_summary: dict[str, str] | None = None
20322059
if release:
20332060
requested = [item.as_wire() for item in targets]
@@ -2050,6 +2077,50 @@ def push_cmd(
20502077
renderer.emit(payload, command="build push", changed=True)
20512078

20522079

2080+
# A warning at this field is comfy-builder's for a model link a deployment could
2081+
# not download; it is the whole contract a held release rests on. Any other
2082+
# field, such as pipDependencies, is printed and never holds a release.
2083+
_MODEL_LINK_FIELD = re.compile(r"models\[\d+\]\.sourceUri")
2084+
2085+
2086+
def _save_warnings(saved: dict) -> list[dict[str, str]]:
2087+
"""The warnings a save returned, as field and reason, dropping anything the
2088+
builder sent in another shape rather than failing a push that landed."""
2089+
raw = saved.get("warnings")
2090+
if not isinstance(raw, list):
2091+
return []
2092+
return [
2093+
{"field": item["field"], "reason": item["reason"]}
2094+
for item in raw
2095+
if isinstance(item, dict) and isinstance(item.get("field"), str) and isinstance(item.get("reason"), str)
2096+
]
2097+
2098+
2099+
def _hold_release_on_link_warnings(
2100+
renderer, warnings: list[dict[str, str]], build_id: str, revision: str, cutting: bool
2101+
) -> None:
2102+
"""Print every warning the save returned, and refuse the cut a push asked for
2103+
while one is about a model link a deployment could not download. The build is
2104+
already saved and the spec already carries its revision, so a second push with
2105+
the go-ahead option cuts without saving anything twice."""
2106+
for warning in warnings:
2107+
renderer.warn(f"{warning['field']}: {warning['reason']}")
2108+
held = [warning for warning in warnings if _MODEL_LINK_FIELD.fullmatch(warning["field"])]
2109+
if not cutting or not held:
2110+
return
2111+
details: dict = {"id": build_id, "syncedRevision": revision}
2112+
# Text mode has already printed each warning above; only JSON carries them again.
2113+
if not renderer.is_pretty():
2114+
details["warnings"] = warnings
2115+
renderer.error(
2116+
code="build_release_held",
2117+
message=f"saved build {build_id}, but cut no release: the save warned that a deployment could not "
2118+
"download " + ", ".join(warning["field"] for warning in held),
2119+
details=details,
2120+
)
2121+
raise typer.Exit(code=1)
2122+
2123+
20532124
def _prompt_build_id(renderer, client) -> str | None:
20542125
from comfy_cli.ui import prompt_select
20552126

comfy_cli/error_codes.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1235,6 +1235,13 @@ class ErrorCode:
12351235
"reconstruct every requested public node.",
12361236
"edit the spec to name a published registry version or normalized repository, or remove the node",
12371237
),
1238+
ErrorCode(
1239+
"build_release_held",
1240+
"`comfy build push --release` saved the build, but the save warned about a model link a deployment "
1241+
"could not download, so no release was cut. `details` carries the saved `id`, `syncedRevision` and "
1242+
"every `warnings` entry; a warning at `models[<n>].sourceUri` is the one that holds a release.",
1243+
"fix the model links and push again, or push with --release --release-despite-warnings to cut anyway",
1244+
),
12381245
ErrorCode(
12391246
"build_release_limit",
12401247
"The builder refused the release cut because the workspace already holds as many releases as its "

comfy_cli/schemas/build_push.json

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,20 @@
2525
"upload_bytes": {"type": "integer", "minimum": 0},
2626
"uploaded": {"description": "Blobs whose bytes were transferred. Never exceeds `upload_count`: the builder answers with an id it already holds, and no upload URL, when the workspace already stored those exact bytes.", "type": "integer", "minimum": 0},
2727
"deduped": {"description": "Blobs the builder already held, so `upload_count` was planned but not transferred. Always `upload_count - uploaded`.", "type": "integer", "minimum": 0},
28+
"warnings": {
29+
"description": "Non-blocking advisories the save returned; present only when it returned at least one. A warning at `models[<n>].sourceUri` names a model link a deployment could not download.",
30+
"type": "array",
31+
"minItems": 1,
32+
"items": {
33+
"type": "object",
34+
"required": ["field", "reason"],
35+
"additionalProperties": false,
36+
"properties": {
37+
"field": { "type": "string" },
38+
"reason": { "type": "string" }
39+
}
40+
}
41+
},
2842
"skipped_symlinks": {
2943
"description": "Symlinks excluded from a packaged custom node; present only when packaging skipped at least one. `location` points into the definition of the spec at `spec_file` (this payload carries no definition of its own), `localPath` names the node under custom_nodes/, and `member` is the symlink's path inside that node.",
3044
"type": "array",

comfy_cli/skills/comfy-build-failures/SKILL.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ the message names the field:
8585
- `build_spec_stale` — the remote moved under you, or `--id` names a Build the
8686
spec's `syncedRevision` does not belong to. See *Revising*.
8787

88+
- **`build_release_held`** — the save warned that a deployment could not download a
89+
model link, so the push cut no release. Under `--json`, `details.warnings` names
90+
each link and how its host refused; in text mode the tool printed each just above. Fix the link, or ask the user before passing
91+
`--release-despite-warnings`.
92+
8893
Two refusals are the workspace being full rather than the definition being wrong,
8994
and neither is fixed by editing anything:
9095

comfy_cli/skills/comfy-build/SKILL.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,14 @@ for comes back as a refusal envelope and exits 1: `build_update_needs_confirm`,
289289
Pass `--yes`, or the option it named, once the user has actually agreed. Do not
290290
pass `--yes` first and disclose after.
291291

292+
**`build_release_held` asks the same way, with its own option.** `comfy build push
293+
--release` saved the build but cut no release, because the save warned that a
294+
deployment could not download a model link. Under `--json` the error carries them
295+
in `details.warnings`; in text mode the tool printed each just above it. Tell the
296+
user which links fail and how, and pass `--release-despite-warnings` only after they say
297+
yes; a fixed link needs no option. `comfy build release create` cuts without this
298+
check.
299+
292300
**Three other refusals block rather than ask — `--yes` does nothing for them.**
293301
Each is cleared by deleting something, and each exits 1:
294302

tests/comfy_cli/command/build_push_support.py

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ def __init__(self) -> None:
2424
self.revision_number = 0
2525
self.stale_updates = 0
2626
self.always_stale = False
27+
# What every save answers with under ``warnings``; empty means none.
28+
self.save_warnings: list[JsonObject] = []
2729
self.build_targets: list[JsonObject] = [
2830
{"target": {"os": "linux", "gpu": "nvidia"}, "label": "Linux NVIDIA", "artifactKind": "image"},
2931
{"target": {"os": "linux", "gpu": "cpu"}, "label": "Linux CPU", "artifactKind": "image"},
@@ -76,7 +78,7 @@ def upload_blob(self, upload_url: str, path: Path) -> None:
7678
self.calls.append({"method": "upload_blob", "url": upload_url})
7779
self.uploaded.append(path.read_bytes())
7880

79-
def create_build(self, name: str, definition: JsonObject, description: str | None = None) -> str:
81+
def create_build_response(self, name: str, definition: JsonObject, description: str | None = None) -> JsonObject:
8082
revision = self._revision()
8183
self.remote_revisions[self.created_id] = revision
8284
self.calls.append(
@@ -88,7 +90,10 @@ def create_build(self, name: str, definition: JsonObject, description: str | Non
8890
"definition": definition,
8991
}
9092
)
91-
return self.created_id
93+
created: JsonObject = {"id": self.created_id, "updatedAt": revision}
94+
if self.save_warnings:
95+
created["warnings"] = self.save_warnings
96+
return created
9297

9398
def get_build(self, build_id: str) -> JsonObject:
9499
revision = self.remote_revisions.setdefault(build_id, self._revision())
@@ -121,7 +126,10 @@ def update_build(
121126
raise stale_error()
122127
revision = self._revision()
123128
self.remote_revisions[build_id] = revision
124-
return {"id": build_id, "updatedAt": revision, "name": name, "description": description}
129+
updated: JsonObject = {"id": build_id, "updatedAt": revision, "name": name, "description": description}
130+
if self.save_warnings:
131+
updated["warnings"] = self.save_warnings
132+
return updated
125133

126134
def list_build_targets(self) -> list[JsonObject]:
127135
self.calls.append({"method": "list_build_targets"})

tests/comfy_cli/command/test_build.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,7 @@ def fake_request_json(url, target, *, method="GET", body=None, max_bytes, timeou
599599

600600
c = BuilderClient("https://builder.test/", "jwt-token")
601601
assert c.create_blob("model", "f.safetensors", "hash", 5) == ("b1", "https://put")
602-
assert c.create_build("n", {"models": [], "customNodes": []}) == "d1"
602+
assert c.create_build_response("n", {"models": [], "customNodes": []})["id"] == "d1"
603603
assert c.create_release("d1", [{"os": "linux", "gpu": "nvidia"}]) == ("v1", "https://s")
604604
results = c.resolve_models(["a.safetensors"])
605605
assert results[0]["candidates"][0]["sourceUri"] == "https://u"

0 commit comments

Comments
 (0)