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

Patch nightly build flakiness #1856

Merged
merged 3 commits into from
Aug 23, 2022
Merged

Patch nightly build flakiness #1856

merged 3 commits into from
Aug 23, 2022

Conversation

bendnorman
Copy link
Member

This PR retries GCS Cache requests on BadRequest errors to eliminate the flakiness in our nightly builds.

@bendnorman bendnorman changed the base branch from main to dev August 23, 2022 01:01
@zaneselvans
Copy link
Member

The first attempt at running the tests failed with a Zenodo timeout, which seems weird to me. I thought we were trying to download directly from the GCS cache for the CI now?

@codecov
Copy link

codecov bot commented Aug 23, 2022

Codecov Report

Merging #1856 (b659881) into dev (a4284fc) will decrease coverage by 0.0%.
The diff coverage is 80.0%.

@@           Coverage Diff           @@
##             dev   #1856     +/-   ##
=======================================
- Coverage   83.0%   83.0%   -0.1%     
=======================================
  Files         65      65             
  Lines       7327    7335      +8     
=======================================
+ Hits        6088    6092      +4     
- Misses      1239    1243      +4     
Impacted Files Coverage Δ
src/pudl/workspace/resource_cache.py 84.2% <80.0%> (+1.1%) ⬆️
src/pudl/workspace/datastore.py 68.1% <0.0%> (-1.6%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@zaneselvans
Copy link
Member

zaneselvans commented Aug 23, 2022

Hmm, it looks like we added the --gcs-cache-path argument to our pytest configuration, but then didn't tell the CI to use it.

Is there a good reason not to just have the tests default to trying the GCS cache first all the time? If there's a local cache, that'll get priority. If the data isn't in the GCS cache, it'll fall back to pulling from Zenodo (and populate the GCS cache in the process?)

@zaneselvans
Copy link
Member

PR trying to use the GCS Cache in CI: #1858

@bendnorman
Copy link
Member Author

bendnorman commented Aug 23, 2022

Ohh I thought the GCS cache in pytest changes were buried in an EIA flakiness PR.

Is there a good reason not to just have the tests default to trying the GCS cache first all the time? If there's a local cache, that'll get priority. If the data isn't in the GCS cache, it'll fall back to pulling from Zenodo (and populate the GCS cache in the process?)

This is currently how the LayeredCache works. If the files aren't available in the local cache, then it falls back to the GCS cache if --gcs-cache-path is specified. Are you saying we should set it up so folks don't need to specify --gcs-cache-path?

@bendnorman bendnorman merged commit 2044871 into dev Aug 23, 2022
@bendnorman bendnorman deleted the patch-nightly-build-flakiness branch August 23, 2022 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants