Skip to content

Commit d08030d

Browse files
committed
Address review: restore remaining invariant comments, convert root to def, add dev stress-test runbook
- Restore the five invariant comment blocks dropped by the _sync_update_assignments extraction (no-op updated_at rationale, full-replacement-set semantics, valid_community_ids bootstrap path, comments None-vs-[] semantics, assignments_updated_at staleness contract) - Convert root() to def, matching the convention comment above it - stress_test/README.md: "Dev stack run" section — env overrides, capacity sizing, seed-slug precheck
1 parent 2ba17df commit d08030d

2 files changed

Lines changed: 50 additions & 1 deletion

File tree

backend/app/main.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ def duplicate_document_comments(
278278

279279

280280
@app.get("/")
281-
async def root():
281+
def root():
282282
return {"message": "Hello World"}
283283

284284

@@ -790,6 +790,9 @@ def _sync_update_assignments(
790790
status_code=status.HTTP_409_CONFLICT,
791791
detail="Document has been updated since the last update",
792792
)
793+
# Track whether anything actually changed so we can skip the updated_at bump on
794+
# true no-op requests (which would otherwise break optimistic concurrency for
795+
# other clients).
793796
mutated = False
794797

795798
# Snapshot pre-existing district-mode assignments to compute which zones
@@ -814,6 +817,10 @@ def _sync_update_assignments(
814817
{"document_id": document_id},
815818
)
816819

820+
# The assignments field is always a full replacement set:
821+
# [] means "delete all assignments" (user cleared everything)
822+
# [...] means "replace with these assignments"
823+
# Always DELETE existing rows, then INSERT new ones if any.
817824
delete_result = session.connection().execute(
818825
text(f"DELETE FROM {assignment_table} WHERE document_id = :document_id"),
819826
{"document_id": document_id},
@@ -823,6 +830,12 @@ def _sync_update_assignments(
823830
inserted_count = 0
824831
has_assignments = len(assignments) > 0
825832
if has_assignments:
833+
# For community maps, build the set of valid community_ids so we can reject
834+
# orphan-producing writes before they hit the table. 0 is the "unassigned"
835+
# sentinel; positive ids must exist in the effective metadata list. Skip the
836+
# check entirely when no metadata has been established yet (either in this
837+
# request or previously persisted) — that's the bootstrap path where the UI
838+
# writes assignments before the metadata save lands.
826839
valid_community_ids: set[int] | None = None
827840
if is_community_map:
828841
if validated_community_metadata is not None:
@@ -978,6 +991,7 @@ def _sync_update_assignments(
978991
):
979992
mutated = True
980993

994+
# Sync scoped comments via comments schema (None = no change, [] = delete all)
981995
if data.comments is not None:
982996
comment_inputs: list[DistrictCommentInput] = []
983997
for c in data.comments:
@@ -1050,10 +1064,16 @@ def _sync_update_assignments(
10501064
if mutated:
10511065
updated_at = update_timestamp(session, document_id)
10521066
else:
1067+
# No-op request (e.g. assignments=[] on an already-empty doc with no metadata
1068+
# or comment changes). Keep updated_at pinned to its current value so other
1069+
# clients' optimistic-concurrency windows aren't invalidated.
10531070
updated_at = session.exec(
10541071
select(Document.updated_at).where(Document.document_id == document_id)
10551072
).one()
10561073
if dirty_zones:
1074+
# Bump assignments_updated_at so /stats can tell that the CDN object
1075+
# is stale and republish, even on the path that doesn't otherwise
1076+
# change document.updated_at.
10571077
session.connection().execute(
10581078
text(
10591079
"UPDATE document.document SET assignments_updated_at = NOW() "

backend/stress_test/README.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,35 @@ var. `stress-test-seed` uses no CDN override, so it reads the real
179179
`stress-test/config.json`; it aborts before creating anything if any config
180180
slug is missing from the prod `districtrmap` table.)
181181

182+
## Dev stack run
183+
184+
The prod defaults above are all env overrides — no script changes needed. Dev
185+
resources use the same `Name`-tag scheme with the `districtr-dev` prefix, so
186+
provisioning/discovery works identically:
187+
188+
```sh
189+
# provision.sh / teardown.sh
190+
export STACK_PREFIX=districtr-dev
191+
# run.sh + ECS Exec one-liners
192+
export BASE_URL=https://api.dev.districtr.org
193+
export CLUSTER=$(cd ../../infra && pulumi stack select dev >/dev/null && pulumi stack output clusterName)
194+
export RESULTS_BUCKET=$(cd ../../infra && pulumi stack output s3BucketName 2>/dev/null || true) # dev backend bucket
195+
```
196+
197+
Caveats specific to dev:
198+
199+
- **Capacity**: dev runs 1–2 backend tasks at half prod CPU and a
200+
`db.t4g.small` (vs `large`) — roughly ⅛–¼ of prod capacity. Size runs with
201+
`SCALE` accordingly (e.g. `SCALE=0.25` ≈ 3,200 users); a full-scale run
202+
saturates the stack and the failure noise drowns whatever you were
203+
measuring.
204+
- **Seed slugs**: `stress-test-seed` validates the config JSON's slugs against
205+
the target DB's `districtrmap` table and aborts if any is missing. Dev
206+
shares the prod CDN config by default — verify the 10 slugs exist in the
207+
dev DB first (or point `STRESS_CONFIG_URL` at a dev-specific config).
208+
- **Comparisons**: only compare runs against the *same* stack at the same
209+
`SCALE`. Dev numbers are not comparable to prod numbers.
210+
182211
## Runner (ephemeral prod EC2)
183212
184213
Scripts in `runner/`. `provision.sh`/`teardown.sh` run on the operator's

0 commit comments

Comments
 (0)