Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

fix: Make watchdog @BetaApi and watchdog provider implementations @InternalApi #1119

Merged
merged 2 commits into from Jun 5, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions gax-grpc/BUILD.bazel
Expand Up @@ -24,6 +24,7 @@ _COMPILE_DEPS = [
"@com_google_http_client_google_http_client//jar",
"@io_grpc_grpc_java//context:context",
"@io_grpc_grpc_netty_shaded//jar",
"@io_grpc_grpc_grpclb//jar",
"@io_grpc_grpc_java//alts:alts",
"@io_netty_netty_tcnative_boringssl_static//jar",
"@javax_annotation_javax_annotation_api//jar",
Expand Down
Expand Up @@ -31,13 +31,22 @@

import com.google.api.core.ApiClock;
import com.google.api.core.BetaApi;
import com.google.api.core.InternalApi;
import java.util.concurrent.ScheduledExecutorService;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.threeten.bp.Duration;

/**
* A watchdog provider which always returns the same watchdog instance provided to the provider
* during construction.
*
* <p>This is the internal class and is public only for technical reasons. It may change any time
* without notice, please do not depend on it explicitly.
*/
@BetaApi("The surface for streaming is not stable yet and may change in the future.")
public class FixedWatchdogProvider implements WatchdogProvider {
@InternalApi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What side effects will external users see as a result of changing this from BetaApi to InternalApi?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strictly speaking it does not stop being beta API (the beta annotation is still there), it becomes both beta and internal. It is a conventional thing, so nothing should break because of that.

As for conventions, with both of these annotations present, people should be even more discouraged to use these classes in their code. I.e. @BetaApi may mean that this api will change in the future, and adding @InternalApi annotation indicates that the change has happened, and now the users are definitely not supposed to use this class in their code.

public final class FixedWatchdogProvider implements WatchdogProvider {
@Nullable private final Watchdog watchdog;

public static WatchdogProvider create(Watchdog watchdog) {
Expand Down
Expand Up @@ -31,13 +31,21 @@

import com.google.api.core.ApiClock;
import com.google.api.core.BetaApi;
import com.google.api.core.InternalApi;
import com.google.common.base.Preconditions;
import java.util.concurrent.ScheduledExecutorService;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.threeten.bp.Duration;

/**
* A watchdog provider which instantiates a new provider on every request.
*
* <p>This is the internal class and is public only for technical reasons. It may change any time
* without notice, please do not depend on it explicitly.
*/
@BetaApi("The surface for streaming is not stable yet and may change in the future.")
@InternalApi
public final class InstantiatingWatchdogProvider implements WatchdogProvider {
@Nullable private final ApiClock clock;
@Nullable private final ScheduledExecutorService executor;
Expand Down
6 changes: 3 additions & 3 deletions gax/src/main/java/com/google/api/gax/rpc/Watchdog.java
Expand Up @@ -30,7 +30,7 @@
package com.google.api.gax.rpc;

import com.google.api.core.ApiClock;
import com.google.api.core.InternalApi;
import com.google.api.core.BetaApi;
import com.google.api.gax.core.BackgroundResource;
import com.google.common.base.Preconditions;
import java.util.Iterator;
Expand Down Expand Up @@ -59,8 +59,8 @@
* had no outstanding demand. Duration.ZERO disables the timeout.
* </ul>
*/
@InternalApi
public class Watchdog implements Runnable, BackgroundResource {
@BetaApi
public final class Watchdog implements Runnable, BackgroundResource {
// Dummy value to convert the ConcurrentHashMap into a Set
private static Object PRESENT = new Object();
private final ConcurrentHashMap<WatchdogStream, Object> openStreams = new ConcurrentHashMap<>();
Expand Down
Expand Up @@ -230,7 +230,11 @@ private void runTest(
null);
Credentials credentials = Mockito.mock(Credentials.class);
ApiClock clock = Mockito.mock(ApiClock.class);
Watchdog watchdog = Mockito.mock(Watchdog.class);
Watchdog watchdog =
Watchdog.create(
Mockito.mock(ApiClock.class),
Duration.ZERO,
Mockito.mock(ScheduledExecutorService.class));
Duration watchdogCheckInterval = Duration.ofSeconds(11);

builder.setExecutorProvider(executorProvider);
Expand Down
Expand Up @@ -142,7 +142,11 @@ public void testBuilderFromClientContext() throws Exception {
ApiClock clock = Mockito.mock(ApiClock.class);
ApiCallContext callContext = FakeCallContext.createDefault();
Map<String, String> headers = Collections.singletonMap("spiffykey", "spiffyvalue");
Watchdog watchdog = Mockito.mock(Watchdog.class);
Watchdog watchdog =
Watchdog.create(
Mockito.mock(ApiClock.class),
Duration.ZERO,
Mockito.mock(ScheduledExecutorService.class));
Duration watchdogCheckInterval = Duration.ofSeconds(12);

ClientContext clientContext =
Expand Down
Expand Up @@ -49,14 +49,24 @@ public void testNull() {

@Test
public void testSameInstance() {
Watchdog watchdog = Mockito.mock(Watchdog.class);
Watchdog watchdog =
Watchdog.create(
Mockito.mock(ApiClock.class),
Duration.ZERO,
Mockito.mock(ScheduledExecutorService.class));

WatchdogProvider provider = FixedWatchdogProvider.create(watchdog);
assertThat(provider.getWatchdog()).isSameInstanceAs(watchdog);
}

@Test
public void testNoModifications() {
WatchdogProvider provider = FixedWatchdogProvider.create(Mockito.mock(Watchdog.class));
Watchdog watchdog =
Watchdog.create(
Mockito.mock(ApiClock.class),
Duration.ZERO,
Mockito.mock(ScheduledExecutorService.class));
WatchdogProvider provider = FixedWatchdogProvider.create(watchdog);

assertThat(provider.needsCheckInterval()).isFalse();
assertThat(provider.needsClock()).isFalse();
Expand Down
8 changes: 8 additions & 0 deletions repositories.bzl
Expand Up @@ -79,6 +79,14 @@ def com_google_api_gax_java_repositories():
licenses = ["notice", "reciprocal"],
)

_maybe(
jvm_maven_import_external,
name = "io_grpc_grpc_grpclb",
artifact = "io.grpc:grpc-grpclb:%s" % PROPERTIES["version.io_grpc"],
server_urls = ["https://repo.maven.apache.org/maven2/", "http://repo1.maven.org/maven2/"],
licenses = ["notice", "reciprocal"],
)

_maybe(
jvm_maven_import_external,
name = "google_java_format_all_deps",
Expand Down