Keep deployment settings on hybrid mapped phase payloads
Hybrid mapped execution rebuilt its static, mapped, and cleanup phase payloads from a fixed field list that dropped modal_gpu and modal_max_containers. Those payloads resolved default settings and expected a different runtime fingerprint for the same Modal app, so the sibling-validation guard refused them with ModalRemoteAppOutOfDateError. Define the fingerprint-shaping payload fields once in settings and copy them at every derived-payload site (mapped phases, warmup requests, speculative prewarm targets). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
5f951fd626
commit
01cefd2cc5
@@ -759,3 +759,10 @@ generation streams with an empty `modal.exception.RemoteError`. Fixes:
|
||||
- `_replace_outdated_modal_app` refuses to stop an app whose remote fingerprint
|
||||
this process already deployed or validated, so divergent callers can no longer
|
||||
stop each other's deployments.
|
||||
- Follow-up: hybrid mapped execution builds its static, mapped, and cleanup phase
|
||||
payloads in `_shared_subgraph_payload_fields`, which also dropped `modal_gpu` and
|
||||
`modal_max_containers`. The refusal above caught it, raising
|
||||
`ModalRemoteAppOutOfDateError` instead of killing jobs. All derived payloads now
|
||||
copy `settings.MODAL_DEPLOYMENT_PAYLOAD_FIELDS` through
|
||||
`modal_deployment_payload_fields()`; add any new fingerprint-shaping payload
|
||||
field there.
|
||||
|
||||
@@ -24,6 +24,7 @@ if __package__:
|
||||
_node_output_refs,
|
||||
)
|
||||
from .remote_plan_types import InputTarget, LinkedOutputRef
|
||||
from .settings import MODAL_DEPLOYMENT_PAYLOAD_FIELDS
|
||||
else: # pragma: no cover - flat import inside the Modal container.
|
||||
from component_planning import _component_ancestors_of_local_source
|
||||
from execution_environments import ExecutionProvider
|
||||
@@ -40,6 +41,7 @@ else: # pragma: no cover - flat import inside the Modal container.
|
||||
_node_output_refs,
|
||||
)
|
||||
from remote_plan_types import InputTarget, LinkedOutputRef
|
||||
from settings import MODAL_DEPLOYMENT_PAYLOAD_FIELDS
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -48,8 +50,7 @@ _SPECULATIVE_PREWARM_PAYLOAD_FIELDS = frozenset(
|
||||
{
|
||||
"component_id",
|
||||
"prompt_id",
|
||||
"modal_gpu",
|
||||
"modal_max_containers",
|
||||
*MODAL_DEPLOYMENT_PAYLOAD_FIELDS,
|
||||
"execution_provider",
|
||||
"execution_environment_id",
|
||||
"remote_worker_affinity_group",
|
||||
|
||||
@@ -43,7 +43,7 @@ from .modal_warmup import (
|
||||
_prompt_warmup_head_start_seconds,
|
||||
boost_mapped_component_warmup,
|
||||
)
|
||||
from ..settings import get_settings
|
||||
from ..settings import get_settings, modal_deployment_payload_fields
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -144,6 +144,7 @@ def _shared_subgraph_payload_fields(payload: dict[str, Any]) -> dict[str, Any]:
|
||||
payload.get("terminate_container_on_error", True)
|
||||
),
|
||||
"custom_nodes_bundle": payload.get("custom_nodes_bundle"),
|
||||
**modal_deployment_payload_fields(payload),
|
||||
}
|
||||
snapshot_profile_key = payload.get("snapshot_profile_key")
|
||||
if isinstance(snapshot_profile_key, str) and snapshot_profile_key.strip():
|
||||
|
||||
@@ -16,7 +16,11 @@ import threading
|
||||
import time
|
||||
from typing import Any, Callable, Iterator, Mapping
|
||||
|
||||
from ..settings import get_settings, modal_deployment_app_name
|
||||
from ..settings import (
|
||||
get_settings,
|
||||
modal_deployment_app_name,
|
||||
modal_deployment_payload_fields,
|
||||
)
|
||||
from .local_execution import _is_link, _resolve_required_subgraph_nodes
|
||||
from .modal_deployment import (
|
||||
ModalRemoteInvocationError,
|
||||
@@ -542,8 +546,8 @@ def _build_prompt_warmup_request(payload: dict[str, Any]) -> dict[str, Any] | No
|
||||
payload.get("component_id", "modal-warmup"),
|
||||
)
|
||||
),
|
||||
**modal_deployment_payload_fields(payload),
|
||||
"modal_gpu": settings.modal_gpu,
|
||||
"modal_max_containers": payload.get("modal_max_containers"),
|
||||
"remote_worker_affinity_group": payload.get(
|
||||
"remote_worker_affinity_group"
|
||||
),
|
||||
|
||||
+18
@@ -10,6 +10,7 @@ import re
|
||||
from dataclasses import dataclass, replace
|
||||
from functools import lru_cache
|
||||
from pathlib import Path
|
||||
from typing import Any, Mapping
|
||||
|
||||
if __package__:
|
||||
from .instance_identity import (
|
||||
@@ -250,6 +251,23 @@ def modal_deployment_app_name(settings: ModalSyncSettings) -> str:
|
||||
return f"{prefix}-{digest}{suffix}"
|
||||
|
||||
|
||||
MODAL_DEPLOYMENT_PAYLOAD_FIELDS = ("modal_gpu", "modal_max_containers")
|
||||
"""Payload fields that select deployment settings and so shape the runtime fingerprint.
|
||||
|
||||
Any payload derived from another (warmups, prewarm targets, mapped phases) must
|
||||
copy these, or it will expect a different fingerprint for the same Modal app.
|
||||
"""
|
||||
|
||||
|
||||
def modal_deployment_payload_fields(payload: Mapping[str, Any]) -> dict[str, Any]:
|
||||
"""Return the deployment-identity fields present on one payload."""
|
||||
return {
|
||||
field_name: payload[field_name]
|
||||
for field_name in MODAL_DEPLOYMENT_PAYLOAD_FIELDS
|
||||
if payload.get(field_name) is not None
|
||||
}
|
||||
|
||||
|
||||
def _read_path_env(name: str) -> Path | None:
|
||||
"""Resolve an environment variable into a path when present."""
|
||||
value = os.getenv(name)
|
||||
|
||||
@@ -2532,3 +2532,60 @@ def test_trim_subgraph_payload_to_required_nodes_drops_unrelated_mapped_branch(
|
||||
assert trimmed_payload["execute_node_ids"] == ["3"]
|
||||
assert trimmed_payload["mapped_execute_node_ids"] == []
|
||||
assert trimmed_payload["static_execute_node_ids"] == ["3"]
|
||||
|
||||
|
||||
def test_mapped_phase_payloads_preserve_parent_runtime_fingerprint(
|
||||
mapped_execution_module: Any,
|
||||
modal_deployment_module: Any,
|
||||
monkeypatch: Any,
|
||||
) -> None:
|
||||
"""Static, mapped, and cleanup phases must expect the parent's deployed fingerprint."""
|
||||
phase_definition = {
|
||||
"component_node_ids": ["9"],
|
||||
"subgraph_prompt": {},
|
||||
"boundary_inputs": [],
|
||||
"boundary_outputs": [],
|
||||
"execute_node_ids": ["9"],
|
||||
}
|
||||
parent_payload = {
|
||||
"payload_kind": "mapped_subgraph",
|
||||
"component_id": "1",
|
||||
"modal_gpu": "B300",
|
||||
"modal_max_containers": 4,
|
||||
"static_phase": phase_definition,
|
||||
"mapped_phase": phase_definition,
|
||||
"remote_session": {"session_id": "session-1"},
|
||||
"clear_remote_session": True,
|
||||
}
|
||||
derived_payloads = [
|
||||
mapped_execution_module._build_phase_subgraph_payload(
|
||||
parent_payload, "static_phase", "1"
|
||||
),
|
||||
mapped_execution_module._build_phase_subgraph_payload(
|
||||
parent_payload, "mapped_phase", "1__mapped"
|
||||
),
|
||||
]
|
||||
monkeypatch.setattr(
|
||||
mapped_execution_module,
|
||||
"is_remote_session_handle_payload",
|
||||
lambda remote_session: True,
|
||||
)
|
||||
derived_payloads.append(
|
||||
mapped_execution_module._build_remote_session_cleanup_payload(parent_payload)
|
||||
)
|
||||
parent_fingerprint = modal_deployment_module._expected_remote_runtime_fingerprint(
|
||||
parent_payload
|
||||
)
|
||||
|
||||
assert parent_fingerprint != (
|
||||
modal_deployment_module._expected_remote_runtime_fingerprint(
|
||||
{"modal_gpu": "B300"}
|
||||
)
|
||||
)
|
||||
for derived_payload in derived_payloads:
|
||||
assert derived_payload["modal_gpu"] == "B300"
|
||||
assert derived_payload["modal_max_containers"] == 4
|
||||
assert (
|
||||
modal_deployment_module._expected_remote_runtime_fingerprint(derived_payload)
|
||||
== parent_fingerprint
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user