Skip to content

Fix globbing top level buckets and#312

Merged
jayqi merged 3 commits into
masterfrom
311-glob-fixes
Jan 5, 2023
Merged

Fix globbing top level buckets and#312
jayqi merged 3 commits into
masterfrom
311-glob-fixes

Conversation

@pjbull
Copy link
Copy Markdown
Member

@pjbull pjbull commented Jan 5, 2023

#304 introduced a regression when globbing at the top level (CloudPath("s3://bucket").glob("*")) where the bucket was duplicated in the paths that glob yielded (CloudPath("s3://bucket/bucket/file1.txt") is retruned), which was reported in #311. Additionally, #311 mentions that CloudPath("s3://").glob("*") does not work as expected.

This PR adds two fixes:

  • Fix the path construction logic in glob to not duplicate the bucket at the top level.
  • Raise a not implemented error if globbing above a bucket level. Because of the way boto3 works, we would need to repeat our calls for each of the buckets to make globbing above a bucket work. This adds too much complexity. Instead, we give a friendly error that buckets can be listed with iterdir and .glob works within buckets.

Additionally we add a regression test and update setup.py and changelog to make a patch release to fix this bug.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 5, 2023

@pjbull pjbull requested a review from jayqi January 5, 2023 02:00
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jan 5, 2023

Codecov Report

Merging #312 (f780e22) into master (2556df0) will increase coverage by 0.0%.
The diff coverage is 100.0%.

Additional details and impacted files
@@          Coverage Diff           @@
##           master    #312   +/-   ##
======================================
  Coverage    94.1%   94.1%           
======================================
  Files          21      21           
  Lines        1393    1395    +2     
======================================
+ Hits         1312    1314    +2     
  Misses         81      81           
Impacted Files Coverage Δ
cloudpathlib/cloudpath.py 92.5% <100.0%> (+<0.1%) ⬆️

@jayqi jayqi merged commit caeeb50 into master Jan 5, 2023
@jayqi jayqi deleted the 311-glob-fixes branch January 5, 2023 04:50
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