andygrove opened a new pull request, #6318: URL: https://github.com/apache/datafusion-comet/pull/6318
Backport of #6023 to `branch-1.1`. Cherry-picked from `ce455f32d948355e073638e81009ecc3e5dea349` without conflicts, and the diff is byte-identical to upstream. Every file it modifies is identical on `branch-1.1` and on `main` just before #6023, with two exceptions. The two poms differ only by the 1.1.0 version. The S3 credential provider design doc on `main` also has #6106's `FileIO` cache section, which `branch-1.1` does not; the two new sections land after the Iceberg property-bag section and do not refer to it. ## Which issue does this PR close? Closes #6022 on `branch-1.1`. #6023 already closed it on `main`. ## Rationale for this change #6023 merged after `branch-1.1` was cut at 36ab57c68, so without this 1.1.0 ships the failure in #6022. The native Parquet reader accepts only a fixed list of credential provider classes, so an `fs.s3a.aws.credentials.provider` that plain Spark accepts, such as `com.amazonaws.auth.DefaultAWSCredentialsProviderChain`, fails under Comet with `Unsupported credential provider`. The adapters are opt-in: nothing changes unless a user names one in `fs.s3a.comet.credential.provider.class`. One part applies to any SPI provider named on the Parquet path, not only the adapters. Its `initialize()` now receives the `fs.s3a.*` config subset, static keys included, where 1.0 passed an empty map. The S3 credential providers guide documents this, and with this backport 1.1.0 is the first release that behaves this way. ## What changes are included in this PR? The original change, so see #6023 for the details. No adaptations were needed. In short: - `HadoopS3ACredentialProviderAdapter` and `AwsSdkCredentialProviderAdapter` in `org.apache.comet.cloud.s3`, each with an AWS SDK v1 body for Spark 3.4/3.5 and a v2 body for Spark 4.x. - `s3.rs` forwards the `fs.s3a.*` subset to the SPI's `initialize()` on the Parquet path. - `hadoop-aws` and the matching AWS SDK at `provided` scope, versioned per Spark profile, so nothing new is bundled. Spotless now also checks the Java files under the `spark-*` shim directories, which reformats three existing test files. - The S3 credential providers guide, the design doc and the versioning policy's table of config-referenced class names cover the adapters. ## How are these changes tested? The original PR's tests, run locally on `branch-1.1` with JDK 17: - Rust: the 45 `parquet::objectstore::s3` tests pass, including the two new ones. `cargo fmt --check` and workspace clippy with `-D warnings` are clean. - JVM unit tests in `org.apache.comet.cloud.s3`: 40 pass on Spark 3.5 (SDK v1) and 47 on Spark 4.1 (SDK v2). `CometPublicApiSuite` passes. - The manual S3 suites, which CI does not run because they need Docker, ran against S3Proxy in place of MinIO, with this branch's native library. On Spark 4.1, `HadoopS3ACredentialProviderAdapterBridgeSuite`, `CometS3CredentialBridgeSuite` and `ParquetReadFromS3Suite` pass, 15 tests. On Spark 3.5, 14 of the 15 pass. The failure is `CometS3CredentialBridgeSuite`'s REST catalog test, whose Iceberg write S3Proxy rejects with an `x-amz-content-sha256` mismatch, and it fails the same way on `branch-1.1` without this PR. - `prettier --check` passes on the three changed docs. Against `branch-1.1`, the changed paths route this pull request to every suite except Spark 3.4's SQL job and the benchmark check. -- 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]
