snmvaughan opened a new issue, #6212:
URL: https://github.com/apache/datafusion-comet/issues/6212

   **Context.** #6031 adds `CometS3LocationScopedCredentialProvider`: the 
Parquet path's `LocationScopedObjectStore` keeps the provider's policy 
locations as a snapshot and fetches them again only when S3 returns 403 
([review](https://github.com/apache/datafusion-comet/pull/6031#discussion_r4098072041)).
   
   **Problem.** A provider with no policy for the path it is asked about 
usually throws from `getCredentialsForPath` instead of returning a credential 
that S3 then rejects. The bridge turns that exception into 
`object_store::Error::Generic`, which does not trigger the refresh. Two ways 
this shows up:
   
   1. The provider has no bucket-wide policy and throws for `/`, then adds 
`warehouse/finance` mid-job. Reads under it route to `/` and keep failing, so 
the new location is never picked up.
   2. The provider folds `warehouse/finance` into `warehouse` and throws for 
the old location. Later reads under it keep failing, while a freshly built 
store would serve the same path from `/warehouse`.
   
   The store lives as long as the executor, so both stay broken until a 
restart. The user guide says a location added while a job runs is picked up, 
and suggests keying the provider's cache on the location, which leads naturally 
to that throw.
   
   **Options.**
   
   - **(a)** Treat a credential failure from a location's store like a 403: 
mark the bridge's errors with a dedicated error type the store can recognize, 
and run the same refresh and single retry.
   - **(b)** Require `getCredentialsForPath` to answer for `/` and for 
locations the provider has dropped, and say so in the Javadoc and user guide.
   
   **Proposal.** (a). It keeps the documented behavior and a simple vendor 
contract, and it changes only the implementation. If we choose (b) instead, it 
should land before `CometS3LocationScopedCredentialProvider` first ships, since 
the versioning policy treats a change in a method's meaning as breaking.
   


-- 
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]

Reply via email to