Skip to content
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 the reconstructable job not take any arguments at all. #3996

Merged
merged 1 commit into from
Dec 27, 2024

Conversation

jdangerx
Copy link
Member

@jdangerx jdangerx commented Dec 27, 2024

Overview

What problem does this address?

... the nightly builds are still broken because the reconstructable job doesn't want any parameters at all.

What did you change?

Moved the parameters to the wrapper function.

Documentation

Make sure to update relevant aspects of the documentation.

Testing

How did you make sure this worked? How can a reviewer verify this?

  • Run make pytest-integration
  • Run pudl_etl --gcs-cache-path gs://internal-zenodo-cache.catalyst.coop ~/work/pudl/src/pudl/package_data/settings/etl_full.yml - see if it even starts

@e-belfer e-belfer self-requested a review December 27, 2024 19:01
@@ -365,6 +365,8 @@ def check_rows(context: AssetCheckExecutionContext) -> AssetCheckResult:
logger.info("Check VCE RARE hourly table is the expected length")

# Define row counts for fast/full etl
# TODO 2024-12-27: make this check row counts per year instead of having
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, but we'll probably eventually wind up with distribution tests that do want fast vs. full specified and can't be parameterized by year, so the solution will probably come in handy anyways?

Copy link
Member

@e-belfer e-belfer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming this runs as expected, this seems to be a reasonable reconfiguration to me. Left a note about the parameterization by year (tldr - I agree but we might need this kind of implementation for other tests).

@jdangerx jdangerx marked this pull request as ready for review December 27, 2024 21:53
@jdangerx jdangerx added this pull request to the merge queue Dec 27, 2024
Merged via the queue into main with commit 19efa7e Dec 27, 2024
17 checks passed
@jdangerx jdangerx deleted the fix-reconstructable-issue branch December 27, 2024 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants