Review three importer patches against data and retry contracts
Application and assignment
The importer now correctly stores finance records and resumes from its checkpoint. Three teams propose changes to storage, retry timing, and logging. A patch can look smaller or faster while weakening a guarantee another module relies on.
Review the three supplied diff files against the working reference. They are proposals, not edits already applied to your checkout. Submit a separate decision for each patch, with a minimal input or schedule that supports it. Keep the answer key closed until you have made those decisions.
Contract and starting evidence
“Three teams want to merge changes before tonight's reconciliation. You have 25 minutes. Which change blocks release, which needs a contract decision, and which can ship? Provide a minimal reproduction before suggesting a rewrite.”
This constructed review is a separate session after the importer. Open PR 101 (download file, source below), PR 102 (download file, source below) and PR 103 (download file, source below). They are proposed patches against the reference, not already applied code. Keep the review key closed.
Read the supplied code · pr-101.diff
--- a/reference/store.py
+++ b/reference/store.py
@@ -16,9 +16,6 @@
def persist(self, rows):
with self.db:
for row in rows:
- old = self.db.execute("SELECT id,cents,currency FROM transactions WHERE id=?", (row[0],)).fetchone()
- if old is not None and old != row:
- raise ValueError("duplicate id with conflicting payload")
self.db.execute("INSERT OR IGNORE INTO transactions VALUES(?,?,?)", row)
def advance(self, cursor):
Read the supplied code · pr-102.diff
--- a/reference/transport.py
+++ b/reference/transport.py
@@ -44,7 +44,7 @@
if attempt == attempts - 1:
raise TimeoutError("attempt budget exhausted")
# This exercise's API specifies numeric seconds, not HTTP-date syntax.
- delay = response.get("retryAfter", 0.1 * 2**attempt)
+ delay = min(response.get("retryAfter", 0.1 * 2**attempt), 0.05)
if isinstance(delay, bool) or not isinstance(delay, (int, float)) or not 0 <= delay < float("inf"):
raise ValueError("invalid Retry-After")
if clock.now + delay >= deadline:
Read the supplied code · pr-103.diff
--- a/reference/transport.py
+++ b/reference/transport.py
@@ -1,4 +1,8 @@
"""Injected transport owns timeout enforcement; importer owns the total budget."""
+import logging
+logger = logging.getLogger(__name__)
+
+
class FakeClock:
def __init__(self):
self.now = 0.0
@@ -34,6 +38,7 @@
if remaining <= 0:
raise TimeoutError("total deadline")
response = server.get(cursor, timeout=remaining)
+ logger.info("import attempt=%d status=%d", attempt + 1, response["status"])
if clock.now >= deadline:
raise TimeoutError("total deadline")
status = response["status"]
PR 101 (storage team) removes payload comparison to save a SELECT. PR 102 (integration
team) caps the partner's retry delay to finish earlier. PR 103 (operations) logs
attempt/status without finance payload. These are applicable unified diffs. After the
assessment, python -m unittest discover -s curriculum/02-applications/04-testing/labs/importer/review -v
from the repository root applies each in a temporary copy: the first two must produce
their named regressions, while the third must emit only the bounded metadata.
| Constraint | Expected evidence |
|---|---|
| Caller | Nightly importer restarts using an existing progress row |
| Input | Repeated transaction a with amount changing 0.29 → 2.00 |
| Invariant | Replays cannot silently change or conceal finance data |
| Output | Three separate review decisions, one executable regression each |
| Excluded | Style-only cleanup and rewriting the API client |
Read the diff and state the claimed benefit. Trace the contract across its caller, construct a counterexample, then give the narrowest release decision. A review comment should contain behavior, input, consequence and a regression; “looks unsafe” is not enough.
a reviewer says all duplicate IDs are harmless. Predict the ledger after the changed amount before opening the key. Expected answer: idempotence requires equality of normalized payload, not just matching ID.
release is in ten minutes and retries overload the partner. Expected answer: stop the faulty optimization, retain bounded behavior, assign the service owner to clarify Retry-After rather than inventing a zero-delay retry. Senior review must reproduce effects; lead review also assigns the contract decision and rollback.