laxmanmamididd commented on PR #7363: URL: https://github.com/apache/datafusion-comet/pull/7363#issuecomment-6584185787
Thanks for the thorough review. All of it is addressed in e7d76ad4e, and the issues are filed: - **Automatic selection** (#7387). Comet selects `IcebergVendedS3CredentialProvider` when the FileIO properties carry `client.refresh-credentials-endpoint`, refresh isn't disabled, the class is on the class path, and no provider is configured. An explicit setting wins, and an empty value opts out. This happens in `CometIcebergNativeScan.withVendedCredentialProvider`, for both the scan and the write. - **The reference class** (#7388). `IcebergRESTVendedS3Provider` and its test are deleted, and the docs point at the shipped provider. - **Spark 3.x** (#7389). The docs say the provider is Spark 4.x only. That issue stays open for a 3.x port. - **`SdkCredentialExtraction`.** The provider converts credentials through it, with an Iceberg-specific hint for anonymous providers, so empty keys fail with that message instead of a bare 403. - **Class loader errors.** A missing iceberg-aws, a failure inside Iceberg (unwrapped from `InvocationTargetException`), a missing AWS SDK (`NoClassDefFoundError`), and two copies of the SDK (`ClassCastException`, with a note on `userClassPathFirst`) each get their own message. - **Static key expiry.** When the SDK reports no expiry and the access key is the vended one, the provider reports `s3.session-token-expires-at-ms`. - **Tests.** Added a relative endpoint resolved against `uri`, the `InvocationTargetException` unwrap (endpoint with no `uri`), a missing iceberg-aws, and anonymous credentials. The static-key test sets `uri` and asserts the expiry. - **Contributor doc.** It lists three SPI implementations, the new one marked Spark 4.x only. sunchao's lifecycle finding (#7390) is fixed in the same commit; see the reply on `IcebergVendedS3CredentialProvider.java`. -- 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]
