johnsturgeon/tgfp-web

Watch

Fork 0

Rename tgfp_nfl id columns to provider-neutral external ids + add data_source #368

New issue

Closed

opened 2026-08-13 13:15:05 +02:00 by johnsturgeon · 0 comments

johnsturgeon commented

2026-08-13 13:15:05 +02:00

Owner

Phase 2 of the TGFPNfl → ESPNNfl vestige cleanup. Phase 1 (dead code, internal
renames, docs) landed separately; this is the schema half, deliberately split
because it carries a migration.

Prerequisite for the ESPN → TheSportsDB data-source swap.

Context

Two columns still carry the name of a library that no longer exists:

Column	Type	Current constraint
`game.tgfp_nfl_game_id`	`str`	`ix_game_tgfp_nfl_game_id` — UNIQUE btree
`team.tgfp_nfl_team_id`	`str`	`ix_team_tgfp_nfl_team_id` — UNIQUE btree

Both are described in the models as "External TGFP/NFL game id", but they
hold ESPN ids and will hold TheSportsDB ids after the swap.

Decisions already settled — do not reopen

No internal references. Neither column is a foreign key or join key.
All internal relationships go through integer PKs (Game.home_team_id → team.id, PlayerGamePick.game_id/player_id/picked_team_id). Renaming is
safe for referential integrity.
No historical-data blocker. Team records are derivable from game
(each row carries both teams, both scores, and the season); Team.wins/ losses/ties is a display cache for the current record only.
update_game only ever touches in-flight games and never revisits history.
A clean cutover is therefore sufficient — no backfill of historical
provider data is required.
data_source will be added even though there is no immediate consumer.
It costs one small column now and preserves a programmatic route to
re-fetch provider data later; without it, post-swap the column holds two id
namespaces with no way to tell them apart.

Multi-season correctness of player records is a separate, unrelated issue.

Scope

1. Rename the columns

game.tgfp_nfl_game_id → game.external_game_id
team.tgfp_nfl_team_id → team.external_team_id

Update the description= text on both Field(...) definitions, which
currently says "External TGFP/NFL …".

2. Add `data_source`

To both game and team. Follow the existing enum idiom in
app/models/award.py:7:

class DataSource(str, Enum):
    ESPN = "espn"
    THESPORTSDB = "thesportsdb"

Backfill every existing row to espn. Add with a server_default, then drop
the default in the same migration — precedent:
alembic/versions/1bba3f7edaac_remove_server_defaults_from_new_columns.py.

3. Move uniqueness to a composite

Single-column uniqueness becomes wrong the moment two providers coexist: the
same id string could legitimately appear once per provider.

Replace the unique index with a composite UniqueConstraint, leading with
data_source so equality lookups on both columns are served:

game: (data_source, external_game_id)
team: (data_source, external_team_id)

In-repo precedent for __table_args__ with a named UniqueConstraint:
app/models/player_award.py:16–25.

⚠️ Migration hazards

Alembic autogenerate does not detect column renames. It will emit a
drop_column + add_column pair, which silently destroys every external id
in both tables. The generated migration must be hand-edited to:

op.alter_column("game", "tgfp_nfl_game_id", new_column_name="external_game_id")
op.alter_column("team", "tgfp_nfl_team_id", new_column_name="external_team_id")

Review the generated file before applying it, and confirm the downgrade()
path reverses the rename rather than dropping columns.

The unique indexes must also be renamed or dropped/recreated —
ix_game_tgfp_nfl_game_id and ix_team_tgfp_nfl_team_id — since both names
embed the old column name. Verify with \d game and \d team after applying
that the composite constraint exists and no orphaned single-column unique
index remains (which would reject legitimate cross-provider duplicates).

Do not edit existing migrations. 9ff5e4af6bbc_01_initial_schema.py and
1885e311300d_02_added_field_uniqueness.py reference the old names as applied
history. A repo-wide find-and-replace would corrupt the revision chain. This
change is a new revision:

scripts/alembic_generate_migration.sh "rename tgfp_nfl ids to external ids and add data_source"

Code sites to update (6)

Phase 1 already removed a seventh, in app/espn_nfl/espn_nfl.py. The data
source module no longer references the schema at all — keep it that way.

File	Line(s)	Usage
`app/models/game.py`	32	Field definition
`app/models/team.py`	18	Field definition
`app/jobs/create_picks.py`	31, 34, 38	Inbound — maps provider team id → `Team` row
`app/jobs/create_picks.py`	53	Stores provider game id on `Game` creation
`app/jobs/sync_team_records.py`	12	Outbound — queries provider by stored id
`app/jobs/update_game.py`	31	Outbound — queries provider by stored id

The three lookups in create_picks.py use .one(), so they raise
NoResultFound on a miss. They should also filter on data_source once it
exists, otherwise a stale ESPN row could satisfy a TheSportsDB lookup.

Open decision: how `team` holds its mapping

game is unambiguous — each row is historical and immutable, so 2025 rows
keep data_source='espn' forever while new rows get 'thesportsdb'. The
column approach is exactly right there.

team is different: one row per franchise, so a data_source column on
team means one mapping at a time.

Option 1 — data_source column on team (simpler). At cutover,
overwrite all 32 rows with TheSportsDB ids. The ESPN mapping is lost, but
it is re-derivable (32 stable franchises, matchable by name). Keeps both
tables symmetrical.
Option 2 — separate team_external_id table (team_id, data_source,
external_id, unique on (data_source, external_id)). Both providers
coexist, so TheSportsDB ids can be populated and verified while ESPN is
still live, making cutover a flag flip and rollback trivial.

Recommendation: Option 1, on the grounds already established — team data
is a display cache, the franchise set is small and stable, and a re-map script
is cheap. Choose Option 2 only if a dual-run verification period before
cutover is wanted.

Note this decision only affects team; game uses the column either way.

Acceptance criteria

Columns renamed via a new Alembic migration using alter_column, with
no data loss — row counts and id values identical before/after
data_source present on game and team, all existing rows espn,
no lingering server default
Composite unique constraints in place; old single-column unique indexes
gone
downgrade() verified to reverse cleanly
All 6 code sites updated; create_picks.py lookups filter on
data_source
Team mapping decision recorded above
pylint reports 10.00/10
App boots; create_picks, sync_team_records and update_game still
resolve teams and games against the live provider

Verification

Before and after the migration:

SELECT count(*) FROM game;
SELECT count(*) FROM team;
SELECT count(DISTINCT external_game_id) FROM game;   -- must equal game count
SELECT data_source, count(*) FROM team GROUP BY 1;   -- 32 rows, all 'espn'

Then confirm no vestiges remain outside applied migration history:

grep -rniE "tgfp[_-]?nfl" . \
  --exclude-dir=.venv --exclude-dir=.git --exclude-dir=node_modules \
  --exclude-dir=__pycache__ --exclude-dir=.pytest_cache --exclude=tgfp.dump

Expected result: only alembic/versions/9ff5e4af6bbc_01_initial_schema.py
and alembic/versions/1885e311300d_02_added_field_uniqueness.py.

Repo has no test suite, so verification is lint plus a manual boot and an
exercise of the three affected jobs.

Phase 2 of the TGFPNfl → ESPNNfl vestige cleanup. Phase 1 (dead code, internal renames, docs) landed separately; this is the schema half, deliberately split because it carries a migration. Prerequisite for the ESPN → TheSportsDB data-source swap. ## Context Two columns still carry the name of a library that no longer exists: | Column | Type | Current constraint | |---|---|---| | `game.tgfp_nfl_game_id` | `str` | `ix_game_tgfp_nfl_game_id` — UNIQUE btree | | `team.tgfp_nfl_team_id` | `str` | `ix_team_tgfp_nfl_team_id` — UNIQUE btree | Both are described in the models as `"External TGFP/NFL game id"`, but they hold **ESPN** ids and will hold **TheSportsDB** ids after the swap. ### Decisions already settled — do not reopen - **No internal references.** Neither column is a foreign key or join key. All internal relationships go through integer PKs (`Game.home_team_id → team.id`, `PlayerGamePick.game_id/player_id/picked_team_id`). Renaming is safe for referential integrity. - **No historical-data blocker.** Team records are derivable from `game` (each row carries both teams, both scores, and the season); `Team.wins/ losses/ties` is a display cache for the *current* record only. `update_game` only ever touches in-flight games and never revisits history. A clean cutover is therefore sufficient — no backfill of historical provider data is required. - **`data_source` will be added** even though there is no immediate consumer. It costs one small column now and preserves a programmatic route to re-fetch provider data later; without it, post-swap the column holds two id namespaces with no way to tell them apart. Multi-season correctness of *player* records is a separate, unrelated issue. --- ## Scope ### 1. Rename the columns - `game.tgfp_nfl_game_id` → `game.external_game_id` - `team.tgfp_nfl_team_id` → `team.external_team_id` Update the `description=` text on both `Field(...)` definitions, which currently says "External TGFP/NFL …". ### 2. Add `data_source` To **both** `game` and `team`. Follow the existing enum idiom in `app/models/award.py:7`: ```python class DataSource(str, Enum): ESPN = "espn" THESPORTSDB = "thesportsdb" ``` Backfill every existing row to `espn`. Add with a `server_default`, then drop the default in the same migration — precedent: `alembic/versions/1bba3f7edaac_remove_server_defaults_from_new_columns.py`. ### 3. Move uniqueness to a composite Single-column uniqueness becomes wrong the moment two providers coexist: the same id string could legitimately appear once per provider. Replace the unique index with a composite `UniqueConstraint`, leading with `data_source` so equality lookups on both columns are served: - `game`: `(data_source, external_game_id)` - `team`: `(data_source, external_team_id)` In-repo precedent for `__table_args__` with a named `UniqueConstraint`: `app/models/player_award.py:16–25`. --- ## ⚠️ Migration hazards **Alembic autogenerate does not detect column renames.** It will emit a `drop_column` + `add_column` pair, which silently destroys every external id in both tables. The generated migration **must** be hand-edited to: ```python op.alter_column("game", "tgfp_nfl_game_id", new_column_name="external_game_id") op.alter_column("team", "tgfp_nfl_team_id", new_column_name="external_team_id") ``` Review the generated file before applying it, and confirm the `downgrade()` path reverses the rename rather than dropping columns. The unique indexes must also be renamed or dropped/recreated — `ix_game_tgfp_nfl_game_id` and `ix_team_tgfp_nfl_team_id` — since both names embed the old column name. Verify with `\d game` and `\d team` after applying that the composite constraint exists and no orphaned single-column unique index remains (which would reject legitimate cross-provider duplicates). **Do not edit existing migrations.** `9ff5e4af6bbc_01_initial_schema.py` and `1885e311300d_02_added_field_uniqueness.py` reference the old names as applied history. A repo-wide find-and-replace would corrupt the revision chain. This change is a *new* revision: ```bash scripts/alembic_generate_migration.sh "rename tgfp_nfl ids to external ids and add data_source" ``` --- ## Code sites to update (6) Phase 1 already removed a seventh, in `app/espn_nfl/espn_nfl.py`. The data source module no longer references the schema at all — keep it that way. | File | Line(s) | Usage | |---|---|---| | `app/models/game.py` | 32 | Field definition | | `app/models/team.py` | 18 | Field definition | | `app/jobs/create_picks.py` | 31, 34, 38 | **Inbound** — maps provider team id → `Team` row | | `app/jobs/create_picks.py` | 53 | Stores provider game id on `Game` creation | | `app/jobs/sync_team_records.py` | 12 | **Outbound** — queries provider by stored id | | `app/jobs/update_game.py` | 31 | **Outbound** — queries provider by stored id | The three lookups in `create_picks.py` use `.one()`, so they raise `NoResultFound` on a miss. They should also filter on `data_source` once it exists, otherwise a stale ESPN row could satisfy a TheSportsDB lookup. --- ## Open decision: how `team` holds its mapping `game` is unambiguous — each row is historical and immutable, so 2025 rows keep `data_source='espn'` forever while new rows get `'thesportsdb'`. The column approach is exactly right there. `team` is different: **one row per franchise**, so a `data_source` column on `team` means one mapping at a time. - **Option 1 — `data_source` column on `team` (simpler).** At cutover, overwrite all 32 rows with TheSportsDB ids. The ESPN mapping is lost, but it is re-derivable (32 stable franchises, matchable by name). Keeps both tables symmetrical. - **Option 2 — separate `team_external_id` table** (`team_id`, `data_source`, `external_id`, unique on `(data_source, external_id)`). Both providers coexist, so TheSportsDB ids can be populated and verified *while ESPN is still live*, making cutover a flag flip and rollback trivial. **Recommendation: Option 1**, on the grounds already established — team data is a display cache, the franchise set is small and stable, and a re-map script is cheap. Choose Option 2 only if a dual-run verification period before cutover is wanted. Note this decision only affects `team`; `game` uses the column either way. --- ## Acceptance criteria - [ ] Columns renamed via a new Alembic migration using `alter_column`, with **no data loss** — row counts and id values identical before/after - [ ] `data_source` present on `game` and `team`, all existing rows `espn`, no lingering server default - [ ] Composite unique constraints in place; old single-column unique indexes gone - [ ] `downgrade()` verified to reverse cleanly - [ ] All 6 code sites updated; `create_picks.py` lookups filter on `data_source` - [ ] Team mapping decision recorded above - [ ] `pylint` reports 10.00/10 - [ ] App boots; `create_picks`, `sync_team_records` and `update_game` still resolve teams and games against the live provider ## Verification Before and after the migration: ```sql SELECT count(*) FROM game; SELECT count(*) FROM team; SELECT count(DISTINCT external_game_id) FROM game; -- must equal game count SELECT data_source, count(*) FROM team GROUP BY 1; -- 32 rows, all 'espn' ``` Then confirm no vestiges remain outside applied migration history: ```bash grep -rniE "tgfp[_-]?nfl" . \ --exclude-dir=.venv --exclude-dir=.git --exclude-dir=node_modules \ --exclude-dir=__pycache__ --exclude-dir=.pytest_cache --exclude=tgfp.dump ``` Expected result: **only** `alembic/versions/9ff5e4af6bbc_01_initial_schema.py` and `alembic/versions/1885e311300d_02_added_field_uniqueness.py`. Repo has no test suite, so verification is lint plus a manual boot and an exercise of the three affected jobs.

johnsturgeon referenced this issue from a commit

2026-08-15 16:10:48 +02:00

Rename tgfp_nfl id columns to external ids, add data_source (#368)

johnsturgeon referenced this issue from a pull request that will close it,

2026-08-15 16:11:06 +02:00

Rename tgfp_nfl id columns to provider-neutral external ids + add data_source #385

johnsturgeon closed this issue

2026-08-15 16:14:34 +02:00

johnsturgeon referenced this issue

2026-09-06 19:50:04 +02:00

Migration a390332fa722 crashes: drops non-existent team.data_source column #480