-
-
Notifications
You must be signed in to change notification settings - Fork 110
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
Address loose ends in versioned release mechanics #3421
Conversation
5ce6a2e
to
7222459
Compare
pytest ${pytest_args} -n auto --live-dbs test/validate | ||
pytest ${pytest_args} -n 4 --no-cov --live-dbs test/validate |
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.
These tests aren't used for generating coverage, and coverage is turned on by default in the coverage configuration. Explicitly disable coverage here to avoid a "failure" after the test runs.
Also, switch to using at most 4 CPUs to avoid running out of memory with our currently very memory inefficient system for running the data validations.
@@ -71,7 +71,7 @@ function run_pudl_etl() { | |||
|
|||
function save_outputs_to_gcs() { | |||
echo "Copying outputs to GCP bucket $PUDL_GCS_OUTPUT" && \ | |||
gsutil -m cp -r "$PUDL_OUTPUT" "$PUDL_GCS_OUTPUT" && \ | |||
gsutil -q -m cp -r "$PUDL_OUTPUT" "$PUDL_GCS_OUTPUT" && \ |
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.
I added some -q
and --quiet
flags to the GCS and S3 commands since they produce verbose interactive terminal outputs that we don't want to log.
# If running a tagged release, ensure that outputs can't be accidentally deleted | ||
# It's not clear that an object lock can be applied in S3 with the AWS CLI | ||
if [[ "$GITHUB_ACTION_TRIGGER" == "push" && "$BUILD_REF" == v20* ]]; then | ||
gcloud storage objects update "gs://pudl.catalyst.coop/$BUILD_REF/*" --temporary-hold 2>&1 | tee -a "$LOGFILE" | ||
GCLOUD_TEMPORARY_HOLD_SUCCESS=${PIPESTATUS[0]} | ||
fi |
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.
This should prevent the accidental deletion of versioned outputs after they've been distributed, on GCS at least.
@@ -285,14 +285,14 @@ filterwarnings = [ | |||
"ignore:Subclassing validator classes is not intended to be part of their public API.:DeprecationWarning", | |||
"ignore:Subclassing validator classes:DeprecationWarning:tableschema", | |||
"ignore:The Shapely GEOS version:UserWarning:geopandas[.*]", | |||
"ignore:Unknown extension:UserWarning:openpyxl.worksheet[.*]", |
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.
Removed this since we aren't using the openpyxl
engine any more.
asset_check_from_schema(asset_key, _package) for asset_key in _asset_keys | ||
asset_check_from_schema(asset_key, _package) | ||
for asset_key in _asset_keys | ||
if asset_key.to_user_string() != "core_epacems__hourly_emissions" |
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.
Asset check was failing on core_epacems__hourly_emissions
since it's written out very differently than the other tables. It also has a billion rows, so we would need to check it in a different way if we wanted to check it.
"valid_till_date": { | ||
"valid_until_date": { |
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.
Use the full word rather than a nonstandard contraction.
path.open("wb").write(content) | ||
with path.open("wb") as file: | ||
file.write(content) |
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.
This was generating a warning about the file resource being left open, so I switched to a context manager.
Overview
Deal with some minor issues in the release mechanics that came up in the last release.
Closes #3375
Testing
How did you make sure this worked? How can a reviewer verify this?
To-do list