Skip to content

Conversation

wang2bo2
Copy link
Contributor

@wang2bo2 wang2bo2 commented Nov 7, 2024

Parsing S3 access point URI, like s3://arn:aws:s3:us-west-2:1234:accesspoint/abc/123.jpg, causes problems in fsspec.utils.infer_storage_options because the :accesspoint was parsed as port number.

This patch removes the need to call fsspec.utils.infer_storage_options in s3fs, as discussed in fsspec/filesystem_spec#1743 (comment)

@martindurant
Copy link
Member

For an example URL like this, does the bucket end up being "arn:aws:s3:us-west-2:1234:accesspoint"?

We should have a test ensuring that this does the right thing.

@wang2bo2
Copy link
Contributor Author

wang2bo2 commented Nov 8, 2024

There’s already a test on checking the accesspoint bucket parsing

"arn:aws:s3:region:123456789012:accesspoint/my-access-point-name",

@martindurant
Copy link
Member

There’s already a test

but the codepath you fixed is for url_to_fs, not split_path, right? That test was already passing without this change.

@wang2bo2
Copy link
Contributor Author

wang2bo2 commented Nov 9, 2024

The newly added test would check S3FileSystem's compatibility with fsspec.url_to_fs and this test would fail without the fix in this PR.

@martindurant martindurant merged commit 78249af into fsspec:main Nov 11, 2024
25 checks passed
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.

2 participants