Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: introduce java.time methods and variables #2826

Merged
merged 3 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,7 @@ final class GrpcStorageImpl extends BaseService<StorageOptions>
public void close() throws Exception {
try (StorageClient s = storageClient) {
s.shutdownNow();
org.threeten.bp.Duration terminationAwaitDuration =
getOptions().getTerminationAwaitDuration();
java.time.Duration terminationAwaitDuration = getOptions().getTerminationAwaitDuration();
s.awaitTermination(terminationAwaitDuration.toMillis(), TimeUnit.MILLISECONDS);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,15 @@

package com.google.cloud.storage;

import static com.google.api.gax.util.TimeConversionUtils.toJavaTimeDuration;
import static com.google.api.gax.util.TimeConversionUtils.toThreetenDuration;
import static com.google.common.base.Preconditions.checkArgument;
import static java.util.Objects.requireNonNull;

import com.google.api.core.ApiClock;
import com.google.api.core.BetaApi;
import com.google.api.core.InternalApi;
import com.google.api.core.ObsoleteApi;
import com.google.api.gax.core.CredentialsProvider;
import com.google.api.gax.core.FixedCredentialsProvider;
import com.google.api.gax.core.NoCredentialsProvider;
Expand Down Expand Up @@ -97,7 +100,6 @@
import java.util.Objects;
import java.util.Set;
import org.checkerframework.checker.nullness.qual.NonNull;
import org.threeten.bp.Duration;

/** @since 2.14.0 */
@TransportCompatibility(Transport.GRPC)
Expand All @@ -110,7 +112,7 @@ public final class GrpcStorageOptions extends StorageOptions
private static final String DEFAULT_HOST = "https://storage.googleapis.com";

private final GrpcRetryAlgorithmManager retryAlgorithmManager;
private final Duration terminationAwaitDuration;
private final java.time.Duration terminationAwaitDuration;
private final boolean attemptDirectPath;
private final boolean enableGrpcClientMetrics;

Expand All @@ -126,7 +128,8 @@ private GrpcStorageOptions(Builder builder, GrpcStorageDefaults serviceDefaults)
builder.storageRetryStrategy, serviceDefaults.getStorageRetryStrategy()));
this.terminationAwaitDuration =
MoreObjects.firstNonNull(
builder.terminationAwaitDuration, serviceDefaults.getTerminationAwaitDuration());
builder.terminationAwaitDuration,
serviceDefaults.getTerminationAwaitDurationJavaTime());
this.attemptDirectPath = builder.attemptDirectPath;
this.enableGrpcClientMetrics = builder.enableGrpcClientMetrics;
this.grpcClientMetricsManuallyEnabled = builder.grpcMetricsManuallyEnabled;
Expand All @@ -145,7 +148,7 @@ GrpcRetryAlgorithmManager getRetryAlgorithmManager() {
}

@InternalApi
Duration getTerminationAwaitDuration() {
java.time.Duration getTerminationAwaitDuration() {
return terminationAwaitDuration;
}

Expand Down Expand Up @@ -314,17 +317,17 @@ private Tuple<StorageSettings, Opts<UserProject>> resolveSettingsAndOpts() throw
// seconds.
// To allow read streams to have longer lifespans, crank up their timeouts, instead rely
// on idleTimeout below.
.setLogicalTimeout(Duration.ofDays(28))
.setLogicalTimeout(java.time.Duration.ofDays(28))
.build();
Duration totalTimeout = baseRetrySettings.getTotalTimeout();
java.time.Duration totalTimeout = baseRetrySettings.getTotalTimeoutDuration();
Set<Code> startResumableWriteRetryableCodes =
builder.startResumableWriteSettings().getRetryableCodes();

// retries for unary methods are generally handled at a different level, except
// StartResumableWrite
builder.applyToAllUnaryMethods(
input -> {
input.setSimpleTimeoutNoRetries(totalTimeout);
input.setSimpleTimeoutNoRetriesDuration(totalTimeout);
return null;
});

Expand All @@ -342,7 +345,7 @@ private Tuple<StorageSettings, Opts<UserProject>> resolveSettingsAndOpts() throw
// for reads, the stream can be held open for a long time in order to read all bytes,
// this is totally valid. instead we want to monitor if the stream is doing work and if not
// timeout.
.setIdleTimeout(totalTimeout);
.setIdleTimeoutDuration(totalTimeout);
return Tuple.of(builder.build(), defaultOpts);
}

Expand Down Expand Up @@ -412,7 +415,7 @@ protected boolean shouldRefreshService(Storage cachedService) {
public static final class Builder extends StorageOptions.Builder {

private StorageRetryStrategy storageRetryStrategy;
private Duration terminationAwaitDuration;
private java.time.Duration terminationAwaitDuration;
private boolean attemptDirectPath = GrpcStorageDefaults.INSTANCE.isAttemptDirectPath();
private boolean enableGrpcClientMetrics =
GrpcStorageDefaults.INSTANCE.isEnableGrpcClientMetrics();
Expand All @@ -436,6 +439,15 @@ public static final class Builder extends StorageOptions.Builder {
this.blobWriteSessionConfig = gso.blobWriteSessionConfig;
}

/**
* This method is obsolete. Use {@link #setTerminationAwaitJavaTimeDuration(java.time.Duration)}
* instead.
*/
@ObsoleteApi("Use setTerminationAwaitJavaTimeDuration(java.time.Duration) instead")
public Builder setTerminationAwaitDuration(org.threeten.bp.Duration terminationAwaitDuration) {
return setTerminationAwaitJavaTimeDuration(toJavaTimeDuration(terminationAwaitDuration));
}

/**
* Set the maximum duration in which to await termination of any outstanding requests when
* calling {@link Storage#close()}
Expand All @@ -444,7 +456,8 @@ public static final class Builder extends StorageOptions.Builder {
* @return the builder
* @since 2.14.0
*/
public Builder setTerminationAwaitDuration(Duration terminationAwaitDuration) {
public Builder setTerminationAwaitJavaTimeDuration(
java.time.Duration terminationAwaitDuration) {
this.terminationAwaitDuration =
requireNonNull(terminationAwaitDuration, "terminationAwaitDuration must be non null");
return this;
Expand Down Expand Up @@ -652,9 +665,15 @@ public StorageRetryStrategy getStorageRetryStrategy() {
return StorageRetryStrategy.getDefaultStorageRetryStrategy();
}

/** This method is obsolete. Use {@link #getTerminationAwaitDurationJavaTime()} instead. */
@ObsoleteApi("Use getTerminationAwaitDurationJavaTime() instead")
public org.threeten.bp.Duration getTerminationAwaitDuration() {
return toThreetenDuration(getTerminationAwaitDurationJavaTime());
}

/** @since 2.14.0 */
public Duration getTerminationAwaitDuration() {
return Duration.ofMinutes(1);
public java.time.Duration getTerminationAwaitDurationJavaTime() {
return java.time.Duration.ofMinutes(1);
}

/** @since 2.14.0 */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import com.google.common.base.Strings;
import java.io.IOException;
import java.io.InputStream;
import java.time.Duration;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
Expand All @@ -44,7 +45,6 @@
import java.util.concurrent.TimeoutException;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.threeten.bp.Duration;

/**
* Utility to create a remote storage configuration for testing. Storage options can be obtained via
Expand Down Expand Up @@ -233,13 +233,13 @@ public static RemoteStorageHelper create() throws StorageHelperException {
private static RetrySettings retrySettings() {
return RetrySettings.newBuilder()
.setMaxAttempts(10)
.setMaxRetryDelay(Duration.ofMillis(30000L))
.setTotalTimeout(Duration.ofMillis(120000L))
.setInitialRetryDelay(Duration.ofMillis(250L))
.setMaxRetryDelayDuration(Duration.ofMillis(30000L))
.setTotalTimeoutDuration(Duration.ofMillis(120000L))
.setInitialRetryDelayDuration(Duration.ofMillis(250L))
.setRetryDelayMultiplier(1.0)
.setInitialRpcTimeout(Duration.ofMillis(120000L))
.setInitialRpcTimeoutDuration(Duration.ofMillis(120000L))
.setRpcTimeoutMultiplier(1.0)
.setMaxRpcTimeout(Duration.ofMillis(120000L))
.setMaxRpcTimeoutDuration(Duration.ofMillis(120000L))
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@
import io.grpc.Status.Code;
import io.grpc.stub.StreamObserver;
import java.io.IOException;
import java.time.Duration;
import java.util.Iterator;
import java.util.concurrent.TimeUnit;
import org.junit.Test;
import org.threeten.bp.Duration;

/**
* ReadObject leverages gRPC ServerStream to read a stream of ReadObjectResponse messages spanning
Expand All @@ -48,7 +48,7 @@
* #getTotalTimeout()}, gax will interrupt the stream with a DEADLINE_EXCEEDED error.
*
* <p>Instead of relying on total stream timeout, we rely on idleTimeout for the stream via {@link
* com.google.api.gax.rpc.ServerStreamingCallSettings.Builder#setIdleTimeout(Duration)}.
* com.google.api.gax.rpc.ServerStreamingCallSettings.Builder#setIdleTimeoutDuration(Duration)}.
*
* <p>These tests force specific timeout scenarios to happen against an in-process grpc server to
* ensure our configuration of the StorageClient properly translates to the behavior we want.
Expand Down Expand Up @@ -117,7 +117,7 @@ public void readObject(
.setRetrySettings(
RetrySettings.newBuilder()
.setMaxAttempts(3)
.setTotalTimeout(Duration.ofMillis(totalTimeoutMillis))
.setTotalTimeoutDuration(Duration.ofMillis(totalTimeoutMillis))
.build())
.build()
.getStorageSettings();
Expand Down Expand Up @@ -196,7 +196,7 @@ public void readObject(
.setRetrySettings(
RetrySettings.newBuilder()
.setMaxAttempts(3)
.setTotalTimeout(Duration.ofMillis(totalTimeoutMillis))
.setTotalTimeoutDuration(Duration.ofMillis(totalTimeoutMillis))
.build())
.build()
.getStorageSettings();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,16 @@
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.time.Clock;
import java.time.Instant;
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.time.format.DateTimeFormatter;
import java.util.Arrays;
import java.util.Optional;
import java.util.logging.Logger;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.threeten.bp.Clock;
import org.threeten.bp.Instant;
import org.threeten.bp.ZoneId;
import org.threeten.bp.ZoneOffset;
import org.threeten.bp.format.DateTimeFormatter;

@RunWith(StorageITRunner.class)
@SingleBackend(Backend.TEST_BENCH)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@
import com.google.cloud.storage.it.runner.annotations.Backend;
import com.google.cloud.storage.it.runner.annotations.CrossRun;
import com.google.cloud.storage.it.runner.annotations.Inject;
import java.time.Duration;
import java.time.Instant;
import java.util.stream.StreamSupport;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.threeten.bp.Duration;
import org.threeten.bp.Instant;

@RunWith(StorageITRunner.class)
@CrossRun(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,14 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.time.Duration;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.threeten.bp.Duration;

/**
* A {@link ManagedLifecycle} which integrates with the <a target="_blank"
Expand Down Expand Up @@ -260,10 +260,10 @@ public void start() {
runWithRetries(
TestBench.this::listRetryTests,
RetrySettings.newBuilder()
.setTotalTimeout(Duration.ofSeconds(30))
.setInitialRetryDelay(Duration.ofMillis(500))
.setTotalTimeoutDuration(Duration.ofSeconds(30))
.setInitialRetryDelayDuration(Duration.ofMillis(500))
.setRetryDelayMultiplier(1.5)
.setMaxRetryDelay(Duration.ofSeconds(5))
.setMaxRetryDelayDuration(Duration.ofSeconds(5))
.build(),
new BasicResultRetryAlgorithm<List<RetryTestResource>>() {
@Override
Expand Down Expand Up @@ -335,10 +335,10 @@ public void stop() {
throw new NotShutdownException();
},
RetrySettings.newBuilder()
.setTotalTimeout(Duration.ofSeconds(30))
.setInitialRetryDelay(Duration.ofMillis(500))
.setTotalTimeoutDuration(Duration.ofSeconds(30))
.setInitialRetryDelayDuration(Duration.ofMillis(500))
.setRetryDelayMultiplier(1.5)
.setMaxRetryDelay(Duration.ofSeconds(5))
.setMaxRetryDelayDuration(Duration.ofSeconds(5))
.build(),
new BasicResultRetryAlgorithm<List<?>>() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import com.google.common.collect.ImmutableList;
import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
Expand All @@ -43,7 +44,6 @@
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mockito;
import org.threeten.bp.Duration;

public class RemoteStorageHelperTest {

Expand Down Expand Up @@ -273,8 +273,8 @@ public void testCreateFromStream() {
assertEquals(60000, ((HttpTransportOptions) options.getTransportOptions()).getConnectTimeout());
assertEquals(60000, ((HttpTransportOptions) options.getTransportOptions()).getReadTimeout());
assertEquals(10, options.getRetrySettings().getMaxAttempts());
assertEquals(Duration.ofMillis(30000), options.getRetrySettings().getMaxRetryDelay());
assertEquals(Duration.ofMillis(120000), options.getRetrySettings().getTotalTimeout());
assertEquals(Duration.ofMillis(250), options.getRetrySettings().getInitialRetryDelay());
assertEquals(Duration.ofMillis(30000), options.getRetrySettings().getMaxRetryDelayDuration());
assertEquals(Duration.ofMillis(120000), options.getRetrySettings().getTotalTimeoutDuration());
assertEquals(Duration.ofMillis(250), options.getRetrySettings().getInitialRetryDelayDuration());
}
}