-
-
Notifications
You must be signed in to change notification settings - Fork 118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make asset checks run in integration test #3990
Conversation
70ceddc
to
11ccfbe
Compare
11ccfbe
to
ab560e3
Compare
…set-check-fix Fix FERC714 fast CI asset check
…allow test to specify job name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay! Thanks for fixing this.
return Definitions( | ||
assets=pudl.etl.default_assets, | ||
resources=pudl.etl.default_resources, | ||
jobs=jobs, | ||
).get_job_def("etl_job") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see... we redefined the definitions here but didn't add the asset checks so they weren't getting run. Importing defs
from pudl.etl
includes the assets checks.
Overview
Closes #3928 .
What problem does this address?
#3711 inadvertently disabled the asset checks again, in service of fixing
pudl_etl
😞Getting rid of
pudl_etl
(#3161) seemed out of scope here, but I made the tests use the sameDefinitions
as the nightly builds again.Testing
How did you make sure this worked? How can a reviewer verify this?
Made the pandera checks all fail, ran
make integration-test
, and checked to make sure that the integration tests bailed out once we ran into a failure.Then got other people to fix the failing asset checks and re-ran integration tests. They passed.
To-do list