feat: auto-deploy modal app on first remote run
This commit is contained in:
@@ -86,14 +86,16 @@ For real Modal execution, you need:
|
||||
|
||||
The remote runtime skeleton lives in [remote/modal_app.py](/home/user/git/ComfyUI-Modal/remote/modal_app.py). The extension is structured for Modal, but you still need to supply a working Modal environment.
|
||||
|
||||
When `COMFY_MODAL_EXECUTION_MODE` is remote, the runtime now treats deployed Modal class lookup via the configured app name as the normal execution path. If no deployed app is available, remote execution fails fast by default instead of silently creating a fresh ephemeral `app.run()` session.
|
||||
The intended production path is now a deployed Modal app. Ephemeral `app.run()` fallback is disabled by default because it causes expensive per-run app/image setup and defeats the point of scaling remote execution down to zero between workflows.
|
||||
When `COMFY_MODAL_EXECUTION_MODE` is remote, the runtime now treats deployed Modal class lookup via the configured app name as the normal execution path. If no deployed app is available yet, the custom node will first try to deploy it programmatically before giving up or falling back.
|
||||
The intended production path is still a deployed Modal app. Ephemeral `app.run()` fallback is disabled by default because it causes expensive per-run app/image setup and defeats the point of scaling remote execution down to zero between workflows.
|
||||
On the first remote invocation, the custom node now also attempts a programmatic `App.deploy()` automatically if the named app does not exist yet. That auto-deploy path is now the default way to bootstrap a new Modal app from inside ComfyUI.
|
||||
|
||||
Important distinction:
|
||||
- remote mode now assumes you have deployed [comfyui_modal_sync_cloud.py](/home/user/git/ComfyUI-Modal/comfyui_modal_sync_cloud.py) once, for example with `modal deploy comfyui_modal_sync_cloud.py`
|
||||
- repeated remote workflow executions should then reuse the deployed app and cached image instead of rebuilding through ephemeral `app.run()`
|
||||
- the extension still does not auto-deploy a persistent Modal app for you
|
||||
- remote mode will auto-deploy [comfyui_modal_sync_cloud.py](/home/user/git/ComfyUI-Modal/comfyui_modal_sync_cloud.py) on first use when the named app is missing, assuming Modal credentials are already configured in the ComfyUI environment
|
||||
- repeated remote workflow executions then reuse the deployed app and cached image instead of rebuilding through ephemeral `app.run()`
|
||||
- the extension now auto-deploys the persistent Modal app for you on first remote use, but only if Modal credentials and workspace access are already configured in the ComfyUI environment
|
||||
- the extension still does not auto-create a persistent web endpoint
|
||||
- `COMFY_MODAL_AUTO_DEPLOY=false` disables first-run auto-deploy if you want strict deployed-only behavior
|
||||
- `COMFY_MODAL_ALLOW_EPHEMERAL_FALLBACK=true` can re-enable the old temporary `app.run()` path if you explicitly want it
|
||||
- the actual Modal cloud service now lives in a stable importable module, [comfyui_modal_sync_cloud.py](/home/user/git/ComfyUI-Modal/comfyui_modal_sync_cloud.py), because ComfyUI’s custom-node loader does not assign Modal-safe module names to node-pack directories
|
||||
- the Modal image now installs the core ComfyUI runtime Python packages automatically, but remote execution may still surface additional environment gaps as broader workflows are exercised
|
||||
@@ -217,6 +219,7 @@ These environment variables are supported:
|
||||
- `COMFY_MODAL_EXECUTION_MODE`: Set to `local` for in-process fallback execution. Default: `local`.
|
||||
- `COMFY_MODAL_SYNC_CUSTOM_NODES`: Force-enable or disable custom-node bundle sync. Default: disabled in `local` mode, enabled otherwise.
|
||||
- `COMFY_MODAL_APP_NAME` and `COMFY_MODAL_VOLUME_NAME`: Override Modal app and volume naming.
|
||||
- `COMFY_MODAL_AUTO_DEPLOY`: Automatically deploy the Modal app on the first remote invocation when lookup fails. Default: `true`.
|
||||
- `COMFY_MODAL_ALLOW_EPHEMERAL_FALLBACK`: Re-enable slow `app.run()` fallback in remote mode. Default: `false`.
|
||||
- `COMFY_MODAL_ENABLE_MEMORY_SNAPSHOT`: Enable Modal CPU memory snapshots for the deployed remote class. Default: `true`.
|
||||
- `COMFY_MODAL_ENABLE_GPU_MEMORY_SNAPSHOT`: Opt into Modal GPU memory snapshots. Default: `false`.
|
||||
|
||||
@@ -11,6 +11,7 @@
|
||||
- Per-node proxying is not enough for real ComfyUI sampler graphs. Connected remote-marked nodes must be partitioned into a single remote component so internal edges like `MODEL`, `CLIP`, and `CONDITIONING` stay inside the remote subgraph and only the true local/remote boundary has to be transport-safe.
|
||||
- The local fallback runtime cannot call `PromptExecutor.execute()` inline from inside a proxy node, because ComfyUI is already running inside an active asyncio loop. Running remote subgraph execution in a worker thread avoids nested-event-loop failures while still reusing ComfyUI’s executor.
|
||||
- Modal objects defined in-process are not directly callable from an arbitrary long-running app like ComfyUI unless they are inside a hydrated `app.run()` session or looked up from a deployed app with `modal.Cls.from_name(...)`. In practice the correct default for this project is deployed lookup only, because ephemeral `app.run()` fallback rebuilds too much state between workflow executions and undermines the scale-to-zero cost model.
|
||||
- A deployed-only strategy is still correct, but the custom node should bootstrap that deployment itself on first use. The pragmatic flow is: try `Cls.from_name(...)`, and if the app is missing, programmatically call `app.deploy(...)` from the stable cloud entry module, then retry the lookup.
|
||||
- ComfyUI loads custom node packs from directories under synthetic module names derived from filesystem paths, and for directory-based packs that module key is not guaranteed to be a valid Python import path. Modal container startup requires a real importable module name for `@app.cls` services, so the cloud entrypoint has to live in a separate, stable module such as `comfyui_modal_sync_cloud.py` rather than on the custom-node package module itself.
|
||||
- Shipping the ComfyUI source tree into the Modal image is not enough by itself. The container also needs ComfyUI's import-time Python dependencies such as `psutil`, `aiohttp`, `einops`, `torchsde`, `torchvision`, and related runtime packages, or the remote worker fails before it can execute any graph nodes.
|
||||
- Quantized ComfyUI models add another hard dependency: if `comfy-kitchen` is missing in the Modal image, `comfy.quant_ops.get_layout_class(...)` falls back to `None` and FP8/FP4 model loading eventually fails with `'NoneType' object has no attribute 'Params'`.
|
||||
|
||||
+79
-3
@@ -8,6 +8,7 @@ import logging
|
||||
import os
|
||||
import sys
|
||||
import tempfile
|
||||
import threading
|
||||
import time
|
||||
import zipfile
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
@@ -24,6 +25,8 @@ from ..settings import get_settings
|
||||
logger = logging.getLogger(__name__)
|
||||
_REMOTE_MODAL_CALL_EXECUTOR = ThreadPoolExecutor(max_workers=1)
|
||||
_MODAL_CLOUD_MODULE_NAME = "comfyui_modal_sync_cloud"
|
||||
_MODAL_AUTO_DEPLOY_LOCK = threading.Lock()
|
||||
_MODAL_AUTO_DEPLOYED_APPS: set[tuple[str, str | None]] = set()
|
||||
|
||||
try:
|
||||
import modal # type: ignore
|
||||
@@ -491,6 +494,67 @@ def _lookup_deployed_modal_method(payload: dict[str, Any]) -> Any:
|
||||
return remote_engine.execute_payload
|
||||
|
||||
|
||||
def _modal_environment_name() -> str | None:
|
||||
"""Return the active Modal environment name when explicitly configured."""
|
||||
environment_name = os.getenv("MODAL_ENVIRONMENT")
|
||||
if environment_name is None:
|
||||
return None
|
||||
normalized = environment_name.strip()
|
||||
return normalized or None
|
||||
|
||||
|
||||
def _modal_deploy_cache_key() -> tuple[str, str | None]:
|
||||
"""Return the cache key for auto-deployed Modal apps."""
|
||||
settings = get_settings()
|
||||
return (settings.app_name, _modal_environment_name())
|
||||
|
||||
|
||||
def _auto_deploy_modal_app(payload: dict[str, Any], lookup_error: BaseException) -> None:
|
||||
"""Deploy the stable Modal cloud app once when deployed lookup fails."""
|
||||
if modal is None:
|
||||
raise ModalRemoteInvocationError("Modal SDK is unavailable.")
|
||||
|
||||
settings = get_settings()
|
||||
deploy_key = _modal_deploy_cache_key()
|
||||
cloud_module = _load_modal_cloud_module()
|
||||
cloud_app = getattr(cloud_module, "app", None)
|
||||
if cloud_app is None:
|
||||
raise ModalRemoteInvocationError(
|
||||
"Stable Modal cloud entry module did not expose a deployable app."
|
||||
)
|
||||
|
||||
with _MODAL_AUTO_DEPLOY_LOCK:
|
||||
if deploy_key in _MODAL_AUTO_DEPLOYED_APPS:
|
||||
logger.info(
|
||||
"Auto-deploy already completed for app=%s env=%s; reusing cached deployment state.",
|
||||
settings.app_name,
|
||||
deploy_key[1] or "<default>",
|
||||
)
|
||||
return
|
||||
|
||||
logger.warning(
|
||||
"Deployed Modal app lookup failed for app=%s component=%s: %s. "
|
||||
"Attempting first-run auto-deploy from the custom node.",
|
||||
settings.app_name,
|
||||
payload.get("component_id"),
|
||||
lookup_error,
|
||||
)
|
||||
output_context = modal.enable_output() if hasattr(modal, "enable_output") else nullcontext()
|
||||
deploy_started_at = time.perf_counter()
|
||||
with output_context:
|
||||
cloud_app.deploy(
|
||||
name=settings.app_name,
|
||||
environment_name=deploy_key[1],
|
||||
)
|
||||
_MODAL_AUTO_DEPLOYED_APPS.add(deploy_key)
|
||||
logger.info(
|
||||
"Auto-deployed Modal app %s for env=%s in %.3fs.",
|
||||
settings.app_name,
|
||||
deploy_key[1] or "<default>",
|
||||
time.perf_counter() - deploy_started_at,
|
||||
)
|
||||
|
||||
|
||||
def _invoke_modal_payload_blocking(payload: dict[str, Any], kwargs_payload: bytes) -> bytes:
|
||||
"""Invoke the Modal runtime from a worker thread using deployed or ephemeral app state."""
|
||||
if modal is None:
|
||||
@@ -508,12 +572,24 @@ def _invoke_modal_payload_blocking(payload: dict[str, Any], kwargs_payload: byte
|
||||
)
|
||||
return remote_method.remote(payload, kwargs_payload)
|
||||
except lookup_error_types as exc:
|
||||
if settings.auto_deploy:
|
||||
_auto_deploy_modal_app(payload, exc)
|
||||
try:
|
||||
remote_method = _lookup_deployed_modal_method(payload)
|
||||
logger.info(
|
||||
"Using auto-deployed Modal app %s for component %s.",
|
||||
settings.app_name,
|
||||
payload.get("component_id"),
|
||||
)
|
||||
return remote_method.remote(payload, kwargs_payload)
|
||||
except lookup_error_types as retry_exc:
|
||||
exc = retry_exc
|
||||
if not settings.allow_ephemeral_fallback:
|
||||
raise ModalRemoteInvocationError(
|
||||
"Remote execution requires a deployed Modal app. "
|
||||
"Remote execution requires a deployed Modal app or a successful first-run auto-deploy. "
|
||||
f"Lookup failed for app={settings.app_name!r} component={payload.get('component_id')!r}: {exc}. "
|
||||
"Deploy the app once and retry, or set COMFY_MODAL_ALLOW_EPHEMERAL_FALLBACK=true "
|
||||
"to allow slow ephemeral app.run() fallback behavior."
|
||||
"Ensure Modal credentials are configured so the custom node can auto-deploy, "
|
||||
"or set COMFY_MODAL_ALLOW_EPHEMERAL_FALLBACK=true to allow slow ephemeral app.run() fallback behavior."
|
||||
) from exc
|
||||
logger.warning(
|
||||
"Deployed Modal app lookup failed for app=%s component=%s: %s. Falling back to ephemeral app.run(); this creates a temporary Modal app session, not a persistent deployment or endpoint.",
|
||||
|
||||
@@ -16,6 +16,7 @@ class ModalSyncSettings:
|
||||
"""Resolved configuration for local and remote Modal-Sync operations."""
|
||||
|
||||
app_name: str
|
||||
auto_deploy: bool
|
||||
allow_ephemeral_fallback: bool
|
||||
enable_memory_snapshot: bool
|
||||
enable_gpu_memory_snapshot: bool
|
||||
@@ -105,6 +106,7 @@ def get_settings() -> ModalSyncSettings:
|
||||
|
||||
settings = ModalSyncSettings(
|
||||
app_name=os.getenv("COMFY_MODAL_APP_NAME", "comfy-modal-sync"),
|
||||
auto_deploy=_read_bool_env("COMFY_MODAL_AUTO_DEPLOY") is not False,
|
||||
allow_ephemeral_fallback=_read_bool_env("COMFY_MODAL_ALLOW_EPHEMERAL_FALLBACK") or False,
|
||||
enable_memory_snapshot=_read_bool_env("COMFY_MODAL_ENABLE_MEMORY_SNAPSHOT") is not False,
|
||||
enable_gpu_memory_snapshot=_read_bool_env("COMFY_MODAL_ENABLE_GPU_MEMORY_SNAPSHOT") or False,
|
||||
|
||||
@@ -127,6 +127,7 @@ def reset_modal_environment(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) ->
|
||||
"""Isolate Modal-Sync environment variables between tests."""
|
||||
monkeypatch.setenv("COMFY_MODAL_EXECUTION_MODE", "local")
|
||||
monkeypatch.setenv("COMFY_MODAL_LOCAL_STORAGE_ROOT", str(tmp_path / "storage"))
|
||||
monkeypatch.delenv("COMFY_MODAL_AUTO_DEPLOY", raising=False)
|
||||
monkeypatch.delenv("COMFY_MODAL_ALLOW_EPHEMERAL_FALLBACK", raising=False)
|
||||
monkeypatch.delenv("COMFY_MODAL_CUSTOM_NODES_DIR", raising=False)
|
||||
monkeypatch.delenv("COMFY_MODAL_ENABLE_MEMORY_SNAPSHOT", raising=False)
|
||||
|
||||
@@ -53,6 +53,7 @@ def test_rewrite_groups_connected_remote_nodes_into_single_proxy(
|
||||
|
||||
settings = settings_module.ModalSyncSettings(
|
||||
app_name="app",
|
||||
auto_deploy=True,
|
||||
allow_ephemeral_fallback=False,
|
||||
enable_memory_snapshot=True,
|
||||
enable_gpu_memory_snapshot=False,
|
||||
@@ -153,6 +154,7 @@ def test_rewrite_rejects_non_transportable_remote_inputs(
|
||||
|
||||
settings = settings_module.ModalSyncSettings(
|
||||
app_name="app",
|
||||
auto_deploy=True,
|
||||
allow_ephemeral_fallback=False,
|
||||
enable_memory_snapshot=True,
|
||||
enable_gpu_memory_snapshot=False,
|
||||
@@ -230,6 +232,7 @@ def test_rewrite_rejects_non_transportable_remote_outputs(
|
||||
|
||||
settings = settings_module.ModalSyncSettings(
|
||||
app_name="app",
|
||||
auto_deploy=True,
|
||||
allow_ephemeral_fallback=False,
|
||||
enable_memory_snapshot=True,
|
||||
enable_gpu_memory_snapshot=False,
|
||||
|
||||
@@ -4,6 +4,7 @@ from __future__ import annotations
|
||||
|
||||
import sys
|
||||
import types
|
||||
from contextlib import nullcontext
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
@@ -178,11 +179,90 @@ def test_modal_cloud_builds_snapshot_enabled_cls_options(
|
||||
assert gpu_snapshot_options["experimental_options"] == {"enable_gpu_snapshot": True}
|
||||
|
||||
|
||||
def test_remote_modal_requires_deployed_app_by_default(
|
||||
def test_remote_modal_auto_deploys_missing_app_by_default(
|
||||
remote_modal_app_module: Any,
|
||||
monkeypatch: Any,
|
||||
) -> None:
|
||||
"""Remote mode should reject slow ephemeral fallback unless explicitly enabled."""
|
||||
"""Remote mode should auto-deploy the stable Modal app on first lookup failure."""
|
||||
|
||||
class FakeLookupError(Exception):
|
||||
"""Stand-in for Modal deployed lookup failures."""
|
||||
|
||||
deploy_calls: list[tuple[str | None, str | None]] = []
|
||||
|
||||
class FakeExecuteMethod:
|
||||
"""Minimal Modal method handle that records remote calls."""
|
||||
|
||||
def remote(self, payload: dict[str, Any], kwargs_payload: bytes) -> bytes:
|
||||
"""Return deterministic remote bytes."""
|
||||
return b"remote-response"
|
||||
|
||||
class FakeRemoteEngine:
|
||||
"""Minimal deployed remote engine instance."""
|
||||
|
||||
execute_payload = FakeExecuteMethod()
|
||||
|
||||
class FakeApp:
|
||||
"""Minimal deployable cloud app double."""
|
||||
|
||||
def deploy(self, *, name: str | None = None, environment_name: str | None = None, **_: Any) -> "FakeApp":
|
||||
"""Record the deploy request and mark the deployment available."""
|
||||
deploy_calls.append((name, environment_name))
|
||||
FakeModal.deployed = True
|
||||
return self
|
||||
|
||||
class FakeModal:
|
||||
"""Minimal modal SDK double with deployed lookup failure types."""
|
||||
|
||||
deployed = False
|
||||
exception = types.SimpleNamespace(
|
||||
NotFoundError=FakeLookupError,
|
||||
ExecutionError=FakeLookupError,
|
||||
InvalidError=FakeLookupError,
|
||||
)
|
||||
|
||||
class Cls:
|
||||
"""Namespace for deployed class lookups."""
|
||||
|
||||
@staticmethod
|
||||
def from_name(app_name: str, class_name: str) -> Any:
|
||||
"""Return a deployed class after the first auto-deploy."""
|
||||
if not FakeModal.deployed:
|
||||
raise FakeLookupError("not deployed")
|
||||
return lambda: FakeRemoteEngine()
|
||||
|
||||
@staticmethod
|
||||
def enable_output() -> Any:
|
||||
"""Provide a no-op output context manager."""
|
||||
return nullcontext()
|
||||
|
||||
monkeypatch.setattr(remote_modal_app_module, "modal", FakeModal)
|
||||
monkeypatch.setattr(
|
||||
remote_modal_app_module,
|
||||
"_load_modal_cloud_module",
|
||||
lambda: types.SimpleNamespace(app=FakeApp()),
|
||||
)
|
||||
monkeypatch.setenv("COMFY_MODAL_AUTO_DEPLOY", "true")
|
||||
remote_modal_app_module.get_settings.cache_clear()
|
||||
remote_modal_app_module._MODAL_AUTO_DEPLOYED_APPS.clear()
|
||||
try:
|
||||
response = remote_modal_app_module._invoke_modal_payload_blocking(
|
||||
{"component_id": "component-1"},
|
||||
b"{}",
|
||||
)
|
||||
finally:
|
||||
remote_modal_app_module.get_settings.cache_clear()
|
||||
remote_modal_app_module._MODAL_AUTO_DEPLOYED_APPS.clear()
|
||||
|
||||
assert response == b"remote-response"
|
||||
assert deploy_calls == [("comfy-modal-sync", None)]
|
||||
|
||||
|
||||
def test_remote_modal_requires_manual_deploy_when_auto_deploy_disabled(
|
||||
remote_modal_app_module: Any,
|
||||
monkeypatch: Any,
|
||||
) -> None:
|
||||
"""Remote mode should fail clearly when auto-deploy and ephemeral fallback are both disabled."""
|
||||
|
||||
class FakeLookupError(Exception):
|
||||
"""Stand-in for Modal deployed lookup failures."""
|
||||
@@ -205,6 +285,7 @@ def test_remote_modal_requires_deployed_app_by_default(
|
||||
raise FakeLookupError("not deployed")
|
||||
|
||||
monkeypatch.setattr(remote_modal_app_module, "modal", FakeModal)
|
||||
monkeypatch.setenv("COMFY_MODAL_AUTO_DEPLOY", "false")
|
||||
monkeypatch.setenv("COMFY_MODAL_ALLOW_EPHEMERAL_FALLBACK", "false")
|
||||
remote_modal_app_module.get_settings.cache_clear()
|
||||
try:
|
||||
@@ -220,7 +301,7 @@ def test_remote_modal_requires_deployed_app_by_default(
|
||||
finally:
|
||||
remote_modal_app_module.get_settings.cache_clear()
|
||||
|
||||
assert "requires a deployed Modal app" in message
|
||||
assert "requires a deployed Modal app or a successful first-run auto-deploy" in message
|
||||
assert "COMFY_MODAL_ALLOW_EPHEMERAL_FALLBACK=true" in message
|
||||
|
||||
|
||||
|
||||
@@ -19,6 +19,7 @@ def test_sync_file_deduplicates_by_hash(
|
||||
|
||||
settings = settings_module.ModalSyncSettings(
|
||||
app_name="app",
|
||||
auto_deploy=True,
|
||||
allow_ephemeral_fallback=False,
|
||||
enable_memory_snapshot=True,
|
||||
enable_gpu_memory_snapshot=False,
|
||||
@@ -59,6 +60,7 @@ def test_sync_custom_nodes_directory_creates_archive(
|
||||
|
||||
settings = settings_module.ModalSyncSettings(
|
||||
app_name="app",
|
||||
auto_deploy=True,
|
||||
allow_ephemeral_fallback=False,
|
||||
enable_memory_snapshot=True,
|
||||
enable_gpu_memory_snapshot=False,
|
||||
@@ -97,6 +99,7 @@ def test_hash_directory_ignores_virtualenv_and_bytecode_artifacts(
|
||||
|
||||
settings = settings_module.ModalSyncSettings(
|
||||
app_name="app",
|
||||
auto_deploy=True,
|
||||
allow_ephemeral_fallback=False,
|
||||
enable_memory_snapshot=True,
|
||||
enable_gpu_memory_snapshot=False,
|
||||
@@ -189,6 +192,7 @@ def test_remote_mode_uses_modal_volume_backend_when_sdk_is_available(
|
||||
|
||||
settings = settings_module.ModalSyncSettings(
|
||||
app_name="app",
|
||||
auto_deploy=True,
|
||||
allow_ephemeral_fallback=False,
|
||||
enable_memory_snapshot=True,
|
||||
enable_gpu_memory_snapshot=False,
|
||||
|
||||
Reference in New Issue
Block a user