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

Support versions of GCS without transfer_manager #410

Merged
merged 6 commits into from
Feb 26, 2024

Conversation

pjbull
Copy link
Member

@pjbull pjbull commented Feb 26, 2024

Add support for google-cloud-storage<2.7.0 per discussion in #408 and #409.

  • Manually confirmed the test suite passes with google-cloud-storage==2.6.0 (and fails without the fix)
  • Warns if user passes transfer_manager kwargs that can't be used
  • Adds changelog notes
  • Bumps version

Copy link
Contributor

github-actions bot commented Feb 26, 2024

@pjbull pjbull requested a review from jayqi February 26, 2024 21:45
@github-actions github-actions bot temporarily deployed to pull request February 26, 2024 21:46 Inactive
Copy link

codecov bot commented Feb 26, 2024

Codecov Report

Attention: Patch coverage is 62.50000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 93.9%. Comparing base (6d13512) to head (0f6c570).

Additional details and impacted files
@@                          Coverage Diff                          @@
##           hotfix/v0.18.1-google-cloud-storage    #410     +/-   ##
=====================================================================
- Coverage                                 93.9%   93.9%   -0.1%     
=====================================================================
  Files                                       23      23             
  Lines                                     1637    1643      +6     
=====================================================================
+ Hits                                      1538    1543      +5     
- Misses                                      99     100      +1     
Files Coverage Δ
cloudpathlib/gs/gsclient.py 93.1% <62.5%> (-1.9%) ⬇️

... and 1 file with indirect coverage changes

HISTORY.md Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to pull request February 26, 2024 22:03 Inactive
@jayqi
Copy link
Member

jayqi commented Feb 26, 2024

I guess now I'm wondering if yanking v0.18.0 is necessary. It's a little hard to tell how severe of an action yanking is, and most of the examples given are worse errors.

In the approach where we have a version floor, it's definitely necessary or v0.18.0 would be the newest version of cloudpathlib for google-cloud-storage<2.7.0. In this case, v0.18.1 would just be newer than v0.18.0 in all cases.

I guess since this can cause an error on import just as a side-effect of having both packages installed, it probably doesn't hurt to still yank.

HISTORY.md Outdated Show resolved Hide resolved
Co-authored-by: Jay Qi <2721979+jayqi@users.noreply.github.com>
@github-actions github-actions bot temporarily deployed to pull request February 26, 2024 22:08 Inactive
@pjbull pjbull merged commit 29a147a into hotfix/v0.18.1-google-cloud-storage Feb 26, 2024
28 checks passed
@pjbull pjbull deleted the support-older-gcs branch February 26, 2024 22:33
pjbull added a commit that referenced this pull request Feb 26, 2024
* Fix 408

* Version and changelog

* add warning

* update history PR

* Tweak description of change

* Update HISTORY.md

Co-authored-by: Jay Qi <2721979+jayqi@users.noreply.github.com>

---------

Co-authored-by: Jay Qi <2721979+jayqi@users.noreply.github.com>
jayqi added a commit that referenced this pull request Feb 27, 2024
* Support versions of GCS without transfer_manager (#410)

* Fix 408

* Version and changelog

* add warning

* update history PR

* Tweak description of change

* Update HISTORY.md

Co-authored-by: Jay Qi <2721979+jayqi@users.noreply.github.com>

---------

Co-authored-by: Jay Qi <2721979+jayqi@users.noreply.github.com>

* Update HISTORY.md

Co-authored-by: Jay Qi <2721979+jayqi@users.noreply.github.com>

---------

Co-authored-by: Jay Qi <2721979+jayqi@users.noreply.github.com>
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.

3 participants