laxmanmamididd commented on PR #7363: URL: https://github.com/apache/datafusion-comet/pull/7363#issuecomment-6670456190
Thanks @dwsmith1983. All of it is in 286d711c8, and the branch is now rebased onto #7464's head. - **`uri` is required whatever form the endpoint takes.** `withVendedCredentialProvider` selects the provider only when the properties carry a non-blank `uri`. The `CometIcebergNativeScanSuite` case now runs a relative and an absolute endpoint, each with no `uri`, a blank one and a real one. The other selection tests' fixture uses a relative endpoint with a `uri`. - **Docs.** The design doc now says the provider is selected for the native scan, not the writer, and that it needs a refresh endpoint and a `uri`. The class Javadoc says scans. - **Refresh test.** The `CometIcebergNativeSuite` test now has the catalog return `/v1/credentials`. On Spark 4.x it also checks that the serialized catalog properties carry the REST catalog's `uri`. - **`close()`.** `closeClosesTheIcebergProvider` sets `client.credentials-provider` to a closeable test provider, reads through it, closes ours, and checks the delegate was closed once. - **Branch layout.** The provider commits now sit directly on #7464's head, with no cherry-picked copy of the lifecycle commit, and the description no longer counts commits. Run locally with `-Pspark-4.0` on JDK 17: - `IcebergVendedS3CredentialProviderTest` (11); - `CometIcebergNativeScanSuite`; - the `CometIcebergNativeSuite` refresh test; - scalastyle. -- 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]
