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.

Sign up for GitHub

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

Add options to mkdir #610

Merged
merged 2 commits into from
Mar 12, 2024
Merged

Add options to mkdir #610

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dfc8ee5
add mkdir options
slevang Mar 3, 2024
bad5b79
different approach
slevang Mar 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion gcsfs/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,8 @@ async def _mkdir(
location=None,
create_parents=True,
enable_versioning=False,
uniform_access=False,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how many kwargs we should pass before we start to make some Options dataclass

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair. Would you be in favor of replicating the entire Buckets:Insert api? Or at least the gcloud storage buckets create options? I wanted to add these in particular because they are the default (and recommended) options for buckets created through the console UI.

Looks like some others we are missing are class/autoclass, retention policy and requester pays. Plus some more niche options.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we allow "iamConfiguration" as a kwarg, so the caller can put whatever they like in it? We could also other arguments that the API has - then it's just a JSON blob we pass along.

public_access_prevention=True,
**kwargs,
):
"""
Expand Down Expand Up @@ -865,6 +867,12 @@ async def _mkdir(
enable_versioning: bool
If True, creates the bucket in question with object versioning
enabled.
uniform_access: bool
If True, creates the bucket in question with uniform access
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if most people will know what "uniform access" means

enabled.
public_access_prevention: bool
If True, creates the bucket in question with public access
prevention enabled.
"""
bucket, object, generation = self.split_path(path)
if bucket in ["", "/"]:
Expand All @@ -877,12 +885,22 @@ async def _mkdir(
return
raise FileNotFoundError(bucket)

json_data = {"name": bucket}
json_data = {"name": bucket, "iamConfiguration": {}}
location = location or self.default_location
if location:
json_data["location"] = location
if enable_versioning:
json_data["versioning"] = {"enabled": True}
if uniform_access:
# Cannot use ACLs with uniform access
acl = None
default_acl = None
json_data["iamConfiguration"]["uniformBucketLevelAccess"] = {
"enabled": True
}
if public_access_prevention:
json_data["iamConfiguration"]["publicAccessPrevention"] = "enforced"

await self._call(
method="POST",
path="b",
Expand Down
1 change: 1 addition & 0 deletions gcsfs/tests/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@
TEST_BUCKET = os.getenv("GCSFS_TEST_BUCKET", "gcsfs_test")
TEST_PROJECT = os.getenv("GCSFS_TEST_PROJECT", "project")
TEST_REQUESTER_PAYS_BUCKET = "gcsfs_test_req_pay"
TEST_CUSTOM_BUCKET = os.getenv("GCSFS_TEST_CUSTOM_BUCKET", "gcsfs_test_custom")
14 changes: 14 additions & 0 deletions gcsfs/tests/test_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
TEST_BUCKET = gcsfs.tests.settings.TEST_BUCKET
TEST_PROJECT = gcsfs.tests.settings.TEST_PROJECT
TEST_REQUESTER_PAYS_BUCKET = gcsfs.tests.settings.TEST_REQUESTER_PAYS_BUCKET
TEST_CUSTOM_BUCKET = gcsfs.tests.settings.TEST_CUSTOM_BUCKET


def test_simple(gcs):
Expand Down Expand Up @@ -1478,3 +1479,16 @@ def test_find_maxdepth(gcs):

with pytest.raises(ValueError, match="maxdepth must be at least 1"):
gcs.find(f"{TEST_BUCKET}/nested", maxdepth=0)


def test_mkdir_options(gcs):
gcs = GCSFileSystem(endpoint_url=gcs._endpoint)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why make a new instance here?

if not gcs.on_google:
pytest.skip("emulator doesn't support IAM policies.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead, we should mock to make sure that at least the right IAM config dict is being sent


gcs.mkdir(TEST_CUSTOM_BUCKET, uniform_access=True, public_access_prevention=True)
info = gcs.info(TEST_CUSTOM_BUCKET)
gcs.rm(TEST_CUSTOM_BUCKET, recursive=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a fixture to make sure the bucket is cleaned up even on failure


assert info["iamConfiguration"]["uniformBucketLevelAccess"]["enabled"]
assert info["iamConfiguration"]["publicAccessPrevention"] == "enforced"
Loading