feat(storage/opendal): route oss:// URLs through the S3 backend#2332
feat(storage/opendal): route oss:// URLs through the S3 backend#2332plusplusjiajia wants to merge 2 commits into
Conversation
CTTY
left a comment
There was a problem hiding this comment.
Hi @plusplusjiajia thanks for the contribution! Logic wise this makes sense, but I'm concerned that this will confuse users that OSS storage has two modes. Do you think we should just keep one?
| #[cfg(feature = "opendal-oss")] | ||
| // OSS is S3-API-compatible; route through S3 so `s3.*` props | ||
| // work for OSS-backed tables (mirrors pyiceberg/Java S3FileIO). | ||
| #[cfg(feature = "opendal-s3")] |
There was a problem hiding this comment.
| #[cfg(feature = "opendal-s3")] | |
| #[cfg(all(feature = "opendal-oss", feature = "opendal-s3"))] |
Only having s3 feature flag will introduce oss related code to non-oss users
| } | ||
| // Fallback: builds without `opendal-s3` but with `opendal-oss` | ||
| // still use the native OSS service (which consumes `oss.*` keys). | ||
| #[cfg(all(feature = "opendal-oss", not(feature = "opendal-s3")))] |
There was a problem hiding this comment.
I'm not very familiar with OSS storage so not sure about it: Is the fallback actually necessary? I'm thinking if using OSS exactly like S3 is the common case, maybe we can just use OpenDalS3Storage directly?
There was a problem hiding this comment.
@CTTY Agreed, removed it. OSS is S3-compatible and the Java/Python Iceberg implementations both route OSS through S3FileIO without a native fallback.
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions. |
db91b0b to
3260943
Compare
Instead of trying to route OSS to S3, I suggest allowing OSS to parse options from the S3 config. OpenDAL's OSS support is natively implemented, which means it works with Aliyun OSS's RAM out of the box. Switching to S3 services prevents users from working with Aliyun's OIDC or assume roles. |
Good point. I've removed the native OSS fallback entirely — OSS now only routes through S3, matching what pyiceberg does (apache/iceberg-python#1392). |
3260943 to
b73b446
Compare
Which issue does this PR close?
N/A
What changes are included in this PR?
Aliyun OSS exposes an S3-compatible API, and the other two Iceberg implementations already read oss:// tables through their S3 code path with canonical s3.* properties:
iceberg-rust routes oss:// to opendal's native Oss service instead, which reads a separate oss.* namespace and can't be driven by the same credentials. Align with the rest of the ecosystem.