CTTY commented on code in PR #2338:
URL: https://github.com/apache/iceberg-rust/pull/2338#discussion_r3103951303
##########
crates/storage/opendal/src/lib.rs:
##########
@@ -274,15 +259,21 @@ impl OpenDalStorage {
}
#[cfg(feature = "opendal-s3")]
OpenDalStorage::S3 {
- configured_scheme,
config,
customized_credential_load,
} => {
let op = s3_config_build(config, customized_credential_load,
path)?;
let op_info = op.info();
- // Check prefix of s3 path.
- let prefix = format!("{}://{}/", configured_scheme,
op_info.name());
+ // Use the URL scheme in the path for prefix matching. This
enables
+ // use of S3-compatible storage backends using custom schemes
(e.g., `minio://`, `r2://`).
Review Comment:
this is a good point, maybe OSS storage can use this directly so we don't
even need to fallback to OpenDalStorage::OSS?
cc @plusplusjiajia
--
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]
