sachinnn99 commented on PR #15869:
URL: https://github.com/apache/iceberg/pull/15869#issuecomment-4363704494
Thanks for the thorough review @ajayvembu! I've addressed all your comments
in the latest push:
**1. Race condition on timer scheduling** — Added a guard to check
`refreshFuture` state before scheduling:
```java
if (refreshFuture == null || refreshFuture.isCancelled() ||
refreshFuture.isDone()) {
scheduleCredentialRefresh();
}
```
**2. NPE guard on `storageCredentials`** — Added a null/empty check at the
top of `scheduleCredentialRefresh()`.
**3. Fuzzy `startsWith()` match** — Good catch on the fuzzy match concern. I
went with a slightly different approach than suggested — `cred.prefix()` is the
full ABFS URI (e.g., `abfss://[email protected]/dir`),
not the storage account name, so using it directly as the key suffix wouldn't
match the config keys (which use just the account name like
`adls.sas-token-expires-at-ms.account1`). Instead, I extract the account via
`new ADLSLocation(cred.prefix()).storageAccount()` for an exact key lookup,
which is consistent with how
`VendedAdlsCredentialProvider.sasTokenFromProperties()` constructs the same
keys.
**4. ROOT_PREFIX** — Thanks!
**5. `clientCache` invalidation after refresh** — Added `synchronized (this)
{ this.clientCache = null; }` after updating `storageCredentials` in
`refreshStorageCredentials()`.
--
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]
