-
Notifications
You must be signed in to change notification settings - Fork 9
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
Restore optional cache target #118
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #118 +/- ##
=======================================
Coverage 96.36% 96.36%
=======================================
Files 15 15
Lines 495 495
=======================================
Hits 477 477
Misses 18 18 ☔ View full report in Codecov by Sentry. |
LGTM @sbquinlan! A basic test would be great. I don't think we have a test_storage.py yet, so testing might require adding that module. |
Just a note to say IMO we can merge this as soon as a test is added. |
@cisaacstern yeah I'm just caught up in other things. I'll try come back to this pr this week. |
No rush from my end! Thanks! |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Dataflow tests are failing because the head branch is from a fork, so credentials are not available in Github Workflows. But that's okay, because the unit tests are convincing in themselves! Thanks @sbquinlan! |
Currently leaving the CacheTarget as it's default empty string throws:
This change fixes that by restoring the former behavior of ignoring the default config (of empty string).