kevinjqliu commented on code in PR #13966:
URL: https://github.com/apache/iceberg/pull/13966#discussion_r2314524909
##########
azure/src/main/java/org/apache/iceberg/azure/adlsv2/VendedAdlsCredentialProvider.java:
##########
@@ -78,6 +81,29 @@ Mono<String> credentialForAccount(String storageAccount) {
}
private AccessToken sasTokenForAccount(String storageAccount) {
+ return sasTokenFromProperties(storageAccount).orElseGet(() ->
refreshSasToken(storageAccount));
+ }
Review Comment:
I see, so this first checks for sas token from `properties`, which is from
`AzureProperties`/ `ADLSFileIO` / `getTable`.
Otherwise, we fetch credential from `credentialsEndpoint`
nit on naming here, i dont think tokens are "refreshed". they are fetched
from the credentials endpoint.
Token refresh is done automatically by `SimpleTokenCache.getToken`
##########
azure/src/main/java/org/apache/iceberg/azure/adlsv2/VendedAdlsCredentialProvider.java:
##########
@@ -78,6 +81,29 @@ Mono<String> credentialForAccount(String storageAccount) {
}
private AccessToken sasTokenForAccount(String storageAccount) {
+ return sasTokenFromProperties(storageAccount).orElseGet(() ->
refreshSasToken(storageAccount));
+ }
+
+ private Optional<AccessToken> sasTokenFromProperties(String storageAccount) {
+ String sasToken = properties.get(AzureProperties.ADLS_SAS_TOKEN_PREFIX +
storageAccount);
+ String tokenExpiresAtMillis =
+ properties.get(AzureProperties.ADLS_SAS_TOKEN_EXPIRES_AT_MS_PREFIX +
storageAccount);
+
+ if (Strings.isNullOrEmpty(sasToken) ||
Strings.isNullOrEmpty(tokenExpiresAtMillis)) {
+ return Optional.empty();
+ }
+
+ Instant expiresAt =
Instant.ofEpochMilli(Long.parseLong(tokenExpiresAtMillis));
+ Instant prefetchAt = expiresAt.minus(5, ChronoUnit.MINUTES);
+
+ if (Instant.now().isAfter(prefetchAt)) {
+ return Optional.empty();
+ }
Review Comment:
curious why we set a 5 minute limit here with `prefetchAt`?
could we just return the token here, regardless of its expiration, and let
`SimpleTokenCache.getToken` decide when to refresh?
--
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]
