Repository: drivendataorg/cloudpathlib
Pull request #312 (2023-01-05 01:56:30)

Fix globbing top level buckets and
#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[bot]: <!-- NETLIFY DEPLOY COMMENT GENERATED BY ACTIONS_NETLIFY -->
🚀 Deployed on https://deploy-preview-312--gallant-agnesi-5f7bb3.netlify.app
codecov-commenter: # [Codecov](https://codecov.io/gh/drivendataorg/cloudpathlib/pull/312?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=drivendataorg) Report
> Merging [#312](https://codecov.io/gh/drivendataorg/cloudpathlib/pull/312?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=drivendataorg) (f780e22) into [master](https://codecov.io/gh/drivendataorg/cloudpathlib/commit/2556df0d5d6b928ce313e1c27f6f68876e470563?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=drivendataorg) (2556df0) will **decrease** coverage by `0.4%`.
> The diff coverage is `100.0%`.

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff            @@
##           master    #312     +/-   ##
========================================
- Coverage    94.1%   93.7%   -0.5%     
========================================
  Files          21      21             
  Lines        1393    1392      -1     
========================================
- Hits         1312    1305      -7     
- Misses         81      87      +6     
```


| [Impacted Files](https://codecov.io/gh/drivendataorg/cloudpathlib/pull/312?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=drivendataorg) | Coverage Δ | |
|---|---|---|
| [cloudpathlib/cloudpath.py](https://codecov.io/gh/drivendataorg/cloudpathlib/pull/312?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=drivendataorg#diff-Y2xvdWRwYXRobGliL2Nsb3VkcGF0aC5weQ==) | `92.5% <100.0%> (+<0.1%)` | :arrow_up: |
| [cloudpathlib/s3/s3client.py](https://codecov.io/gh/drivendataorg/cloudpathlib/pull/312?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=drivendataorg#diff-Y2xvdWRwYXRobGliL3MzL3MzY2xpZW50LnB5) | `90.1% <0.0%> (-3.1%)` | :arrow_down: |
| [cloudpathlib/gs/gsclient.py](https://codecov.io/gh/drivendataorg/cloudpathlib/pull/312?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=drivendataorg#diff-Y2xvdWRwYXRobGliL2dzL2dzY2xpZW50LnB5) | `92.7% <0.0%> (-1.7%)` | :arrow_down: |
| [cloudpathlib/local/implementations/azure.py](https://codecov.io/gh/drivendataorg/cloudpathlib/pull/312?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=drivendataorg#diff-Y2xvdWRwYXRobGliL2xvY2FsL2ltcGxlbWVudGF0aW9ucy9henVyZS5weQ==) | `97.5% <0.0%> (-0.1%)` | :arrow_down: |
| [cloudpathlib/local/implementations/gs.py](https://codecov.io/gh/drivendataorg/cloudpathlib/pull/312?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=drivendataorg#diff-Y2xvdWRwYXRobGliL2xvY2FsL2ltcGxlbWVudGF0aW9ucy9ncy5weQ==) | `100.0% <0.0%> (ø)` | |
| [cloudpathlib/local/implementations/s3.py](https://codecov.io/gh/drivendataorg/cloudpathlib/pull/312?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=drivendataorg#diff-Y2xvdWRwYXRobGliL2xvY2FsL2ltcGxlbWVudGF0aW9ucy9zMy5weQ==) | `100.0% <0.0%> (ø)` | |

</details>

Patch:
diff --git a/HISTORY.md b/HISTORY.md
index fdfad99..894234e 100644
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -1,5 +1,9 @@
 # cloudpathlib Changelog
 
+## v0.12.1 (2023-01-04)
+
+ - Fix glob logic for buckets; add regression test; add error on globbing all buckets ([Issue #311](https://github.com/drivendataorg/cloudpathlib/issues/311), [PR #312](https://github.com/drivendataorg/cloudpathlib/pull/312))
+
 ## v0.12.0 (2022-12-30)
 
  - API Change: `S3Client` supports an `extra_args` kwarg now to pass extra args down to `boto3` functions; this enables Requester Pays bucket access and bucket encryption. (Issues [#254](https://github.com/drivendataorg/cloudpathlib/issues/254), [#180](https://github.com/drivendataorg/cloudpathlib/issues/180); [PR #307](https://github.com/drivendataorg/cloudpathlib/pull/307))
diff --git a/cloudpathlib/cloudpath.py b/cloudpathlib/cloudpath.py
index 74cbf4f..a4e91db 100644
--- a/cloudpathlib/cloudpath.py
+++ b/cloudpathlib/cloudpath.py
@@ -357,6 +357,11 @@ class CloudPath(metaclass=CloudPathMeta):
         if pattern.startswith(self.cloud_prefix) or pattern.startswith("/"):
             raise CloudPathNotImplementedError("Non-relative patterns are unsupported")
 
+        if self.drive == "":
+            raise CloudPathNotImplementedError(
+                ".glob is only supported within a bucket or container; you can use `.iterdir` to list buckets; for example, CloudPath('s3://').iterdir()"
+            )
+
     def _glob(
         self: DerivedCloudPath, selector, recursive: bool
     ) -> Generator[DerivedCloudPath, None, None]:
@@ -390,12 +395,13 @@ class CloudPath(metaclass=CloudPathMeta):
 
         root = _CloudPathSelectable(
             self.name,
-            [p.name for p in self.parents[:-1]],  # all parents except bucket/container
+            [],  # nothing above self will be returned, so initial parents is empty
             file_tree,
         )
 
         for p in selector.select_from(root):
-            yield self.client.CloudPath(f"{self.cloud_prefix}{self.drive}/{p}")
+            # select_from returns self.name/... so strip before joining
+            yield (self / str(p)[len(self.name) + 1 :])
 
     def glob(self: DerivedCloudPath, pattern: str) -> Generator[DerivedCloudPath, None, None]:
         self._glob_checks(pattern)
diff --git a/setup.py b/setup.py
index 1857150..6b1b804 100644
--- a/setup.py
+++ b/setup.py
@@ -61,5 +61,5 @@ setup(
         "Source Code": "https://github.com/drivendataorg/cloudpathlib",
     },
     url="https://github.com/drivendataorg/cloudpathlib",
-    version="0.12.0",
+    version="0.12.1",
 )


Test patch:
diff --git a/tests/test_cloudpath_file_io.py b/tests/test_cloudpath_file_io.py
index 3592d49..1ba8fd2 100644
--- a/tests/test_cloudpath_file_io.py
+++ b/tests/test_cloudpath_file_io.py
@@ -191,6 +191,25 @@ def test_glob(glob_test_dirs):
     )
 
 
+def test_glob_buckets(rig):
+    # CloudPath("s3://").glob("*") results in error
+    drive_level = rig.path_class(rig.path_class.cloud_prefix)
+
+    with pytest.raises(CloudPathNotImplementedError):
+        list(drive_level.glob("*"))
+
+    # CloudPath("s3://bucket").glob("*") should work
+    # bucket level glob returns correct results
+    # regression test for #311
+    bucket = rig.path_class(f"{rig.path_class.cloud_prefix}{rig.drive}")
+
+    first_result = next(bucket.glob("*"))
+
+    # assert all parts are unique
+    assert first_result.drive == rig.drive
+    assert len(first_result.parts) == len(set(first_result.parts))
+
+
 def test_glob_many_open_files(rig):
     # test_glob_many_open_files
     #  Adapted from: https://github.com/python/cpython/blob/7ffe7ba30fc051014977c6f393c51e57e71a6648/Lib/test/test_pathlib.py#L1697-L1712