Drop deprecated lanes.size columns (PP-4506)#3563
Conversation
Release-2 of the online-migration sequence started in the parent PR. Now that a release marking size / size_by_entrypoint as deferred and moving size's default into the database (server_default) has shipped, no running release (current or N-1) reads or writes these columns, so they can be dropped safely. Removes the columns from the Lane model (and the now-unused deferred / text / JSON imports) and adds the drop migration. The downgrade recreates the columns in their post-release-1 state (size NOT NULL with server_default 0, size_by_entrypoint nullable JSON). Stacked on the release-1 PR; do not merge until that release has gone out.
|
Claude finished @jonathangreen's task in 2m 28s —— View job Review
SummaryThis is a clean, correct release-2 drop and the online-migration reasoning holds up. I traced the full sequence: release-1 ( |
Greptile SummaryThis is the second (drop) release of a two-step online migration that removes the deprecated
Confidence Score: 5/5Safe to merge once the release containing #3481 has been deployed; the migration and model change are self-consistent and complete. The two-release online-migration pattern is followed correctly: release 1 stopped all reads and writes on both columns, and this release drops them. The downgrade recreates both columns in exactly their post-release-1 state. No remaining references to the dropped columns exist anywhere in src/ or tests/. The model cleanup (removed imports, removed mappings) is consistent with the migration. No files require special attention. The migration and model change are straightforward and fully consistent with each other. Important Files Changed
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3563 +/- ##
=======================================
Coverage 93.46% 93.47%
=======================================
Files 512 512
Lines 46611 46609 -2
Branches 6352 6352
=======================================
- Hits 43567 43566 -1
+ Misses 1968 1967 -1
Partials 1076 1076 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Description
Release-2 (the drop) of the online-migration sequence for removing the deprecated lane-size cache columns. The release-1 PR (#3481) has merged: it marked
size/size_by_entrypointasdeferred()(stop reading) and movedsize's default into the database viaserver_default(stop writing, so the column is omitted fromINSERTs). This PR removes the columns from theLanemodel and drops them from the schema.I created this so we don't forget to drop these columns since I merged #3481 and @dbernstein is out on vacation.
This is safe only once a release containing #3481 has actually gone out. I'll leave this in draft until then.
Motivation and Context
JIRA: PP-4506
Completes the two-release removal started in #3481.
How Has This Been Tested?
tests/migration/test_alembic_builtin_tests.pypasses, includingtest_model_definitions_match_ddl(model matches the schema after the drop) and the up/down migration round-trip (downgrade recreates the columns, upgrade drops them again).mypyclean on the Lane model.size/size_by_entrypointonLaneanywhere insrc/ortests/.Checklist