Re-write the Tank API so that it is a smart python wrapper of the actual API #427
Labels
No labels
Kestra
bug
enhancement
someday
subtask
☁️ api
🎛️ infrastructure
🐞 sentry
📆 2025 Season
📝 pages
allpicks
📝 pages
picks
📝 pages
standings
🚀 performance
No milestone
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
johnsturgeon/tgfp-web#427
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Provider boundary: shaping Tank01 data into pool data
Design notes, not a plan. This records why the provider boundary is being
re-cut and which direction the pieces should face, so each step can be scoped
on its own later without re-deriving the reasoning.
Deliberately does not enumerate the steps. Work is scoped one piece at a time,
as each becomes obvious; every piece is expected to be non-breaking and tested
on its own. Nothing here commits to an order or a finish line.
The problem
app/tank01_api/tank01_api.pyfetches Tank01 payloads and shapes them intoNflApiGame, which the ingest job then copies intoGame. That copy(
_game_from_api_game) is close to a 1:1 field mapping, which reads as thoughthe intermediate class is not paying for itself.
A boring mapper is not the problem. The problem is that
NflApiGamewas shapedto resemble
Gamerather than to resemble Tank01, so we maintain two types thatmust change together and get the benefit of neither.
The mapping is also not truly 1:1:
NflApiGamecarries Tank01 team-idstrings, while
Gamecarries FK ints. Identity resolution across a systemboundary is exactly what this layer is for — it is just doing less than it
appears to.
The target shape
Four responsibilities:
decides nothing.
the provider's vocabulary. Dumb on purpose.
gameStatusCodestays a stringcalled
gameStatusCode.code that speaks both languages.
Game, and the upsert that writes it.The industry name for the whole boundary is an anti-corruption layer; the middle
two pieces are the DTO and the mapper.
Three tiers of type fall out of that:
gameStatusCode: str,homeTeamSpread: "+-2.5".primary key, no FKs, no session.
Line,GameFacts, and today'sNflApiScore.Game,Team,Week. Tables, FKs, ids.Tier 2 is what lets mappers be pure. Going DTO →
Gamedirectly means everymapper needs FK lookups, so every mapper needs a session, so every mapper test
needs a database.
GameFactsis the mapper's output; resolving it into a row isthe persistence step's job.
The test for whether the seam is in the right place
Ask which change forces which edit:
provider file is touched.
The second one currently fails. Median-across-books and favorite selection are
pool rules living in
tank01_api.py. Switching providers would mean carryingthem across, which is the outcome the boundary exists to prevent. We already ran
that migration once (ESPN → Tank01), which is the evidence that the boundary
earns its keep here rather than being ceremony.
Where the code lives
Tier 2 lands in two places
app/models/already holds tier-2 objects —WeekInfois a plain dataclasswith no table, and
GameStatus/SeasonType/OddsStateare enums. That isthe right home for shared vocabulary: things routers, templates, jobs and
mappers all speak.
GameFactsis different. It is meaningful only on the write path — produced bya mapper, consumed by an upsert two lines later, seen by nothing else. That
belongs in
app/ingest/facts.py.The line: shared vocabulary →
models/. Ingest-only fact bundles →ingest/.Dependency direction
tank01_api/imports nothing fromapp/models/. It speaks Tank01 and HTTP.ingest/imports both — schemas from the provider, enums and tables frommodels. It is the only layer allowed to.
jobs/owns policy: readsodds_state, decides whether odds are worthfetching, calls the right mapper.
The first rule is mechanically checkable, which is what makes it worth stating:
Empty means the seam is clean. Today it fails by exactly one line —
tank01_api.pyimportsGameStatus, because the provider wrapper currently doesthe
"2"→FINALtranslation. Move that into the mapper and the import goesaway on its own.
The data path, end to end
One week of games, top to bottom.
app/tank01_api/schemas.py— provider vocabulary, verbatim.app/tank01_api/client.py— memoized, week-pinned, returns DTOs and nothing else.app/ingest/facts.py— tier 2. Inert. Carries Tank01 team-id strings, not FKs.app/ingest/mappers.py— pure, no session, testable from a dict.app/ingest/games.py— the only step that touches the DB. ext-id → FK happens here.app/jobs/sync_games_for_current_week.py— policy. The job decides what isworth fetching.
Two things in that last block carry the whole design.
odds = client.odds if ... else {}must be hoisted out of the comprehension.Referencing
client.oddsinside the loop would trigger thecached_propertyandspend the ~3 calls even on a locked week — the exact "a flag can only skip
writes" trap already documented in
sync_scores_for_current_week.py. That oneline is the whole of #425.
And
line=Noneflows all the way toupsert_games, where it means do not touchthose columns. No fabricated 0.5 anywhere in the path, and nothing can overwrite
a locked line, because the value that would have overwritten it was never
constructed.
What moves where
Run this sort over
NflApiGameand nothing is left, which is why what remains isGameFacts: a container with no behavior.The test is does it need to know what Tank01 said?
→ Mapper (yes, it does):
_parse_spreadturning"+-2.5"into-2.5abs()+ round-half-up""→0for blank points on unstarted games"2"→GameStatus.FINALint(espnID)→ Stays a property, downstream (reads only normalized fields):
is_pregame,is_final,winning_team. These know nothing about Tank01 — theyread
statusand two ints. They are domain predicates, and they already exist onGame(game.py:84,:89,:137), duplicated withNflApiGametoday. Theduplication disappears on its own once
NflApiGamedoes.→ Persistence (needs the DB):
home_team,away_team,favored_team. Not computation — resolution.ext-id → FK, in
upsert_games, which has the session.→ Deleted, not moved:
The pick'em fallback — no odds →
spread = 0.5, home favored. That exists onlybecause "unknown" was not representable.
line: Line | Nonemakes itrepresentable, so the workaround evaporates rather than relocating.
Caching and fetch policy
Splitting mappers out does not cost the per-week caching, because caching is a
transport concern and stays in the client. The seam is already visible in
today's class:
games_data,scores_data,teams_data,odds_data,current_info—cached_propertyover an HTTP call, pinned bythe constructor.
games,live_scores,teams,find_teams.Two different kinds of "smart", and they belong in different places:
client's job. Keep it.
Policy. Depends on
odds_state, which is a pool concept the client has nobusiness knowing. The job's job.
Today
NflApi.gamesmakes the second decision by always touchingodds_data,which is the whole of #425.
Pulling mappers out does not multiply calls: two mappers needing the same
schedule hit the same memo on the same client instance. Cache the fetch, not
the translation.
One scope note:
cached_propertymeans the memo lives as long as the object,which is correct here because a client is already constructed per job run. Keep
doing that. The failure mode would be a module-level singleton, which turns a
per-run cache into a stale-forever cache.
Worked example: odds
Odds is the least 1:1 thing in the system. There is no odds table;
OddsStatein
week_state.pyis an enum. Odds resolve to exactly two columns onGame:So the shape of the work is:
~3 date-scoped calls → N games × M sportsbooks × a string like
"+-2.5"→ one float and one FK.
Parsing
"+-2.5"is provider handling; median-across-books and favoriteselection are pool rules. Both land in the mapper, for different reasons — the
first because only the mapper should know Tank01's string forms, the second
because no provider file should encode how the pool picks a favorite.
On DTO scope: model the fields we consume and let Pydantic ignore the rest. A
faithful mirror of every Tank01 field is not needed, because drift is already
covered by the
livecontract tests and the weeklyapi-driftworkflow. Strictvalidation on the fields we use, loose on everything else.
GameFacts is roughly NflApiGame today
Field-for-field it is close to a rename. Three differences, and they are the point:
NflApiGameholds a live client.self._data_sourceis stashed in__init__, sohome_team/away_team/favored_teamcan spend HTTP callson access.
GameFactsholds ext-id strings and resolves nothing. That is whatmakes it inert: construct it, and everything it will ever say is decided.
NflApiGamecannot say "I do not know." No odds →spread = 0.5.GameFacts.lineisLine | None.NflApiGamecomputes;GameFactscarries. Parsing, median, sign androunding happen in
__init__today. Under the split the mapper does that andhands over the result — a container, not a calculator. Same data, different
author:
NflApiGameis built by the client,GameFactsby a mapper outside it.The rest of the delta is small:
spread+_favored_team_idcollapse intoline,_data_sourcedisappears, the team ids become plain strings, andgame_datedrops out entirely — it never escapesapp/tank01_api/, existingonly as the key for the
/getNFLBettingOddsfetch loop, which makes it atransport detail.
NflApiScoreis already a proper tier-2 object — inert, no client handle, nosession. It is the template.
NflApiGameis the one that drifted.Approach
Build the pieces in place inside
app/tank01_api/tank01_api.pyfirst, thenmigrate them to their own modules once they exist and are covered. Growing the
new shape next to the old one keeps every step non-breaking; moving files is a
separate, mechanical step that can happen when the contents have settled.
How this relates to #425
#425 wants the
odds fetch split from the games fetch, because a locked week re-fetches the
schedule all week (flex scheduling) and pays ~3 extra
/getNFLBettingOddscallsevery time, on a metered plan.
It is awkward to fix today because the only way to say "skip odds" through
NflApiGame.__init__isodds_data=None, which silently resolves tospread=0.5/ home favored — and the ingest job then writes that over thelocked line.
Under the shape above, #425 stops being a flag or a second return type. "Schedule
only" is just a mapper with a narrower signature (
to_kickoff_facts), and thereis no odds parameter to pass
Noneto. The awkwardness is this design questionsurfacing as a bug.
Noticed along the way
Game.spreadandGame.favorite_team_idare bothOptionalwithdefault=None. But the comment intest_game_without_odds_defaults_to_home_favoritejustifies the 0.5 fallback by asserting both are non-nullable. That is stale:
"odds unknown" is already representable, so the fabricated pick'em is a
workaround for a constraint that no longer exists. Worth resolving on its own.
NflApi.gameshas exactly one production consumer,sync_games_for_current_week.py. Nothing else can be affected by how it is split.find_gamewas test-only surface and has been removed; tests now use a localgame_by_idhelper.