Sruhvx-jpg commented on PR #3165:
URL: https://github.com/apache/iceberg-rust/pull/3165#issuecomment-5698483835
@laskoviymishka addressed all the feedback and added several hardening
improvements:
1. **SSE & Unexpressible Configs**:
- Wired `with_sse_kms_encryption` (KMS) and `with_ssec_encryption` (SSE-C
/ custom keys) into `build_s3_store`.
- Added explicit fail-fast errors (`ErrorKind::FeatureUnsupported`) for
options `object_store` can't express (`role_arn`, `disable_ec2_metadata`,
`disable_config_load`) instead of dropping them silently.
2. **URL Parsing & Type Encapsulation**:
- Reworked `parse_s3_url` to use native `Url` fields (`url.scheme()`,
`url.host_str()`, `url.path()`) and encapsulated returns in a `ParsedS3Url`
struct.
- Dropped all raw string slicing math; uppercase schemes (`S3://`,
`S3A://`) and percent-encoded paths/buckets are parsed cleanly.
3. **Public API & Cache Encapsulation**:
- Encapsulated `store_cache` and `config` inside a private `S3Storage`
struct with a `StoreCache` type alias. Public enum variant is now opaque
`ObjectStoreStorage::S3(S3Storage)` and `public-api.txt` is updated.
- Introduced a `StoreAndPath` struct wrapper for internal storage
dispatch, eliminating tuple destructuring across all 9 `Storage` methods.
4. **Orphaned Multipart Abort on Drop**:
- Implemented `Drop` on `ObjectStoreWriter` to asynchronously trigger
`writer.abort()` via the Tokio runtime if dropped before `close()`.
5. **Type Safety & Redundant Cast Cleanup**:
- Added `to_file_metadata` constructor helper, eliminating redundant `as
u64` casts in `metadata()` (and opened #3238 to track standardizing this across
other backends).
6. **Integration & Unit Tests (AI-assisted)**:
- Added `crates/storage/object_store/tests/file_io_s3_test.rs` testing
real read/write/delete against MinIO (matching OpenDAL's test suite).
- Added comprehensive unit tests for SSE, unexpressible error guards, and
URL edge cases. (Used AI specifically to accelerate scaffolding and expanding
the test coverage matrix).
All checks, clippy lints, and CI tests are green. Ready for another pass
whenever you have 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: [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]
