Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ history.

### Fixed

- `comfy deploy up` now warns about each deployment of an older release of the
Build that is still running and billing, with the
`comfy deploy stop --deployment <id>` to run. `up` on a new release creates a
new deployment and leaves the old one running; the JSON `supersedes` array
already listed it, but the terminal said nothing. Under `--json` the warning
goes to stderr.
- `insert_workflow` (`comfy workflow insert-workflow`) now rebases the inserted
template beside the target graph's existing nodes instead of leaving its
original absolute `pos` values untouched, which could land it thousands of
Expand Down
10 changes: 10 additions & 0 deletions comfy_cli/command/deploy_up.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,16 @@ def _render_result(renderer, result: UpResult, *, watch: bool) -> None:
if status == "stop_failed"
else f"run `comfy deploy scale --deployment {deployment_id} --min <n> --max <n>` to change them",
)
# A new release gets a new deployment, so the old one keeps billing. The
# JSON envelope carries `supersedes`; a person reading the terminal needs it
# said, on stderr under --json as with every other warning here.
for row in result.supersedes:
old_id = row["id"]
version = row["release"]["version"]
renderer.warn(
f"Deployment {old_id} (release v{version}, {row['status']}) is still running and billing.",
hint=f"run `comfy deploy stop --deployment {old_id}` if you no longer need it",
)
terminal = status in {"failed", "stopped", "stop_failed", "unhealthy"}
renderer.emit(
result.payload(),
Expand Down
5 changes: 3 additions & 2 deletions comfy_cli/skills/comfy-deploy/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,9 @@ running and keeps billing.

The CLI tells you this: `up` returns a `supersedes` array naming every other
live deployment of this Build still holding compute, with its id, status and
release version. **Read it and act on it.** An empty array means nothing else is
running; a non-empty one is a bill the user has not agreed to.
release version, and prints a "still running and billing" warning for each
(on stderr under `--json`). **Read it and act on it.** An empty array means
nothing else is running; a non-empty one is a bill the user has not agreed to.

```shell
comfy deploy up <dir> # note `supersedes` in the output
Expand Down
57 changes: 57 additions & 0 deletions tests/comfy_cli/command/test_deploy_up.py
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,63 @@ def test_a_stop_failed_deployment_is_not_pointed_at_a_scale_that_would_bounce(tm
assert "comfy deploy stop --deployment" in result.stderr


def test_up_warns_that_an_older_release_deployment_is_still_billing(tmp_path, monkeypatch) -> None:
"""A new release gets a new deployment and the old one keeps billing. Only
the JSON `supersedes` array said so; the terminal has to say it too."""
# Given a ready deployment on release 3 and the live one on release 5
module = _deploy()
releases = [
{"id": "release-5", "buildId": "build-1", "version": 5, "deployable": True},
{"id": "release-3", "buildId": "build-1", "version": 3, "deployable": True},
]
client = FakeDeploy([deployment("dep-live"), deployment("dep-old", release_id="release-3", status="ready")])
monkeypatch.setattr(module, "_command_clients", lambda: (FakeBuilder(releases), client))

# When
result = CliRunner().invoke(app, ["--json", "deploy", "up", str(write_spec(tmp_path))])

# Then the old deployment is named with the command that stops it
assert result.exit_code == 0, result.stderr
assert "Deployment dep-old (release v3, ready) is still running and billing." in result.stderr
assert "comfy deploy stop --deployment dep-old" in result.stderr
assert [row["id"] for row in _json_envelope(result)["data"]["supersedes"]] == ["dep-old"]


def test_a_person_at_the_terminal_sees_the_billing_warning(tmp_path, monkeypatch) -> None:
# Given a ready deployment on release 3 and the live one on release 5
module = _deploy()
releases = [
{"id": "release-5", "buildId": "build-1", "version": 5, "deployable": True},
{"id": "release-3", "buildId": "build-1", "version": 3, "deployable": True},
]
client = FakeDeploy([deployment("dep-live"), deployment("dep-old", release_id="release-3", status="ready")])
monkeypatch.setattr(module, "_command_clients", lambda: (FakeBuilder(releases), client))

# When run with human output
result = CliRunner().invoke(app, ["deploy", "up", str(write_spec(tmp_path))], env={"COMFY_OUTPUT": "pretty"})

# Then the warning is on stdout, after the deployment's own line
assert result.exit_code == 0, result.stderr
assert result.stdout.index("Deployment dep-live: ready") < result.stdout.index(
"Deployment dep-old (release v3, ready) is still running and billing."
)
assert "comfy deploy stop --deployment dep-old" in result.stdout


def test_up_with_nothing_else_running_prints_no_billing_warning(tmp_path, monkeypatch) -> None:
# Given only the live deployment of this release
module = _deploy()
monkeypatch.setattr(module, "_command_clients", lambda: (FakeBuilder(), FakeDeploy([deployment("dep-live")])))

# When
result = CliRunner().invoke(app, ["--json", "deploy", "up", str(write_spec(tmp_path))])

# Then
assert result.exit_code == 0, result.stderr
assert "still running and billing" not in result.stderr
assert _json_envelope(result)["data"]["supersedes"] == []


def test_reconcile_rejects_an_immutable_gpu_change() -> None:
# Given
module = _deploy()
Expand Down
Loading