fix: replace stale Modal app versions
This commit is contained in:
@@ -151,6 +151,7 @@ Remote mode requires:
|
||||
The normal remote path is deployed-app lookup by name. On first remote use, the extension can auto-deploy the stable cloud entrypoint from [`comfyui_modal_sync_cloud.py`](comfyui_modal_sync_cloud.py) if the configured app does not exist yet.
|
||||
If that deployed Modal app is later deleted manually, the next lookup failure now invalidates the stale in-process deploy cache and auto-deploys a fresh copy instead of surfacing a hard "app not found" error.
|
||||
If a previously looked-up `RemoteEngine` handle later fails during `hydrate()` or the first `remote()` call because the app was deleted out-of-band, the invoke path now treats that as the same missing-app condition, auto-deploys, and retries instead of first surfacing a spurious prompt failure.
|
||||
The deployed `RemoteEngine` also reports a protocol version. If the local client detects an older or unversioned app, it stops the named Modal app and lets auto-deploy replace it before running the payload.
|
||||
If a previous attempt imported that cloud entrypoint only partially before failing, the loader now discards the stale cached module and retries the import cleanly before auto-deploy.
|
||||
The local and remote subgraph runners also canonicalize malformed singleton-list wrappers in rewritten prompts, including socket indexes like `["node", [0]]`, scalar inputs like `[4]`, and wrapped execute node ids like `["12"]`, before handing the prompt to ComfyUI.
|
||||
When the remote PromptExecutor still fails, the Modal worker now logs the normalized execute-node ids, boundary outputs, suspicious remaining wrapped prompt inputs, and the underlying ComfyUI `execution_error` payload to make the bad field visible.
|
||||
|
||||
@@ -32,6 +32,7 @@ _REPO_ROOT = Path(__file__).resolve().parent
|
||||
_REMOTE_REPO_ROOT = Path("/root/comfyui_modal_sync_repo")
|
||||
_LOCAL_COMFYUI_ROOT = (Path.home() / "git" / "ComfyUI").resolve()
|
||||
_REMOTE_COMFYUI_ROOT = Path("/root/comfyui_src")
|
||||
_REMOTE_APP_PROTOCOL_VERSION = 2
|
||||
_PYTORCH_CUDA_INDEX_URL = "https://download.pytorch.org/whl/cu128"
|
||||
_COMFYUI_TORCH_VERSION = "2.10.0"
|
||||
_COMFYUI_TORCHVISION_VERSION = "0.25.0"
|
||||
@@ -4807,6 +4808,14 @@ if modal is not None: # pragma: no branch - remote entrypoint configuration.
|
||||
"""Prime the current or a newly started Modal container for one prompt."""
|
||||
return _prepare_warm_container_for_request(vol, payload)
|
||||
|
||||
@modal.method()
|
||||
def runtime_version(self) -> dict[str, Any]:
|
||||
"""Return the deployed app protocol version expected by the local client."""
|
||||
return {
|
||||
"protocol_version": _REMOTE_APP_PROTOCOL_VERSION,
|
||||
"app_name": settings.app_name,
|
||||
}
|
||||
|
||||
@modal.method()
|
||||
def execute_payload_stream(
|
||||
self,
|
||||
|
||||
@@ -222,3 +222,4 @@
|
||||
- Modal deployment updates are too permissive for GPU-type changes. Because a named deployed app can be updated in place while its resource expectations and synced storage assumptions may have come from an older `COMFY_MODAL_GPU`, the cloud entrypoint should fail local app construction when that app name already exists and require the operator to stop/delete the old app before building the new one.
|
||||
- ComfyUI's cache setup indexes `NODE_CLASS_MAPPINGS[class_type]` before node execution starts, so missing custom-node registrations can surface as a raw `KeyError` from `get_immediate_node_signature`. Remote workers should preflight prompt class types after bundle extraction, retry external custom-node import once, and raise an explicit missing-class error if the registry is still incomplete.
|
||||
- A successful custom-node sync only proves the package files reached Modal storage; it does not prove ComfyUI imported the package. When a synced class remains unregistered, the remote error should include whether the extracted bundle contains Python files mentioning that class and which top-level package they came from, so the next debugging step is import failure logs instead of another sync toggle check.
|
||||
- Stale deployed Modal apps are not limited to missing-app failures. The local client needs an explicit app protocol handshake with `RemoteEngine`, and an unversioned or older remote app should be stopped and replaced before payload execution so local code changes are not silently tested against an old image.
|
||||
|
||||
+192
-4
@@ -6,6 +6,7 @@ from dataclasses import dataclass, field
|
||||
import hashlib
|
||||
import importlib
|
||||
import importlib.util
|
||||
import inspect
|
||||
from io import BytesIO
|
||||
import json
|
||||
import logging
|
||||
@@ -54,8 +55,10 @@ from ..settings import get_settings
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
_MODAL_CLOUD_MODULE_NAME = "comfyui_modal_sync_cloud"
|
||||
_REMOTE_APP_PROTOCOL_VERSION = 2
|
||||
_MODAL_AUTO_DEPLOY_LOCK = threading.Lock()
|
||||
_MODAL_AUTO_DEPLOY_STATES: dict[tuple[str, str | None], "_ModalAutoDeployState"] = {}
|
||||
_MODAL_REMOTE_APP_VERSION_OK: set[tuple[str, str | None]] = set()
|
||||
_MODAL_INTERRUPT_DICTS_LOCK = threading.Lock()
|
||||
_MODAL_INTERRUPT_DICTS: dict[tuple[str, str | None], Any] = {}
|
||||
_MAPPED_PROGRESS_NODE_IDS_LOCK = threading.Lock()
|
||||
@@ -165,6 +168,10 @@ class ModalRemoteInvocationError(RuntimeError):
|
||||
"""Raised when the Modal client cannot invoke the remote runtime."""
|
||||
|
||||
|
||||
class ModalRemoteAppOutOfDateError(ModalRemoteInvocationError):
|
||||
"""Raised when a deployed Modal app is incompatible with the local client."""
|
||||
|
||||
|
||||
def _is_remote_container_log_stream_enabled() -> bool:
|
||||
"""Return whether remote Modal container logs should be mirrored locally."""
|
||||
return bool(get_settings().stream_remote_container_logs)
|
||||
@@ -3928,6 +3935,168 @@ def _modal_deploy_cache_key() -> tuple[str, str | None]:
|
||||
return (settings.app_name, _modal_environment_name())
|
||||
|
||||
|
||||
def _call_modal_method(method: Any, *args: Any, **kwargs: Any) -> Any:
|
||||
"""Invoke a Modal method handle or an in-process test double."""
|
||||
remote_method = getattr(method, "remote", None)
|
||||
if callable(remote_method):
|
||||
return remote_method(*args, **kwargs)
|
||||
return method(*args, **kwargs)
|
||||
|
||||
|
||||
def _remote_engine_runtime_version(remote_engine: Any) -> dict[str, Any] | None:
|
||||
"""Return runtime version metadata from a deployed engine when available."""
|
||||
version_method = getattr(remote_engine, "runtime_version", None)
|
||||
if version_method is None:
|
||||
return None
|
||||
version_payload = _call_modal_method(version_method)
|
||||
if not isinstance(version_payload, dict):
|
||||
return None
|
||||
return version_payload
|
||||
|
||||
|
||||
def _remote_engine_protocol_version(remote_engine: Any) -> int | None:
|
||||
"""Return the deployed remote app protocol version when the app reports one."""
|
||||
version_payload = _remote_engine_runtime_version(remote_engine)
|
||||
if version_payload is None:
|
||||
return None
|
||||
protocol_version = version_payload.get("protocol_version")
|
||||
if isinstance(protocol_version, bool):
|
||||
return None
|
||||
if isinstance(protocol_version, int):
|
||||
return protocol_version
|
||||
return None
|
||||
|
||||
|
||||
def _is_remote_engine_protocol_current(remote_engine: Any) -> bool:
|
||||
"""Return whether a deployed engine is compatible with this local client."""
|
||||
return _remote_engine_protocol_version(remote_engine) == _REMOTE_APP_PROTOCOL_VERSION
|
||||
|
||||
|
||||
def _stop_modal_app_via_sdk(app_name: str) -> bool:
|
||||
"""Try to stop a Modal app through the SDK if this SDK version exposes app stopping."""
|
||||
if modal is None:
|
||||
return False
|
||||
app_namespace = getattr(modal, "App", None)
|
||||
app_lookup = getattr(app_namespace, "lookup", None)
|
||||
if not callable(app_lookup):
|
||||
return False
|
||||
try:
|
||||
lookup_signature = inspect.signature(app_lookup)
|
||||
except (TypeError, ValueError):
|
||||
lookup_signature = None
|
||||
lookup_kwargs: dict[str, Any] = {}
|
||||
if lookup_signature is not None:
|
||||
if "create_if_missing" in lookup_signature.parameters:
|
||||
lookup_kwargs["create_if_missing"] = False
|
||||
elif "create" in lookup_signature.parameters:
|
||||
lookup_kwargs["create"] = False
|
||||
try:
|
||||
app_handle = app_lookup(app_name, **lookup_kwargs)
|
||||
except _modal_lookup_error_types():
|
||||
return True
|
||||
stop_method = getattr(app_handle, "stop", None)
|
||||
if not callable(stop_method):
|
||||
return False
|
||||
_call_modal_method(stop_method)
|
||||
return True
|
||||
|
||||
|
||||
def _stop_modal_app_via_cli(app_name: str) -> bool:
|
||||
"""Try to stop a Modal app through the Modal CLI."""
|
||||
modal_cli = shutil.which("modal")
|
||||
if modal_cli is None:
|
||||
return False
|
||||
command = [modal_cli, "app", "stop", app_name]
|
||||
completed = subprocess.run(
|
||||
command,
|
||||
check=False,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=120,
|
||||
)
|
||||
if completed.returncode != 0:
|
||||
logger.warning(
|
||||
"Modal CLI app stop failed for app=%s exit_code=%s stderr=%s",
|
||||
app_name,
|
||||
completed.returncode,
|
||||
completed.stderr.strip(),
|
||||
)
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
def _stop_modal_app_for_replacement(app_name: str) -> None:
|
||||
"""Stop an out-of-date Modal app before replacing it with a fresh deployment."""
|
||||
if _stop_modal_app_via_sdk(app_name):
|
||||
logger.warning("Stopped out-of-date Modal app %s through the Modal SDK.", app_name)
|
||||
return
|
||||
if _stop_modal_app_via_cli(app_name):
|
||||
logger.warning("Stopped out-of-date Modal app %s through the Modal CLI.", app_name)
|
||||
return
|
||||
raise ModalRemoteInvocationError(
|
||||
f"Deployed Modal app {app_name!r} is out of date, but Modal-Sync could not stop it automatically. "
|
||||
f"Stop it manually with `modal app stop {app_name}` and retry."
|
||||
)
|
||||
|
||||
|
||||
def _mark_modal_deploy_state_not_ready(deploy_key: tuple[str, str | None]) -> None:
|
||||
"""Invalidate the local in-process deploy readiness cache for one app."""
|
||||
with _MODAL_AUTO_DEPLOY_LOCK:
|
||||
deploy_state = _MODAL_AUTO_DEPLOY_STATES.get(deploy_key)
|
||||
if deploy_state is None:
|
||||
return
|
||||
with deploy_state.condition:
|
||||
deploy_state.ready = False
|
||||
deploy_state.last_error = None
|
||||
deploy_state.condition.notify_all()
|
||||
|
||||
|
||||
def _replace_outdated_modal_app(
|
||||
payload: dict[str, Any],
|
||||
remote_engine: Any,
|
||||
) -> Any:
|
||||
"""Stop and auto-deploy a replacement for an incompatible deployed app."""
|
||||
settings = get_settings()
|
||||
deploy_key = _modal_deploy_cache_key()
|
||||
protocol_version = _remote_engine_protocol_version(remote_engine)
|
||||
logger.warning(
|
||||
"Deployed Modal app %s is out of date for component=%s remote_protocol=%s local_protocol=%s; stopping and replacing it.",
|
||||
settings.app_name,
|
||||
payload.get("component_id"),
|
||||
protocol_version,
|
||||
_REMOTE_APP_PROTOCOL_VERSION,
|
||||
)
|
||||
with _MODAL_AUTO_DEPLOY_LOCK:
|
||||
_MODAL_REMOTE_APP_VERSION_OK.discard(deploy_key)
|
||||
_mark_modal_deploy_state_not_ready(deploy_key)
|
||||
_stop_modal_app_for_replacement(settings.app_name)
|
||||
stale_error = ModalRemoteAppOutOfDateError(
|
||||
f"Modal app {settings.app_name!r} protocol {protocol_version!r} does not match local protocol "
|
||||
f"{_REMOTE_APP_PROTOCOL_VERSION}."
|
||||
)
|
||||
replacement_engine = _auto_deploy_modal_app(payload, stale_error)
|
||||
with _MODAL_AUTO_DEPLOY_LOCK:
|
||||
_MODAL_REMOTE_APP_VERSION_OK.add(deploy_key)
|
||||
return replacement_engine
|
||||
|
||||
|
||||
def _ensure_remote_engine_protocol_current(remote_engine: Any, payload: dict[str, Any]) -> Any:
|
||||
"""Return a compatible remote engine, replacing the deployed app when allowed."""
|
||||
deploy_key = _modal_deploy_cache_key()
|
||||
with _MODAL_AUTO_DEPLOY_LOCK:
|
||||
if deploy_key in _MODAL_REMOTE_APP_VERSION_OK:
|
||||
return remote_engine
|
||||
if _is_remote_engine_protocol_current(remote_engine):
|
||||
with _MODAL_AUTO_DEPLOY_LOCK:
|
||||
_MODAL_REMOTE_APP_VERSION_OK.add(deploy_key)
|
||||
return remote_engine
|
||||
if not get_settings().auto_deploy:
|
||||
raise ModalRemoteInvocationError(
|
||||
"Deployed Modal app is out of date and COMFY_MODAL_AUTO_DEPLOY=false prevents automatic replacement."
|
||||
)
|
||||
return _replace_outdated_modal_app(payload, remote_engine)
|
||||
|
||||
|
||||
def _modal_auto_deploy_state(
|
||||
deploy_key: tuple[str, str | None],
|
||||
) -> _ModalAutoDeployState:
|
||||
@@ -4294,7 +4463,10 @@ def _invoke_modal_warmup_blocking(warmup_request: dict[str, Any]) -> Any:
|
||||
settings = get_settings()
|
||||
if lookup_error_types:
|
||||
try:
|
||||
remote_engine = _lookup_deployed_remote_engine(warmup_request)
|
||||
remote_engine = _ensure_remote_engine_protocol_current(
|
||||
_lookup_deployed_remote_engine(warmup_request),
|
||||
warmup_request,
|
||||
)
|
||||
return _invoke_remote_engine_warmup_with_recovery(remote_engine, warmup_request)
|
||||
except lookup_error_types as exc:
|
||||
if settings.auto_deploy:
|
||||
@@ -4312,7 +4484,10 @@ def _invoke_modal_warmup_blocking(warmup_request: dict[str, Any]) -> Any:
|
||||
f"Lookup failed for app={settings.app_name!r}: {exc}."
|
||||
) from exc
|
||||
else:
|
||||
remote_engine = _lookup_deployed_remote_engine(warmup_request)
|
||||
remote_engine = _ensure_remote_engine_protocol_current(
|
||||
_lookup_deployed_remote_engine(warmup_request),
|
||||
warmup_request,
|
||||
)
|
||||
return _invoke_remote_engine_warmup_with_recovery(remote_engine, warmup_request)
|
||||
|
||||
cloud_module = _load_modal_cloud_module()
|
||||
@@ -4586,6 +4761,11 @@ def _auto_deploy_modal_app(payload: dict[str, Any], lookup_error: BaseException)
|
||||
time.perf_counter() - deploy_started_at,
|
||||
)
|
||||
remote_engine = _lookup_deployed_remote_engine_with_retry(payload)
|
||||
if not _is_remote_engine_protocol_current(remote_engine):
|
||||
raise ModalRemoteInvocationError(
|
||||
f"Auto-deployed Modal app {settings.app_name!r} did not report expected protocol "
|
||||
f"{_REMOTE_APP_PROTOCOL_VERSION}."
|
||||
)
|
||||
except BaseException as exc:
|
||||
with deploy_state.condition:
|
||||
deploy_state.deploy_in_progress = False
|
||||
@@ -4598,6 +4778,8 @@ def _auto_deploy_modal_app(payload: dict[str, Any], lookup_error: BaseException)
|
||||
deploy_state.ready = True
|
||||
deploy_state.last_error = None
|
||||
deploy_state.condition.notify_all()
|
||||
with _MODAL_AUTO_DEPLOY_LOCK:
|
||||
_MODAL_REMOTE_APP_VERSION_OK.add(deploy_key)
|
||||
logger.info(
|
||||
"Deployed Modal app %s for env=%s is now lookup-ready.",
|
||||
settings.app_name,
|
||||
@@ -4697,7 +4879,10 @@ def _invoke_modal_payload_blocking(
|
||||
settings = get_settings()
|
||||
if lookup_error_types:
|
||||
try:
|
||||
remote_engine = _lookup_deployed_remote_engine(payload)
|
||||
remote_engine = _ensure_remote_engine_protocol_current(
|
||||
_lookup_deployed_remote_engine(payload),
|
||||
payload,
|
||||
)
|
||||
logger.info(
|
||||
"Using deployed Modal app %s for component %s.",
|
||||
settings.app_name,
|
||||
@@ -4740,7 +4925,10 @@ def _invoke_modal_payload_blocking(
|
||||
exc,
|
||||
)
|
||||
else:
|
||||
remote_engine = _lookup_deployed_remote_engine(payload)
|
||||
remote_engine = _ensure_remote_engine_protocol_current(
|
||||
_lookup_deployed_remote_engine(payload),
|
||||
payload,
|
||||
)
|
||||
logger.info(
|
||||
"Using deployed Modal app %s for component %s.",
|
||||
settings.app_name,
|
||||
|
||||
@@ -2871,6 +2871,9 @@ def test_remote_modal_auto_deploys_missing_app_by_default(
|
||||
"""Minimal deployed remote engine instance."""
|
||||
|
||||
execute_payload = FakeExecuteMethod()
|
||||
runtime_version = types.SimpleNamespace(
|
||||
remote=lambda: {"protocol_version": remote_modal_app_module._REMOTE_APP_PROTOCOL_VERSION}
|
||||
)
|
||||
|
||||
class FakeApp:
|
||||
"""Minimal deployable cloud app double."""
|
||||
@@ -2915,6 +2918,7 @@ def test_remote_modal_auto_deploys_missing_app_by_default(
|
||||
monkeypatch.setenv("COMFY_MODAL_AUTO_DEPLOY", "true")
|
||||
remote_modal_app_module.get_settings.cache_clear()
|
||||
remote_modal_app_module._MODAL_AUTO_DEPLOY_STATES.clear()
|
||||
remote_modal_app_module._MODAL_REMOTE_APP_VERSION_OK.clear()
|
||||
try:
|
||||
response = remote_modal_app_module._invoke_modal_payload_blocking(
|
||||
{"component_id": "component-1"},
|
||||
@@ -2923,6 +2927,7 @@ def test_remote_modal_auto_deploys_missing_app_by_default(
|
||||
finally:
|
||||
remote_modal_app_module.get_settings.cache_clear()
|
||||
remote_modal_app_module._MODAL_AUTO_DEPLOY_STATES.clear()
|
||||
remote_modal_app_module._MODAL_REMOTE_APP_VERSION_OK.clear()
|
||||
|
||||
assert response == b"remote-response"
|
||||
assert deploy_calls == [("comfy-modal-sync", None)]
|
||||
@@ -2951,6 +2956,9 @@ def test_remote_modal_redeploys_when_cached_app_was_deleted(
|
||||
"""Minimal deployed remote engine instance."""
|
||||
|
||||
execute_payload = FakeExecuteMethod()
|
||||
runtime_version = types.SimpleNamespace(
|
||||
remote=lambda: {"protocol_version": remote_modal_app_module._REMOTE_APP_PROTOCOL_VERSION}
|
||||
)
|
||||
|
||||
class FakeApp:
|
||||
"""Minimal deployable cloud app double."""
|
||||
@@ -3000,6 +3008,7 @@ def test_remote_modal_redeploys_when_cached_app_was_deleted(
|
||||
monkeypatch.setenv("MODAL_ENVIRONMENT", "main")
|
||||
remote_modal_app_module.get_settings.cache_clear()
|
||||
remote_modal_app_module._MODAL_AUTO_DEPLOY_STATES.clear()
|
||||
remote_modal_app_module._MODAL_REMOTE_APP_VERSION_OK.clear()
|
||||
remote_modal_app_module._MODAL_AUTO_DEPLOY_STATES[("comfy-modal-sync", "main")] = (
|
||||
remote_modal_app_module._ModalAutoDeployState(ready=True)
|
||||
)
|
||||
@@ -3011,6 +3020,7 @@ def test_remote_modal_redeploys_when_cached_app_was_deleted(
|
||||
finally:
|
||||
remote_modal_app_module.get_settings.cache_clear()
|
||||
remote_modal_app_module._MODAL_AUTO_DEPLOY_STATES.clear()
|
||||
remote_modal_app_module._MODAL_REMOTE_APP_VERSION_OK.clear()
|
||||
|
||||
assert response == b"remote-response"
|
||||
assert deploy_calls == [("comfy-modal-sync", "main")]
|
||||
@@ -3034,6 +3044,9 @@ def test_remote_modal_redeploys_when_deployed_handle_disappears_during_payload_i
|
||||
"""Record whether this engine should fail once as a stale handle."""
|
||||
self._stale = stale
|
||||
self.execute_payload = types.SimpleNamespace(remote=self._remote)
|
||||
self.runtime_version = types.SimpleNamespace(
|
||||
remote=lambda: {"protocol_version": remote_modal_app_module._REMOTE_APP_PROTOCOL_VERSION}
|
||||
)
|
||||
|
||||
def _remote(self, payload: dict[str, Any], kwargs_payload: bytes) -> bytes:
|
||||
"""Raise the missing-app error for stale handles and otherwise succeed."""
|
||||
@@ -3094,6 +3107,7 @@ def test_remote_modal_redeploys_when_deployed_handle_disappears_during_payload_i
|
||||
monkeypatch.setenv("MODAL_ENVIRONMENT", "main")
|
||||
remote_modal_app_module.get_settings.cache_clear()
|
||||
remote_modal_app_module._MODAL_AUTO_DEPLOY_STATES.clear()
|
||||
remote_modal_app_module._MODAL_REMOTE_APP_VERSION_OK.clear()
|
||||
try:
|
||||
response = remote_modal_app_module._invoke_modal_payload_blocking(
|
||||
{"component_id": "component-1"},
|
||||
@@ -3102,6 +3116,7 @@ def test_remote_modal_redeploys_when_deployed_handle_disappears_during_payload_i
|
||||
finally:
|
||||
remote_modal_app_module.get_settings.cache_clear()
|
||||
remote_modal_app_module._MODAL_AUTO_DEPLOY_STATES.clear()
|
||||
remote_modal_app_module._MODAL_REMOTE_APP_VERSION_OK.clear()
|
||||
|
||||
assert response == b"remote-response"
|
||||
assert deploy_calls == [("comfy-modal-sync", "main")]
|
||||
@@ -3125,6 +3140,9 @@ def test_remote_modal_redeploys_when_deployed_handle_disappears_during_warmup(
|
||||
"""Expose a warmup method that fails for stale handles."""
|
||||
self._stale = stale
|
||||
self.warmup_for_request = types.SimpleNamespace(remote=self._remote)
|
||||
self.runtime_version = types.SimpleNamespace(
|
||||
remote=lambda: {"protocol_version": remote_modal_app_module._REMOTE_APP_PROTOCOL_VERSION}
|
||||
)
|
||||
|
||||
def _remote(self, payload: dict[str, Any]) -> dict[str, Any]:
|
||||
"""Raise the missing-app error for stale handles and otherwise succeed."""
|
||||
@@ -3184,6 +3202,7 @@ def test_remote_modal_redeploys_when_deployed_handle_disappears_during_warmup(
|
||||
monkeypatch.setenv("MODAL_ENVIRONMENT", "main")
|
||||
remote_modal_app_module.get_settings.cache_clear()
|
||||
remote_modal_app_module._MODAL_AUTO_DEPLOY_STATES.clear()
|
||||
remote_modal_app_module._MODAL_REMOTE_APP_VERSION_OK.clear()
|
||||
try:
|
||||
response = remote_modal_app_module._invoke_modal_warmup_blocking(
|
||||
{"component_id": "component-1::warmup:0"},
|
||||
@@ -3191,11 +3210,122 @@ def test_remote_modal_redeploys_when_deployed_handle_disappears_during_warmup(
|
||||
finally:
|
||||
remote_modal_app_module.get_settings.cache_clear()
|
||||
remote_modal_app_module._MODAL_AUTO_DEPLOY_STATES.clear()
|
||||
remote_modal_app_module._MODAL_REMOTE_APP_VERSION_OK.clear()
|
||||
|
||||
assert response == {"component_id": "component-1::warmup:0"}
|
||||
assert deploy_calls == [("comfy-modal-sync", "main")]
|
||||
|
||||
|
||||
def test_remote_modal_replaces_out_of_date_deployed_app(
|
||||
remote_modal_app_module: Any,
|
||||
monkeypatch: Any,
|
||||
) -> None:
|
||||
"""A deployed app with an old protocol should be stopped and auto-deployed again."""
|
||||
|
||||
class FakeLookupError(Exception):
|
||||
"""Stand-in for Modal deployed lookup failures."""
|
||||
|
||||
deploy_calls: list[tuple[str | None, str | None]] = []
|
||||
stop_calls: list[str] = []
|
||||
|
||||
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."""
|
||||
del payload, kwargs_payload
|
||||
return b"remote-response"
|
||||
|
||||
class FakeRemoteEngine:
|
||||
"""Minimal deployed remote engine with protocol metadata."""
|
||||
|
||||
def __init__(self, *, current: bool) -> None:
|
||||
"""Record whether this engine should report the current protocol."""
|
||||
protocol_version = (
|
||||
remote_modal_app_module._REMOTE_APP_PROTOCOL_VERSION
|
||||
if current
|
||||
else remote_modal_app_module._REMOTE_APP_PROTOCOL_VERSION - 1
|
||||
)
|
||||
self.execute_payload = FakeExecuteMethod()
|
||||
self.runtime_version = types.SimpleNamespace(
|
||||
remote=lambda: {"protocol_version": protocol_version}
|
||||
)
|
||||
|
||||
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 make subsequent lookups current."""
|
||||
deploy_calls.append((name, environment_name))
|
||||
FakeModal.deployed = True
|
||||
FakeModal.current = True
|
||||
return self
|
||||
|
||||
class FakeModal:
|
||||
"""Minimal Modal SDK double with an initially stale deployed app."""
|
||||
|
||||
deployed = True
|
||||
current = 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 the stale app first and the current replacement after deploy."""
|
||||
del app_name, class_name
|
||||
if not FakeModal.deployed:
|
||||
raise FakeLookupError("Lookup failed for Cls 'RemoteEngine': not deployed")
|
||||
return lambda **kwargs: FakeRemoteEngine(current=FakeModal.current)
|
||||
|
||||
@staticmethod
|
||||
def enable_output() -> Any:
|
||||
"""Provide a no-op output context manager."""
|
||||
return nullcontext()
|
||||
|
||||
def fake_stop_app(app_name: str) -> None:
|
||||
"""Record that stale app replacement stopped the old deployment."""
|
||||
stop_calls.append(app_name)
|
||||
FakeModal.deployed = False
|
||||
|
||||
monkeypatch.setattr(remote_modal_app_module, "modal", FakeModal)
|
||||
monkeypatch.setattr(remote_modal_app_module, "_stop_modal_app_for_replacement", fake_stop_app)
|
||||
monkeypatch.setattr(
|
||||
remote_modal_app_module,
|
||||
"_load_modal_cloud_module",
|
||||
lambda: types.SimpleNamespace(app=FakeApp()),
|
||||
)
|
||||
monkeypatch.setenv("COMFY_MODAL_AUTO_DEPLOY", "true")
|
||||
monkeypatch.setenv("MODAL_ENVIRONMENT", "main")
|
||||
remote_modal_app_module.get_settings.cache_clear()
|
||||
remote_modal_app_module._MODAL_AUTO_DEPLOY_STATES.clear()
|
||||
remote_modal_app_module._MODAL_REMOTE_APP_VERSION_OK.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_DEPLOY_STATES.clear()
|
||||
remote_modal_app_module._MODAL_REMOTE_APP_VERSION_OK.clear()
|
||||
|
||||
assert response == b"remote-response"
|
||||
assert stop_calls == ["comfy-modal-sync"]
|
||||
assert deploy_calls == [("comfy-modal-sync", "main")]
|
||||
|
||||
|
||||
def test_remote_modal_auto_deploy_is_shared_across_concurrent_first_run_callers(
|
||||
remote_modal_app_module: Any,
|
||||
monkeypatch: Any,
|
||||
@@ -3230,6 +3360,9 @@ def test_remote_modal_auto_deploy_is_shared_across_concurrent_first_run_callers(
|
||||
|
||||
execute_payload = FakeExecuteMethod()
|
||||
warmup_for_request = FakeWarmupMethod()
|
||||
runtime_version = types.SimpleNamespace(
|
||||
remote=lambda: {"protocol_version": remote_modal_app_module._REMOTE_APP_PROTOCOL_VERSION}
|
||||
)
|
||||
|
||||
class FakeApp:
|
||||
"""Minimal deployable cloud app double."""
|
||||
@@ -3289,6 +3422,7 @@ def test_remote_modal_auto_deploy_is_shared_across_concurrent_first_run_callers(
|
||||
monkeypatch.setenv("MODAL_ENVIRONMENT", "main")
|
||||
remote_modal_app_module.get_settings.cache_clear()
|
||||
remote_modal_app_module._MODAL_AUTO_DEPLOY_STATES.clear()
|
||||
remote_modal_app_module._MODAL_REMOTE_APP_VERSION_OK.clear()
|
||||
|
||||
try:
|
||||
payload_response: list[bytes] = []
|
||||
@@ -3328,6 +3462,7 @@ def test_remote_modal_auto_deploy_is_shared_across_concurrent_first_run_callers(
|
||||
finally:
|
||||
remote_modal_app_module.get_settings.cache_clear()
|
||||
remote_modal_app_module._MODAL_AUTO_DEPLOY_STATES.clear()
|
||||
remote_modal_app_module._MODAL_REMOTE_APP_VERSION_OK.clear()
|
||||
|
||||
assert thread_errors == []
|
||||
assert payload_response == [b"remote-response"]
|
||||
|
||||
Reference in New Issue
Block a user