Skip to content

Commit

Permalink
Apply suggestions from code review.
Browse files Browse the repository at this point in the history
  • Loading branch information
ChaladiMohanVamsi committed Dec 14, 2024
1 parent c92479d commit bc08205
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public Optional<Long> adlsWriteBlockSize() {
*/
public void applyClientConfiguration(String account, DataLakeFileSystemClientBuilder builder) {
String sasToken = adlsSasTokens.get(account);
if (adlsRefreshCredentialsEnabled && !Strings.isNullOrEmpty(adlsRefreshCredentialsEndpoint)) {
if (vendedAdlsCredentialProvider != null) {
builder.credential(vendedAdlsCredentialProvider.credentialForAccount(account));
} else if (sasToken != null && !sasToken.isEmpty()) {
builder.sasToken(sasToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.iceberg.util.Pair;

class AzureSasCredentialRefresher {
private final Supplier<Pair<String, Long>> sasTokenWithExpirationSupplier;
Expand All @@ -38,8 +38,8 @@ class AzureSasCredentialRefresher {
this.sasTokenWithExpirationSupplier = sasTokenWithExpirationSupplier;
this.refreshExecutor = refreshExecutor;
Pair<String, Long> sasTokenWithExpiration = sasTokenWithExpirationSupplier.get();
this.azureSasCredential = new AzureSasCredential(sasTokenWithExpiration.getLeft());
scheduleRefresh(sasTokenWithExpiration.getRight());
this.azureSasCredential = new AzureSasCredential(sasTokenWithExpiration.first());
scheduleRefresh(sasTokenWithExpiration.second());
}

public AzureSasCredential azureSasCredential() {
Expand All @@ -50,21 +50,19 @@ private void scheduleRefresh(Long expiresAtMillis) {
this.refreshExecutor.schedule(
() -> {
Pair<String, Long> sasTokenWithExpiration = sasTokenWithExpirationSupplier.get();
azureSasCredential.update(sasTokenWithExpiration.getLeft());
if (sasTokenWithExpiration.getRight() != null) {
this.scheduleRefresh(sasTokenWithExpiration.getRight());
}
azureSasCredential.update(sasTokenWithExpiration.first());
this.scheduleRefresh(sasTokenWithExpiration.second());
},
refreshDelayMillis(expireAtMillis),
refreshDelayMillis(expiresAtMillis),
TimeUnit.MILLISECONDS);
}

private long refreshDelayMillis(Long expiresAtMillis) {
long expireInMillis = expireAtMillis - System.currentTimeMillis();
long expiresInMillis = expiresAtMillis - System.currentTimeMillis();
// how much ahead of time to start the request to allow it to complete
long refreshWindowMillis = Math.min(expireInMillis / 10, MAX_REFRESH_WINDOW_MILLIS);
long refreshWindowMillis = Math.min(expiresInMillis / 10, MAX_REFRESH_WINDOW_MILLIS);
// how much time to wait before expiration
long waitIntervalMillis = expireInMillis - refreshWindowMillis;
long waitIntervalMillis = expiresInMillis - refreshWindowMillis;
// how much time to actually wait
return Math.max(waitIntervalMillis, MIN_REFRESH_WAIT_MILLIS);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.iceberg.azure.AzureProperties;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
import org.apache.iceberg.relocated.com.google.common.collect.Maps;
Expand All @@ -39,6 +38,7 @@
import org.apache.iceberg.rest.auth.OAuth2Util;
import org.apache.iceberg.rest.credentials.Credential;
import org.apache.iceberg.rest.responses.LoadCredentialsResponse;
import org.apache.iceberg.util.Pair;
import org.apache.iceberg.util.SerializableMap;
import org.apache.iceberg.util.ThreadPools;
import org.slf4j.Logger;
Expand Down

0 comments on commit bc08205

Please sign in to comment.