You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "Fokko (via GitHub)" <gi...@apache.org> on 2025/02/22 20:24:13 UTC

[PR] Arrow: Suppress warning and cache bucket location [iceberg-python]

Fokko opened a new pull request, #1709:
URL: https://github.com/apache/iceberg-python/pull/1709

   Attemt to remove the unneccessary warning, and cache the location of the bucket independent of the FileIO.
   
   Fixes #1705
   Fixes #1708


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Arrow: Suppress warning and cache bucket location [iceberg-python]

Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu merged PR #1709:
URL: https://github.com/apache/iceberg-python/pull/1709


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Arrow: Suppress warning and cache bucket location [iceberg-python]

Posted by "hussein-awala (via GitHub)" <gi...@apache.org>.
hussein-awala commented on code in PR #1709:
URL: https://github.com/apache/iceberg-python/pull/1709#discussion_r1966816889


##########
pyiceberg/io/pyarrow.py:
##########
@@ -414,19 +421,19 @@ def _initialize_oss_fs(self) -> FileSystem:
         return S3FileSystem(**client_kwargs)
 
     def _initialize_s3_fs(self, netloc: Optional[str]) -> FileSystem:
-        from pyarrow.fs import S3FileSystem, resolve_s3_region
+        from pyarrow.fs import S3FileSystem
 
         # Resolve region from netloc(bucket), fallback to user-provided region
         provided_region = get_first_property_value(self.properties, S3_REGION, AWS_REGION)
 
         try:
-            bucket_region = resolve_s3_region(bucket=netloc)
+            bucket_region = _cached_resolve_s3_region(bucket=netloc)
         except (OSError, TypeError):
             bucket_region = None
             logger.warning(f"Unable to resolve region for bucket {netloc}, using default region {provided_region}")

Review Comment:
   How about moving try/except to `_cached_resolve_s3_region` and caching even the None value? Does the result change when we retry to resolve the same netloc? Or do we need to warn the user every time it fails to resolve the netloc?



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Arrow: Suppress warning and cache bucket location [iceberg-python]

Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #1709:
URL: https://github.com/apache/iceberg-python/pull/1709#issuecomment-2677145552

   Thanks for the suggestion @hussein-awala 
   
   I like this approach. My only question before proceeding is whether we want the `resolve_s3_region` behavior to be on by default. https://github.com/apache/iceberg-python/issues/1708#issuecomment-2676418044
   
   We can also revisit this at a later time. 
   


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Arrow: Suppress warning and cache bucket location [iceberg-python]

Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #1709:
URL: https://github.com/apache/iceberg-python/pull/1709#discussion_r1966885810


##########
pyiceberg/io/pyarrow.py:
##########
@@ -414,19 +421,19 @@ def _initialize_oss_fs(self) -> FileSystem:
         return S3FileSystem(**client_kwargs)
 
     def _initialize_s3_fs(self, netloc: Optional[str]) -> FileSystem:
-        from pyarrow.fs import S3FileSystem, resolve_s3_region
+        from pyarrow.fs import S3FileSystem
 
         # Resolve region from netloc(bucket), fallback to user-provided region
         provided_region = get_first_property_value(self.properties, S3_REGION, AWS_REGION)
 
         try:
-            bucket_region = resolve_s3_region(bucket=netloc)
+            bucket_region = _cached_resolve_s3_region(bucket=netloc)
         except (OSError, TypeError):
             bucket_region = None
             logger.warning(f"Unable to resolve region for bucket {netloc}, using default region {provided_region}")

Review Comment:
   Hey @hussein-awala Thanks for jumping in here, and I really like your suggestion. Just pushed the suggestion, LMKWYT.
   
   > Or do we need to warn the user every time it fails to resolve the netloc?
   
   I was surprised by hearing that folks are seeing multiple warnings, since Python [only warns once by design](https://docs.python.org/3/library/warnings.html):
   
   > Repetitions of a particular warning for the same source location are typically suppressed.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org