stats-dev commented on code in PR #2948:
URL: https://github.com/apache/iceberg-python/pull/2948#discussion_r2726184453
##########
pyiceberg/io/fsspec.py:
##########
@@ -205,7 +207,13 @@ def _s3(properties: Properties) -> AbstractFileSystem:
else:
anon = False
- fs = S3FileSystem(anon=anon, client_kwargs=client_kwargs,
config_kwargs=config_kwargs)
+ session = None
+ if profile_name := get_first_property_value(properties, S3_PROFILE_NAME,
AWS_PROFILE_NAME):
+ from aiobotocore.session import AioSession
+
+ session = AioSession(profile=profile_name)
Review Comment:
I agree with your opinion.. following the original s3fs docs, then getting
much simple code.
How about this suggestion including your feedback?
```
# Define arguments for S3FileSystem initialization
s3_fs_kwargs = {
"anon": anon,
"client_kwargs": client_kwargs,
"config_kwargs": config_kwargs,
}
# 1. Retrieve the profile name from properties
if profile_name := get_first_property_value(properties, S3_PROFILE_NAME,
AWS_PROFILE_NAME):
s3_fs_kwargs["profile"] = profile_name # 2. Add 'profile' to the
arguments if it exists
fs = S3FileSystem(**s3_fs_kwargs) # 3. Initialize S3FileSystem
unpacking the arguments
```
I've refactored the code to pass the profile as a keyword argument to
S3FileSystem instead, allowing s3fs to handle the session creation internally
as intended. I also updated the tests to verify that the profile argument is
passed correctly.
The implementation is now much cleaner. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
