kevinjqliu commented on code in PR #1392: URL: https://github.com/apache/iceberg-python/pull/1392#discussion_r1866397952
########## pyiceberg/io/pyarrow.py: ########## @@ -350,7 +351,7 @@ def parse_location(location: str) -> Tuple[str, str, str]: return uri.scheme, uri.netloc, f"{uri.netloc}{uri.path}" def _initialize_fs(self, scheme: str, netloc: Optional[str] = None) -> FileSystem: - if scheme in {"s3", "s3a", "s3n"}: + if scheme in {"s3", "s3a", "s3n", "oss", "r2"}: Review Comment: what is `oss`? I've never heard of it. And does S3FileSystem support both oss and r2? ########## mkdocs/docs/configuration.md: ########## @@ -115,6 +115,7 @@ For the FileIO there are several configuration options available: | s3.region | us-west-2 | Sets the region of the bucket | | s3.proxy-uri | <http://my.proxy.com:8080> | Configure the proxy server to be used by the FileIO. | | s3.connect-timeout | 60.0 | Configure socket connection timeout, in seconds. | +| s3.force-virtual-addressing | False | Configure the style of requests. Set `False` to use path-style request and `True` for virtual-hosted-style request. | Review Comment: nit: perhaps just copy/paste S3FileSystem docs https://arrow.apache.org/docs/python/generated/pyarrow.fs.S3FileSystem.html ``` force_virtual_addressing bool, default False Whether to use virtual addressing of buckets. If true, then virtual addressing is always enabled. If false, then virtual addressing is only enabled if endpoint_override is empty. This can be used for non-AWS backends that only support virtual hosted-style access. ``` ########## pyiceberg/io/pyarrow.py: ########## @@ -373,6 +374,9 @@ def _initialize_fs(self, scheme: str, netloc: Optional[str] = None) -> FileSyste if session_name := get_first_property_value(self.properties, S3_ROLE_SESSION_NAME, AWS_ROLE_SESSION_NAME): client_kwargs["session_name"] = session_name + if force_virtual_addressing := self.properties.get(S3_FORCE_VIRTUAL_ADDRESSING): + client_kwargs["force_virtual_addressing"] = property_as_bool(self.properties, force_virtual_addressing, False) Review Comment: `force_virtual_addressing` docs https://arrow.apache.org/docs/python/generated/pyarrow.fs.S3FileSystem.html ########## pyiceberg/io/__init__.py: ########## @@ -304,6 +305,7 @@ def delete(self, location: Union[str, InputFile, OutputFile]) -> None: "s3": [ARROW_FILE_IO, FSSPEC_FILE_IO], "s3a": [ARROW_FILE_IO, FSSPEC_FILE_IO], "s3n": [ARROW_FILE_IO, FSSPEC_FILE_IO], + "oss": [ARROW_FILE_IO], Review Comment: nit, missing r2 here -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org