use anon argument when configuring s3fs.S3FileSystem

[gweaverbiodev]

Rationale for this change

s3fs.S3FileSystem requires that we pass anon directly, it cannot be passed through config_kwargs.

Are these changes tested?

yes, unit tests updated.

Are there any user-facing changes?

no

[Fokko]

@gmweaver Thanks for catching this. Looking at the PyArrow docs, shouldn't be anon be anonymouse instead?

[gweaverbiodev]

Looking at the PyArrow docs, shouldn't be anon be anonymouse instead?

s3fs.S3FileSystem uses anon, while Pyarrow uses anonymous. No changes were required for pyarrow since all arguments are passed in like this S3FileSystem(**client_kwargs).

[Fokko]

@gmweaver Good one, the names of the classes are identical 🤣

[kevinjqliu]

okay i see, we're not passing the configs to S3FileSystem as generic **kwargs but as client_kwargs and config_kwargs
https://github.com/fsspec/s3fs/blob/d46c149d89590b4b9109213f9f92cea01504eb0b/s3fs/core.py#L288-L334

[kevinjqliu]

i think theres actually an underlying bug here.
endpoint_url here is also part of S3FileSystem constructor. I dont think it should be passed via client_kwargs

iceberg-python/pyiceberg/io/fsspec.py

Line 139 in 06b9467

"endpoint_url": properties.get(S3_ENDPOINT),

[gweaverbiodev]

i think theres actually an underlying bug here.
endpoint_url here is also part of S3FileSystem constructor. I dont think it should be passed via client_kwargs

endpoint_url can be passed to the constructor or in client_kwargs.

endpoint_url (string (None)) – Use this endpoint_url, if specified. Needed for connecting to non-AWS S3 buckets. Takes precedence over endpoint_url in client_kwargs.

in older versions of s3fs, endpoint_url was not available in the constructor, so client_kwargs may be safer. overall, there is not a 1:1 correspondence between constructor params and valid params you can pass in client_kwargs.

[gweaverbiodev]

@kevinjqliu any concerns with the PR based on my last comment? hoping to get this one into 0.10.0 release 😄

[kevinjqliu]

LGTM

I double checked all the configs and verified that they are set for the right constructors.
Would be great if we can add a link to the 3 different constructors here

• S3FileSystem
• boto3.session.Session.client for client_kwargs
• botocore.config.Config for config_kwargs

[kevinjqliu]

Thanks for the PR @gmweaver great catch! And thanks @Fokko for the review

[kevinjqliu]

we're cutting a new RC for 0.10 so this should be included! :)