From 7cc1ddbcb57660207fcc2f3acdf1026d132fe674 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Mart=C3=ADnez=20Gonz=C3=A1lez?= Date: Thu, 4 Aug 2022 18:55:16 +0200 Subject: [PATCH 1/4] make prefix not being recognised as dirs anymore --- cloudpathlib/s3/s3client.py | 6 +++--- tests/test_s3_specific.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/cloudpathlib/s3/s3client.py b/cloudpathlib/s3/s3client.py index 20666fbc..62d92500 100644 --- a/cloudpathlib/s3/s3client.py +++ b/cloudpathlib/s3/s3client.py @@ -156,13 +156,13 @@ def _s3_file_query(self, cloud_path: S3Path): # else, confirm it is a dir by filtering to the first item under the prefix except ClientError: + key = cloud_path.key.rstrip("/") + "/" + return next( ( obj for obj in ( - self.s3.Bucket(cloud_path.bucket) - .objects.filter(Prefix=cloud_path.key) - .limit(1) + self.s3.Bucket(cloud_path.bucket).objects.filter(Prefix=key).limit(1) ) ), None, diff --git a/tests/test_s3_specific.py b/tests/test_s3_specific.py index 2ce5b269..d04ea304 100644 --- a/tests/test_s3_specific.py +++ b/tests/test_s3_specific.py @@ -164,6 +164,36 @@ def test_fake_directories(s3_like_rig): assert not test_case.is_file() +def test_directories(s3_like_rig): + """Fixing bug that marks as directories prefixes of existing paths + + Ref: https://github.com/drivendataorg/cloudpathlib/issues/208 + """ + boto3_s3_client = s3_like_rig.client_class._default_client.client + if not s3_like_rig.live_server: + pytest.skip("This test only runs against live servers.") + + response = boto3_s3_client.put_object( + Bucket=f"{s3_like_rig.drive}", + Body="123", + Key=f"{s3_like_rig.test_dir}/superfile", + ) + + assert response["ResponseMetadata"]["HTTPStatusCode"] == 200 + + super_path = s3_like_rig.create_cloud_path("superfile") + assert super_path.exists() + assert not super_path.is_dir() + + super_path = s3_like_rig.create_cloud_path("super2") + assert not super_path.exists() + assert not super_path.is_dir() + + super_path = s3_like_rig.create_cloud_path("super") + assert not super_path.exists() + assert not super_path.is_dir() + + def test_no_sign_request(s3_rig, tmp_path): """Tests that we can pass no_sign_request to the S3Client and we will be able to access public resources but not private ones. From c1adf921dca64400b0f307089aca839a4dcc69b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Mart=C3=ADnez=20Gonz=C3=A1lez?= Date: Mon, 8 Aug 2022 10:48:25 +0200 Subject: [PATCH 2/4] update tests --- tests/conftest.py | 10 ++++++++++ tests/test_s3_specific.py | 24 +++++++----------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 4e84e0e1..93ebcf20 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -402,3 +402,13 @@ def local_s3_rig(request, monkeypatch, assets_dir): custom_s3_rig, ], ) + +# use all available s3 implementations (include mocked local one) +s3_like_rig_all = fixture_union( + "s3_like_rig_all", + [ + s3_rig, + custom_s3_rig, + local_s3_rig, + ], +) diff --git a/tests/test_s3_specific.py b/tests/test_s3_specific.py index d04ea304..b22e6c9b 100644 --- a/tests/test_s3_specific.py +++ b/tests/test_s3_specific.py @@ -164,32 +164,22 @@ def test_fake_directories(s3_like_rig): assert not test_case.is_file() -def test_directories(s3_like_rig): +def test_directories(s3_like_rig_all): """Fixing bug that marks as directories prefixes of existing paths Ref: https://github.com/drivendataorg/cloudpathlib/issues/208 """ - boto3_s3_client = s3_like_rig.client_class._default_client.client - if not s3_like_rig.live_server: - pytest.skip("This test only runs against live servers.") - - response = boto3_s3_client.put_object( - Bucket=f"{s3_like_rig.drive}", - Body="123", - Key=f"{s3_like_rig.test_dir}/superfile", - ) - - assert response["ResponseMetadata"]["HTTPStatusCode"] == 200 + s3_like_rig = s3_like_rig_all - super_path = s3_like_rig.create_cloud_path("superfile") + super_path = s3_like_rig.create_cloud_path("dir_0/file0_0.txt") assert super_path.exists() assert not super_path.is_dir() - super_path = s3_like_rig.create_cloud_path("super2") - assert not super_path.exists() - assert not super_path.is_dir() + super_path = s3_like_rig.create_cloud_path("dir_0") + assert super_path.exists() + assert super_path.is_dir() - super_path = s3_like_rig.create_cloud_path("super") + super_path = s3_like_rig.create_cloud_path("dir_0/file0") assert not super_path.exists() assert not super_path.is_dir() From 55487f08a3aeed9a99020cdd6c22a03891ce9248 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Mart=C3=ADnez=20Gonz=C3=A1lez?= Date: Mon, 8 Aug 2022 10:51:59 +0200 Subject: [PATCH 3/4] remove unnecessary fixture --- tests/conftest.py | 10 ---------- tests/test_s3_specific.py | 4 +--- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 93ebcf20..4e84e0e1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -402,13 +402,3 @@ def local_s3_rig(request, monkeypatch, assets_dir): custom_s3_rig, ], ) - -# use all available s3 implementations (include mocked local one) -s3_like_rig_all = fixture_union( - "s3_like_rig_all", - [ - s3_rig, - custom_s3_rig, - local_s3_rig, - ], -) diff --git a/tests/test_s3_specific.py b/tests/test_s3_specific.py index b22e6c9b..4c9ac851 100644 --- a/tests/test_s3_specific.py +++ b/tests/test_s3_specific.py @@ -164,13 +164,11 @@ def test_fake_directories(s3_like_rig): assert not test_case.is_file() -def test_directories(s3_like_rig_all): +def test_directories(s3_like_rig): """Fixing bug that marks as directories prefixes of existing paths Ref: https://github.com/drivendataorg/cloudpathlib/issues/208 """ - s3_like_rig = s3_like_rig_all - super_path = s3_like_rig.create_cloud_path("dir_0/file0_0.txt") assert super_path.exists() assert not super_path.is_dir() From c76c96a6175098286d9f0364e7234dafd70978be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Mart=C3=ADnez=20Gonz=C3=A1lez?= Date: Thu, 11 Aug 2022 09:57:28 +0200 Subject: [PATCH 4/4] update history --- HISTORY.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/HISTORY.md b/HISTORY.md index 987ed080..251c3373 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,5 +1,9 @@ # cloudpathlib Changelog +## v0.9.1 (UNRELEASED) + + - Fixed S3Path.exists() returns True on partial matches. ([Issue #208](https://github.com/drivendataorg/cloudpathlib/issues/208), [PR #244](https://github.com/drivendataorg/cloudpathlib/pull/244)) + ## v0.9.0 (2022-06-03) - Added `absolute` to `CloudPath` (does nothing as `CloudPath` is always absolute) ([PR #230](https://github.com/drivendataorg/cloudpathlib/pull/230)) - Added `resolve` to `CloudPath` (does nothing as `CloudPath` is resolved in advance) ([Issue #151](https://github.com/drivendataorg/cloudpathlib/issues/151), [PR #230](https://github.com/drivendataorg/cloudpathlib/pull/230))