yadavay-amzn commented on PR #57191: URL: https://github.com/apache/spark/pull/57191#issuecomment-4963010771
@sarutak Thanks for reviewing! Updated the PR description and addressed the other comments (replies in-line). One note about >The provider instance may be in a half-initialized state after a failed init(). Is that safe to retry? In the `testInitRetryAfterFailure`, the fake throws immediately (before setting any state), so it does not exercise a half-initialized instance. It proves retry happens, not that the retry of partial-state is safe. In this per-workload phase `init()` runs once at startup, so repeated re-init is rare and relying on the documented contract seems reasonable here. If per-session/Connect support later makes re-init frequent, a robust option would be for the loader to discard and re-create a fresh provider instance on an `init()` failure, so a retry never runs on a half-initialized instance regardless of the implementation. Since `CredentialProviderLoader` is `@Private`, that can be added later. I'm assuming this okay to do as a followup but wanted to check if you'd like this incorporated into this PR. -- 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]
