Convert the last five database gates by hand, and record why three stay (ER66 batch 6/6) - #372
Merged
Merged
Conversation
Five modules the codemod declined are converted: they had their own shapes, including one that had built its own once-per-session cache of the migration. Three keep their own gate on purpose - two because running the migration is what they test, one because it migrates a database it creates itself - and the exemption list now carries that reason for each. Also adds what the caching cost. Every module used to run its own migration, so a module that damaged the schema was silently repaired by the next one. Nobody designed that and nothing depended on it, but it is gone, and the symptom would be an unrelated failure much later. The session now checks that the revision is where the migration left it. The Redis guard's docstring records that it reads REDIS_URL at session start while the limiter binds at import, why nothing currently exploits that gap, and how to check whether that is still true.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The last of the six. Five modules converted by hand, three exempted with their
reasons, and two things the series turned up recorded where they will be found.
The five
The codemod declined all five, which is what it is for - each had its own shape.
test_component_approval_service.pyis the interesting one: it had already builta session-level
_ALEMBIC_RANflag so the migration ran once per session, whichis exactly what the shared helper now does. Converting it removed a duplicate
mechanism, not just a duplicate gate.
The three that stay, and why
An exemption means "this module may keep its own arrangement", and the list now
says why for each, because the reason is what tells the next person whether a
file can come off it.
Testing the tool itself - gating on the migration would delete the subject:
test_alembic_upgrade.pyasserts thatalembic upgrade headsucceeds and thatalembic currentthen reports head;test_scan_dependency_fingerprint_migration.pyasserts the shape revision 0070 leaves and that its
downgrade()fails loudly.unit/core/test_config_database_url.pywas already here for the same reason.Migrating a different database -
test_app_role_grant_matrix.pycreates athrowaway database and runs alembic against that, with
DATABASE_URLrepointedin a subprocess, to test role-first-then-migrate ordering. The helper migrates
the configured database, which is not the one under test.
Worth distinguishing from a third thing that is not an exemption: needing the
database in a particular state. A module can arrange that itself, and
test_bootstrap_from_empty.pydoes - it migrates to head through the helper andthen truncates. Its name suggests otherwise, which is why each of these was read
rather than classified by name; the file whose name mentions emptiness needs
head, and the file whose name says nothing about databases builds its own.
What the caching cost
Every module used to run its own
alembic upgrade head, so a module thatdamaged the schema was silently repaired by the next one's migration. Nobody
designed that, and nothing depended on it - but it is gone now, and the symptom
of losing it would be a failure in an unrelated module much later, with the cause
far behind. A comment would not reach the person who writes such a test, so the
session asserts that the revision is where the migration left it. One query, and
it cannot name the guilty test; it says only that the schema moved. Verified by
planting a test that moves the pointer and does not restore it: the run reports
the schema revision changed during this run: 0083 -> 0001.Both current mutators do restore:
test_health_ready.pyrewinds the pointer andputs it back in
finally, andtest_backup_task_round_trip.pyrestores from itsown dump.
The guard's blind spot
conftest.py's Redis check readsREDIS_URLat session start;core/ratelimit.pybinds its storage at import. Anything changing the variable in between would leave
them watching different indexes. Nothing does today, for two independent reasons -
all seven writes are function-scoped
monkeypatch.setenv, and every value names ahost that does not exist - and the docstring says how to re-check both, since
either reason could lapse on its own.
The series
193 modules, converted in five mechanical batches of 43, 33, 33, 38 and 38, plus
these five. Every batch matched its declared count and produced no conversion
shape outside the six recorded in batch 1. The declared-count check stopped a run
three times, and all three were arithmetic on my side rather than the codemod: a
file counted from the wrong directory, and twice
wc -lon a list with notrailing newline. It never once caught the conversion being wrong, which is worth
saying plainly - the weaker input in this work was the person counting.