Skip to content

Commit 433f44e

Browse files
anth-volkclaude
andcommitted
Correct documentation drift and share one revision helper in the tests
- The persistence README's test commands and the Alembic configuration's example now use --frozen, as every install and test run must. - DEPLOYMENT.md said the candidate smoke asks again "up to six times"; the default of six is six requests in all. - The deploy script's comment on the Stage 12 wrapper now says what is separate (the wrapper) and what both blocks still use (the secret lookups that predate the split). - The migration-cycle and migration-script tests read the applied revision through one helper in migration_support instead of two copies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent d121830 commit 433f44e

7 files changed

Lines changed: 37 additions & 37 deletions

File tree

‎.github/scripts/deploy-cloud-run-simulation-entry.sh‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,9 @@ add_runtime_secret() {
7070
runtime_secrets+=("${variable_name}=${resolved_reference}")
7171
}
7272

73-
# The Stage 12 comparison path's own copy, so it shares no deploy helper with
74-
# the main path and is removed with it.
73+
# The Stage 12 comparison path's own wrapper, removed with it, so the main
74+
# path's wrapper can change without touching the Stage 12 block. Both use the
75+
# secret lookups above, which predate the split.
7576
add_stage12_runtime_secret() {
7677
local variable_name="$1"
7778
local resolved_reference

‎libs/policyengine-simulation-persistence/README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,8 @@ the same time rebases onto the new head after the first merges.
102102
## Tests
103103

104104
```bash
105-
uv sync --extra test
106-
uv run pytest tests/
105+
uv sync --frozen --extra test
106+
uv run --frozen pytest tests/
107107
```
108108

109109
Tests marked `postgres` need a disposable PostgreSQL server. Point

‎libs/policyengine-simulation-persistence/tests/migration_support.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@
77

88
from alembic import command
99
from alembic.config import Config
10+
from policyengine_simulation_persistence import (
11+
create_simulation_engine,
12+
read_schema_revision,
13+
)
1014

1115
if TYPE_CHECKING:
1216
from collections.abc import Callable
@@ -59,3 +63,13 @@ def migration_cycle_failures(config: Config, *, check: bool) -> list[str]:
5963
if failure is not None:
6064
return [failure]
6165
return []
66+
67+
68+
def applied_revision(database_url: str) -> str | None:
69+
"""Return the revision the database at ``database_url`` has applied."""
70+
engine = create_simulation_engine(database_url)
71+
try:
72+
with engine.connect() as connection:
73+
return read_schema_revision(connection)
74+
finally:
75+
engine.dispose()

‎libs/policyengine-simulation-persistence/tests/test_migration_cycle.py‎

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,30 +10,21 @@
1010
MIGRATOR_ROLE,
1111
REPOSITORY_ROOT,
1212
alembic_config,
13+
applied_revision,
1314
chain_config,
1415
migration_cycle_failures,
1516
)
1617
from policyengine_simulation_persistence import (
1718
HEAD_REVISION,
1819
SIMULATION_SCHEMA,
1920
create_simulation_engine,
20-
read_schema_revision,
2121
)
2222
from sqlalchemy import create_engine, inspect, text
2323
from sqlalchemy.exc import OperationalError, ProgrammingError
2424

2525
pytestmark = pytest.mark.postgres
2626

2727

28-
def _revision(database_url: str) -> str | None:
29-
engine = create_simulation_engine(database_url)
30-
try:
31-
with engine.connect() as connection:
32-
return read_schema_revision(connection)
33-
finally:
34-
engine.dispose()
35-
36-
3728
def _superuser_engine(database, **options):
3829
return create_engine(
3930
database.superuser_url.replace("postgresql://", "postgresql+psycopg://", 1),
@@ -45,10 +36,10 @@ def test_upgrade_check_downgrade_upgrade_cycle(disposable_database) -> None:
4536
config = alembic_config()
4637

4738
assert migration_cycle_failures(config, check=True) == []
48-
assert _revision(disposable_database.migrator_url) == HEAD_REVISION
39+
assert applied_revision(disposable_database.migrator_url) == HEAD_REVISION
4940

5041
command.downgrade(config, "base")
51-
assert _revision(disposable_database.migrator_url) is None
42+
assert applied_revision(disposable_database.migrator_url) is None
5243

5344

5445
def test_rule_proof_migration_cycle(disposable_database) -> None:
@@ -93,7 +84,7 @@ def test_the_version_table_lives_only_in_the_simulation_schema(
9384

9485

9586
def test_an_unmigrated_database_reports_no_revision(disposable_database) -> None:
96-
assert _revision(disposable_database.migrator_url) is None
87+
assert applied_revision(disposable_database.migrator_url) is None
9788

9889

9990
def _precreate_schema_without_database_create(database) -> None:
@@ -130,16 +121,16 @@ def test_a_precreated_schema_needs_no_database_create_privilege(
130121

131122
command.upgrade(alembic_config(), "head")
132123

133-
assert _revision(disposable_database.migrator_url) == HEAD_REVISION
134-
assert _revision(disposable_database.runtime_url) == HEAD_REVISION
124+
assert applied_revision(disposable_database.migrator_url) == HEAD_REVISION
125+
assert applied_revision(disposable_database.runtime_url) == HEAD_REVISION
135126

136127

137128
def test_the_runtime_role_reads_the_revision_but_cannot_change_the_schema(
138129
disposable_database,
139130
) -> None:
140131
command.upgrade(alembic_config(), "head")
141132

142-
assert _revision(disposable_database.runtime_url) == HEAD_REVISION
133+
assert applied_revision(disposable_database.runtime_url) == HEAD_REVISION
143134
engine = create_simulation_engine(disposable_database.runtime_url)
144135
try:
145136
with (

‎libs/policyengine-simulation-persistence/tests/test_migration_script.py‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@
77
from pathlib import Path
88

99
import pytest
10-
from migration_support import MIGRATION_DATABASE_URL_VARIABLE, REPOSITORY_ROOT
10+
from migration_support import (
11+
MIGRATION_DATABASE_URL_VARIABLE,
12+
REPOSITORY_ROOT,
13+
applied_revision,
14+
)
1115
from policyengine_simulation_persistence import (
1216
HEAD_REVISION,
13-
create_simulation_engine,
14-
read_schema_revision,
1517
)
1618

1719
pytestmark = pytest.mark.postgres
@@ -35,26 +37,17 @@ def _run_script(database, mode: str) -> subprocess.CompletedProcess[str]:
3537
)
3638

3739

38-
def _revision(database) -> str | None:
39-
engine = create_simulation_engine(database.migrator_url)
40-
try:
41-
with engine.connect() as connection:
42-
return read_schema_revision(connection)
43-
finally:
44-
engine.dispose()
45-
46-
4740
def test_verify_mode_refuses_a_database_behind_the_chain(disposable_database) -> None:
4841
result = _run_script(disposable_database, "verify")
4942

5043
assert result.returncode != 0
51-
assert _revision(disposable_database) is None
44+
assert applied_revision(disposable_database.migrator_url) is None
5245

5346

5447
def test_upgrade_mode_migrates_and_then_verify_mode_passes(disposable_database) -> None:
5548
upgraded = _run_script(disposable_database, "upgrade")
5649
assert upgraded.returncode == 0, upgraded.stderr
57-
assert _revision(disposable_database) == HEAD_REVISION
50+
assert applied_revision(disposable_database.migrator_url) == HEAD_REVISION
5851

5952
verified = _run_script(disposable_database, "verify")
6053
assert verified.returncode == 0, verified.stderr

‎migrations/alembic.ini‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# The Simulation Entrypoint's only Alembic chain: the `simulation` schema.
22
# Run it from the persistence library's environment, for example:
3-
# uv run --project libs/policyengine-simulation-persistence --extra migrations \
3+
# uv run --frozen --project libs/policyengine-simulation-persistence --extra migrations \
44
# alembic -c migrations/alembic.ini upgrade head
55
# The database URL comes only from SIMULATION_MIGRATION_DATABASE_URL.
66
# Ownership rules: libs/policyengine-simulation-persistence/README.md

‎projects/policyengine-simulation-entry/DEPLOYMENT.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,9 @@ read it. Modal never receives either URL.
4949
image expects, under `simulation_database`. The database does not change the
5050
readiness status, because no old-gateway or Stage 12 route uses it. The
5151
candidate smoke check requires the two revisions to match, which shows that the
52-
runtime secret reached the revision and the migration ran. It asks again, up to
53-
six times, while a cold candidate has not reached its database yet; the stable and
52+
runtime secret reached the revision and the migration ran. While a cold
53+
candidate has not reached its database yet, it asks again, up to six requests
54+
in all; the stable and
5455
rollback checks do not, because an older image may run against a newer schema.
5556
The ownership rules and roles are described in
5657
`libs/policyengine-simulation-persistence/README.md`.

0 commit comments

Comments
 (0)