Skip to content

Commit cd5aa73

Browse files
authored
Merge pull request #1526 from PolicyEngine/codex/main-modal-worker-autoscaler
Keep production Modal workers warm
2 parents 2db5cc1 + 37319e5 commit cd5aa73

11 files changed

Lines changed: 121 additions & 15 deletions

File tree

.github/scripts/create_pr.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ Related to #1178
3636
modal_release:
3737
new_app_target: frontier
3838
promote_existing_frontier: true
39-
cleanup_target: none
39+
cleanup_target: retired
4040
\`\`\`
4141
4242
## Version Updates

.github/scripts/modal-deploy-release.sh

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
set -euo pipefail
33

44
config_json="${1:?Usage: modal-deploy-release.sh CONFIG_JSON}"
5-
modal_environment="${MODAL_ENVIRONMENT:-main}"
65
output_file="${GITHUB_OUTPUT:-}"
76

87
require_env() {
@@ -36,10 +35,13 @@ github_output() {
3635
}
3736

3837
require_env \
38+
MODAL_ENVIRONMENT \
3939
USER_ANALYTICS_DB_USERNAME \
4040
USER_ANALYTICS_DB_PASSWORD \
4141
USER_ANALYTICS_DB_CONNECTION_NAME
4242

43+
modal_environment="${MODAL_ENVIRONMENT}"
44+
4345
uv run alembic upgrade head
4446
analytics_database_revision="$(
4547
uv run python -m policyengine_household_api.modal_release.analytics_revision
@@ -67,6 +69,7 @@ bash .github/scripts/modal-sync-secrets.sh
6769
new_app_target="$(config_value new_app_target)"
6870
if [ "${new_app_target}" != "none" ]; then
6971
HOUSEHOLD_MODAL_WORKER_APP_NAME="${worker_app_name}" \
72+
MODAL_ENVIRONMENT="${modal_environment}" \
7073
uv run modal deploy \
7174
--env "${modal_environment}" \
7275
-m policyengine_household_api.modal_release.worker_app
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import os
2+
import subprocess
3+
4+
5+
def test_modal_deploy_release_requires_explicit_modal_environment():
6+
env = {
7+
**os.environ,
8+
"USER_ANALYTICS_DB_USERNAME": "user",
9+
"USER_ANALYTICS_DB_PASSWORD": "password",
10+
"USER_ANALYTICS_DB_CONNECTION_NAME": "project:region:instance",
11+
}
12+
env.pop("MODAL_ENVIRONMENT", None)
13+
14+
result = subprocess.run(
15+
[
16+
"bash",
17+
".github/scripts/modal-deploy-release.sh",
18+
'{"new_app_target":"none","promote_existing_frontier":false,"cleanup_target":"none"}',
19+
],
20+
capture_output=True,
21+
env=env,
22+
text=True,
23+
)
24+
25+
assert result.returncode == 1
26+
assert "MODAL_ENVIRONMENT" in result.stdout

.github/scripts/test_resolve_modal_release_config.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ def test_resolve_release_uses_weekly_default_for_empty_workflow_dispatch():
6969
assert resolved.should_deploy is True
7070
assert resolved.source == "workflow-dispatch-inputs"
7171
assert resolved.config is not None
72+
assert resolved.config.cleanup_target == "retired"
7273

7374

7475
def test_resolve_release_skips_regular_push_even_with_pr_config():
@@ -130,3 +131,5 @@ def test_resolve_release_uses_weekly_default_when_no_pr_body_exists():
130131

131132
assert resolved.should_deploy is True
132133
assert resolved.source == "weekly-default"
134+
assert resolved.config is not None
135+
assert resolved.config.cleanup_target == "retired"

.github/workflows/deploy-staged.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ on:
2323
cleanup_target:
2424
description: "Modal app group to stop after updating the manifest"
2525
required: true
26-
default: none
26+
default: retired
2727
type: choice
2828
options:
2929
- none

changelog.d/1525.changed.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Keep a small warm Modal worker pool for the production environment and clean up retired Modal worker apps by default.

docs/engineering/skills/modal-release-prs.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ block:
1919
modal_release:
2020
new_app_target: frontier
2121
promote_existing_frontier: true
22-
cleanup_target: none
22+
cleanup_target: retired
2323
```
2424
2525
Allowed values:
@@ -40,13 +40,15 @@ different deployment behavior:
4040
modal_release:
4141
new_app_target: frontier
4242
promote_existing_frontier: true
43-
cleanup_target: none
43+
cleanup_target: retired
4444
```
4545

4646
This deploys the new worker to `frontier`, promotes the previous `frontier` to
4747
`current`, and moves the previous `current` into the manifest's `retired`
48-
history. Retired apps are not deleted unless `cleanup_target: retired` is
49-
explicitly configured.
48+
history. The default weekly release shape uses `cleanup_target: retired`, so
49+
apps in the retired history are stopped after the manifest is updated. Use
50+
`cleanup_target: none` only when the user explicitly asks to preserve retired
51+
worker apps after release.
5052

5153
Do not use PR labels, branch names, model-specific tags, or title prefixes to
5254
control Modal release behavior. The PR body YAML block is the source of truth.

policyengine_household_api/modal_release/release_config.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ def default_weekly_config() -> ModalReleaseConfig:
116116
return ModalReleaseConfig(
117117
new_app_target=NewAppTarget.FRONTIER,
118118
promote_existing_frontier=True,
119-
cleanup_target=CleanupTarget.NONE,
119+
cleanup_target=CleanupTarget.RETIRED,
120120
)
121121

122122

policyengine_household_api/modal_release/worker_app.py

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,37 @@
2323
)
2424

2525

26-
@app.function(
27-
image=household_api_worker_image(),
28-
secrets=[household_api_secret()],
29-
timeout=180,
30-
scaledown_window=300,
31-
)
26+
def worker_modal_environment(
27+
modal_environment: str | None = None,
28+
) -> str:
29+
environment = (
30+
modal_environment
31+
if modal_environment is not None
32+
else os.getenv("MODAL_ENVIRONMENT")
33+
)
34+
if not environment:
35+
raise RuntimeError("MODAL_ENVIRONMENT must be set for Modal workers")
36+
return environment
37+
38+
39+
def worker_function_options(
40+
modal_environment: str | None = None,
41+
) -> dict[str, Any]:
42+
environment = worker_modal_environment(modal_environment)
43+
options: dict[str, Any] = {
44+
"image": household_api_worker_image(),
45+
"secrets": [household_api_secret()],
46+
"timeout": 180,
47+
"scaledown_window": 300,
48+
}
49+
if environment == "main":
50+
options["min_containers"] = 3
51+
options["buffer_containers"] = 2
52+
options["scaledown_window"] = 600
53+
return options
54+
55+
56+
@app.function(**worker_function_options())
3257
def handle_household_request(payload: dict[str, Any]) -> dict[str, Any]:
3358
configure_google_credentials()
3459
from policyengine_household_api.api import app as flask_app
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import importlib
2+
3+
import pytest
4+
5+
6+
pytestmark = pytest.mark.usefixtures("worker_app")
7+
8+
9+
@pytest.fixture
10+
def worker_app(monkeypatch):
11+
monkeypatch.setenv("MODAL_ENVIRONMENT", "testing")
12+
from policyengine_household_api.modal_release import worker_app
13+
14+
return importlib.reload(worker_app)
15+
16+
17+
def test_worker_function_options_keep_main_workers_warm(worker_app):
18+
options = worker_app.worker_function_options(modal_environment="main")
19+
20+
assert options["min_containers"] == 3
21+
assert options["buffer_containers"] == 2
22+
assert options["scaledown_window"] == 600
23+
24+
25+
def test_worker_function_options_do_not_keep_staging_workers_warm():
26+
from policyengine_household_api.modal_release.worker_app import (
27+
worker_function_options,
28+
)
29+
30+
options = worker_function_options(modal_environment="staging")
31+
32+
assert "min_containers" not in options
33+
assert "buffer_containers" not in options
34+
assert options["scaledown_window"] == 300
35+
36+
37+
def test_worker_function_options_do_not_keep_workers_warm_without_env(
38+
monkeypatch,
39+
):
40+
monkeypatch.delenv("MODAL_ENVIRONMENT", raising=False)
41+
from policyengine_household_api.modal_release.worker_app import (
42+
worker_function_options,
43+
)
44+
45+
with pytest.raises(RuntimeError, match="MODAL_ENVIRONMENT"):
46+
worker_function_options(modal_environment=None)

0 commit comments

Comments
 (0)