Test that DB schema matches the Alembic migrations #3027
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #3027 +/- ##
=====================================
Coverage 88.7% 88.7%
=====================================
Files 90 90
Lines 10988 10988
=====================================
Hits 9752 9752
Misses 1236 1236 ☔ View full report in Codecov by Sentry. |
| def pudl_sqlite_io_manager_fixture(tmp_path, test_pkg): | ||
| """Create a SQLiteIOManager fixture with a PUDL database schema.""" | ||
| db_path = tmp_path / "pudl.sqlite" | ||
| def fake_pudl_sqlite_io_manager_fixture(tmp_path, test_pkg, monkeypatch): |
There was a problem hiding this comment.
does this guy actually need monkeypatch?
| def test_error_when_handling_view_without_metadata(pudl_sqlite_io_manager_fixture): | ||
| def test_migrations_match_metadata(tmp_path, monkeypatch): | ||
| pkg = Package.from_resource_ids() | ||
| monkeypatch.chdir(Path(__file__).parent.parent.parent) |
There was a problem hiding this comment.
why do you need to do this chdir?
| monkeypatch.setenv("PUDL_OUTPUT", tmp_path) | ||
| alembic.config.main(["upgrade", "head"]) | ||
|
|
||
| PudlSQLiteIOManager(base_dir=tmp_path, db_name="pudl", package=pkg) |
There was a problem hiding this comment.
so this guy is running compare_metadata which is what is actually doing the checking?? why wasn't this doing it before in the pudl_sqlite_io_manager_fixture??
There was a problem hiding this comment.
or is it alembic.config.main(["upgrade", "head"]) that is doing the checking? or the combo or the upgrade/hear w/ generating the io manager?
|
I added comments etc. to hopefully address your questions in a more durable way - lmk what you think! |
jdangerx
force-pushed
the
test-db-schema
branch
from
99e03ad
to
2aaafc4
Compare
If the DB schema as defined in
metadata.classes.Package.from_resource_ids()doesn't match the schema you get from runningalembic upgrade head, ANDPudlPaths().pudl_dbis a pre-existing file, thepudl_sqlite_io_managerthrows an error at startup.HOWEVER!
pudl_sqlite_io_manageris initialized,PudlPaths().pudl_dbdoes not actually point to an existing file, so we usemetadata.create_all()- which masks any differences between the migrations and the Packagepudl_sqlite_io_managerisn't initialized until we actually want to write to the DB, which is pretty late in the DAG execution. This is a super avoidable nightly build failure.So, to catch any divergence between the alembic migrations and the package metadata, I added a unit test that runs the migrations on a test db and tries to initialize the io manager.