smaheshwar-pltr commented on code in PR #13966:
URL: https://github.com/apache/iceberg/pull/13966#discussion_r2316266482
##########
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:
I've added an explanatory comment. This PR's approach feels fine to me (the
two prefetches don't mess things up - if the cache sees creds on properties are
about to expire and refreshes, the supplier can verify if it should actually
fetch according to its own prefetch, and in practice the two are the same so it
will fetch) but happy to take suggestions
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
