Skip to content

Commit c90abf0

Browse files
authored
Merge pull request #942 from PolicyEngine/file-length-split-simulation-runtime
Split the simulation runtime and its output builder tests under the file-length limit
2 parents 7913229 + 382e5dc commit c90abf0

36 files changed

Lines changed: 3396 additions & 3204 deletions

‎changelog_entry.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
- Ran Stage 12 US national simulations as 20 region-group calls each, and built an independent, immutable Stage 12 cache of three annual US datasets and 60 current-law baselines before each deployment, publishing it only after a recorded determinism check and recording its release in the v2 version manifest under a new key, while the existing key keeps a copy that earlier readers accept
1717
- Made the budget refuse a household count outside its country's plausible range (above 200 million in the US or 60 million in the UK, negative or not finite), raising ImplausibleEntityCountError instead of publishing a count like #767's
1818
changed:
19+
- Moved the worker's credential setup, request year and policy parsing, and region resolution out of simulation_runtime.py (1,111 lines, now 651) into gcp_credentials.py, request_policy.py and region_resolution.py; simulation_runtime.py keeps dataset selection and loading, simulation building, local-area breakdowns and the run itself, which tests replace by name there, and re-exports the moved names, so every caller still imports them from simulation_runtime; the shared helpers _parse_year, _normalise_policy, _compile_interval_policy, _runtime_policy (now wrapper_policy), _resolve_region, _resolve_dataset_reference and _require_local_area_breakdown are now public, and the credential messages still log under the simulation_runtime logger; the 3,048-line output builder test file is split into seven test files and a module of shared fakes, all named test_simulation_output_*.py so the weighted-frame rule still checks them
1920
- Split the artifact precompute (precompute.py, 931 lines) and the Stage 12 cache precompute (stage12_cache/precompute.py, 844 lines) each into planning, dataset-building and baseline modules, with the manifest and run_precompute left in the original module, which re-exports every public name, so the Modal precompute apps and their tests import from the same paths; the cache precompute's _scoped_planned_simulation and _baseline_identity are now public, and the worker validation imports scoped_planned_simulation from its new module; the 1,835-line precompute test file is split into four, and the cache precompute tests patch each name in the module whose code reads it
2021
- Split the 4,709-line Stage 12 runtime test file into thirteen test files by behaviour (dataset loading, provenance, the single worker and its failures, aggregates, the coordinator and its failures, runtime store retries, failure records, cancellation, child state errors and the wait window) and three support modules for the shared builders, fake stores and fake child calls, whose helpers are now public; the other Stage 12 tests import them from those modules, every test keeps its name and body, and two parametrized cases are named after the renamed bundle helper
2122
- Split Stage 12 report coordination (stage12_runtime/coordination.py, 1,091 lines) into modules for the report coordinator class, its child simulations' records, failures and cancellation, and the parent report's checks and claim, with coordinate_report and every public name still in coordination.py and child failures still logged under its logger; and turned the contract's stage12_execution.py (929 lines) into a stage12_execution package of modules for the shared field types and simulation input, report inputs and output plans, result artifacts, and comparison-run records, which re-exports every name from the same import path, with unchanged JSON schemas; models subclass the base models through the package path ruff.toml lists, so no ruff setting changes

‎docs/generated/rules.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ Every rule that governs `policyengine-sim-api` is listed here with how it is enf
6666

6767
- `sim.ci.multiline-run-is-script`: 13 remaining in `rules/baselines/ci-multiline-run.txt`
6868
- `sim.microdata.weight-stripping-confined`: 0 remaining in `rules/baselines/sim-microdata-weight-stripping.txt`
69-
- `sim.quality.python-file-length`: 4 remaining in `rules/baselines/python-file-length.txt`
69+
- `sim.quality.python-file-length`: 2 remaining in `rules/baselines/python-file-length.txt`
7070

7171
### Exceptions and review dates
7272

‎projects/policyengine-simulation-executor/src/modal/segmented_national.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -581,7 +581,7 @@ def _reduce(
581581
self, child_results: list[dict], *, country_module: ModuleType
582582
) -> dict[str, Any]:
583583
from policyengine_simulation_executor.simulation_runtime import ( # noqa: PLC0415 # Modal worker code: bound per call, like this module's other simulation_runtime imports
584-
_parse_year,
584+
parse_year,
585585
)
586586

587587
# The canonical internal-key stripper, plus the opt-out knob.
@@ -597,7 +597,7 @@ def _reduce(
597597
country=self.country,
598598
simulation_params=simulation_params,
599599
country_module=country_module,
600-
year=_parse_year(simulation_params),
600+
year=parse_year(simulation_params),
601601
resolved_data_version=None,
602602
),
603603
runtime=self.runtime,
@@ -606,7 +606,7 @@ def _reduce(
606606
combine_spm_results(
607607
child_results,
608608
simulation_params.get("spm"),
609-
expected_year=_parse_year(simulation_params),
609+
expected_year=parse_year(simulation_params),
610610
)
611611
)
612612
for key in ("model_version", "data_version"):
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
"""Write the worker's Google Cloud credentials from its environment."""
2+
3+
from __future__ import annotations
4+
5+
import contextlib
6+
import json
7+
import logging
8+
import os
9+
import tempfile
10+
from typing import TYPE_CHECKING
11+
12+
from policyengine_simulation_observability import scopes
13+
from policyengine_simulation_observability.stages import ANNUAL_IMPACT_STAGES, Stage
14+
15+
if TYPE_CHECKING:
16+
from collections.abc import Iterator
17+
18+
from policyengine_observability import ObservabilityRuntime
19+
20+
# Moved out of simulation_runtime.py; it still logs under that module's name.
21+
logger = logging.getLogger("policyengine_simulation_executor.simulation_runtime")
22+
23+
# Credential-adjacent environment values are cut to this many characters.
24+
_ENV_VALUE_PREVIEW_CHARS = 50
25+
26+
27+
def _normalize_credentials_blob(creds_json: str) -> str:
28+
"""Return the raw JSON blob, decoding the outer escape if present.
29+
30+
The upstream Modal secret sometimes stores the credentials payload
31+
double-encoded (the entire JSON object is wrapped in quotes with
32+
backslash-escaped interior quotes). Historically we always attempted
33+
the unescape as a fallback which could accidentally parse an already
34+
clean blob. Only unwrap when the payload looks wrapped.
35+
"""
36+
try:
37+
json.loads(creds_json)
38+
except json.JSONDecodeError:
39+
looks_escaped = creds_json.lstrip().startswith('"') or '\\"' in creds_json
40+
if looks_escaped:
41+
return json.loads(f'"{creds_json}"')
42+
raise
43+
return creds_json
44+
45+
46+
@contextlib.contextmanager
47+
def setup_gcp_credentials(
48+
runtime: ObservabilityRuntime | None = None,
49+
) -> Iterator[None]:
50+
"""Set up GCP credentials from environment variable.
51+
52+
Modal secrets are injected as environment variables. The GCP library
53+
expects GOOGLE_APPLICATION_CREDENTIALS to point to a file path. If
54+
credentials JSON is provided, write it to a temp file that's deleted
55+
on exit. This runs as a context manager to guarantee cleanup even if
56+
the caller raises mid-simulation; the previous fire-and-forget
57+
``tempfile.mkstemp`` path leaked credential material on disk every
58+
time a container served a request.
59+
"""
60+
previous = os.environ.get("GOOGLE_APPLICATION_CREDENTIALS")
61+
creds_file = None
62+
with contextlib.ExitStack() as credentials_file_stack:
63+
try:
64+
credential_span = (
65+
scopes.span(runtime, ANNUAL_IMPACT_STAGES.name(Stage.CREDENTIAL_SETUP))
66+
if runtime is not None
67+
else contextlib.nullcontext()
68+
)
69+
with credential_span:
70+
# Log available GCP-related env vars for debugging.
71+
gcp_vars = {
72+
k: v[:_ENV_VALUE_PREVIEW_CHARS] + "..."
73+
if len(v) > _ENV_VALUE_PREVIEW_CHARS
74+
else v
75+
for k, v in os.environ.items()
76+
if "GOOGLE" in k or "GCP" in k or "CREDENTIAL" in k
77+
}
78+
logger.info("GCP-related env vars: %s", list(gcp_vars.keys()))
79+
80+
if previous:
81+
logger.info("GOOGLE_APPLICATION_CREDENTIALS already set")
82+
else:
83+
creds_json = (
84+
os.environ.get("GOOGLE_APPLICATION_CREDENTIALS_JSON")
85+
or os.environ.get("GCP_CREDENTIALS_JSON")
86+
or os.environ.get("GOOGLE_CREDENTIALS")
87+
or os.environ.get("SERVICE_ACCOUNT_JSON")
88+
)
89+
90+
if not creds_json:
91+
logger.warning(
92+
"No GCP credentials found in environment variables"
93+
)
94+
else:
95+
normalized = _normalize_credentials_blob(creds_json)
96+
# The exit stack keeps the file open across the yield,
97+
# and closing it (``delete=True``) removes the file.
98+
# The finally block first restores any prior value of
99+
# ``GOOGLE_APPLICATION_CREDENTIALS`` so a retry in the same
100+
# container doesn't silently pick up a stale path.
101+
creds_file = credentials_file_stack.enter_context(
102+
tempfile.NamedTemporaryFile(
103+
mode="w",
104+
suffix=".json",
105+
delete=True,
106+
)
107+
)
108+
creds_file.write(normalized)
109+
creds_file.flush()
110+
os.environ["GOOGLE_APPLICATION_CREDENTIALS"] = creds_file.name
111+
logger.info("GCP credentials written to %s", creds_file.name)
112+
113+
yield
114+
finally:
115+
if creds_file is not None:
116+
if previous is None:
117+
os.environ.pop("GOOGLE_APPLICATION_CREDENTIALS", None)
118+
else:
119+
os.environ["GOOGLE_APPLICATION_CREDENTIALS"] = previous

‎projects/policyengine-simulation-executor/src/policyengine_simulation_executor/precompute.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
means deleting its object first, then re-running.
2222
2323
Cohort baselines are built through the executor's own request path
24-
(``_resolve_region`` + ``_build_simulation`` on a synthetic child request),
24+
(``resolve_region`` + ``_build_simulation`` on a synthetic child request),
2525
so scoping, extra variables, and the deterministic simulation id match the
2626
production child's by construction, not by convention — and a regression
2727
test asserts the writer and reader derive identical identities. All

‎projects/policyengine-simulation-executor/src/policyengine_simulation_executor/precompute_planning.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,12 @@ def cohort_resolution(
5353
"""
5454
from policyengine_simulation_executor.simulation_runtime import ( # noqa: PLC0415 # resolved per call so tests can patch simulation_runtime helpers
5555
_country_module,
56-
_resolve_region,
56+
resolve_region,
5757
)
5858

5959
params = cohort_params(year, group)
6060
country_module = _country_module(PRECOMPUTE_COUNTRY)
61-
resolution = _resolve_region(
61+
resolution = resolve_region(
6262
country_module=country_module,
6363
country=PRECOMPUTE_COUNTRY,
6464
params=params,
@@ -123,14 +123,14 @@ class StateDatasetTarget:
123123
def state_dataset_targets() -> list[StateDatasetTarget]:
124124
"""Return every US state region that loads a non-default dataset, by state code.
125125
126-
Each state is resolved exactly as a request for it is (``_resolve_region``
126+
Each state is resolved exactly as a request for it is (``resolve_region``
127127
then ``_resolve_dataset_selection``), so a bundle that sends states back to
128128
the default dataset gets no state files.
129129
"""
130130
from policyengine_simulation_executor.simulation_runtime import ( # noqa: PLC0415 # resolved per call so tests can patch simulation_runtime helpers
131131
_country_module,
132132
_resolve_dataset_selection,
133-
_resolve_region,
133+
resolve_region,
134134
)
135135

136136
country_module = _country_module(PRECOMPUTE_COUNTRY)
@@ -141,7 +141,7 @@ def state_dataset_targets() -> list[StateDatasetTarget]:
141141
"region": region.code,
142142
"time_period": PRECOMPUTE_YEARS[0],
143143
}
144-
resolution = _resolve_region(
144+
resolution = resolve_region(
145145
country_module=country_module, country=PRECOMPUTE_COUNTRY, params=params
146146
)
147147
selection = _resolve_dataset_selection(params, region_resolution=resolution)

0 commit comments

Comments
 (0)