andygrove opened a new pull request, #6223: URL: https://github.com/apache/datafusion-comet/pull/6223
## Which issue does this PR close? Closes #6221. ## Rationale for this change This is stacked on #6031, which adds `CometS3LocationScopedCredentialProvider`. Only the last commit, `baf505ef7`, is new here. Until #6031 merges, the diff also shows its commits, so this stays a draft until then. The location-scoped store fetches the provider's locations again only after a 403. A provider with no policy for a path throws instead. So a location added while the executors are up is never picked up when the provider vends no credential for the bucket root, and a location the provider drops keeps being used after it stops vending it. Reads under those paths fail until the executors restart. #6221 has the details. ## What changes are included in this PR? - `CometS3CredentialBridge::get_credential` gives its failures a `CredentialProviderError` source. The message is unchanged. `object_store` passes a credential provider's error through to the read unchanged, so the location-scoped store can recognize it. - `LocationScopedObjectStore` treats a read that fails with that source like a 403. It uses the same bounded refresh, where the reads routed from one snapshot share one attempt, and the same single retry when the path now routes to a different location. Other errors still return without a refresh. - The "When Comet asks" paragraph in the user guide and the design doc describe the new trigger. Nothing in the `@Public` API changes. Comet just calls `getPolicyLocations` in one more situation, and the guide already asks providers to make it safe to call at any time. ## How are these changes tested? There are new tests in `location_scoped.rs`. The existing fake-provider harness can now make a location's credential fail the way the bridge reports a provider exception. - A location added after the snapshot is picked up when the root credential fails, through both `get_opts` and `get_ranges`. - A read under a dropped location whose credential fails moves to the parent location. - A credential failure on a location that the refreshed list still routes to is returned as it is. - An unrelated error does not trigger a refresh. The four positive tests fail if the trigger goes back to 403s only, and the last one fails if every error triggers a refresh. The `location_scoped`, `objectstore` and `parquet_support` Rust tests pass, and clippy with `-D warnings` and fmt are clean. I did not run the MinIO `CometS3CredentialBridgeSuite`, which needs Docker and is not part of PR CI. -- 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]
