-
Notifications
You must be signed in to change notification settings - Fork 55
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
fix: use native exists()
method in GSClient
#420
fix: use native exists()
method in GSClient
#420
Conversation
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.
@pjbull Confirmed, I have this working on an empty GCP bucket. |
Thanks @bachya, happy to merge this once the code quality and mocked tests are passing. |
Tests need an update in the mocked version (which mock the cloud SDKs):
More info on running the test suite here: https://cloudpathlib.drivendata.org/stable/contributing/#tests |
Thanks, @pjbull. I couldn't see the results until you approved the CI run; got them now, and I'll update the tests. |
@bachya just fyi, you can run |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #420 +/- ##
========================================
- Coverage 93.8% 93.4% -0.4%
========================================
Files 23 23
Lines 1644 1639 -5
========================================
- Hits 1543 1532 -11
- Misses 101 107 +6
|
Looks good! I'm going to merge into a repo-local branch so we can run against the live server backends, which is not possible on PRs from forks. |
SSIA
Closes #356
Contributor checklist:
CONTRIBUTING.md
Closes #issue
appears in the PR summary (e.g.,Closes #123
).HISTORY.md
with the issue that is addressed and the PR you are submitting. If the top section is not `## UNRELEASED``, then you need to add a new section to the top of the document for your change.