fix: Make watchdog @BetaApi
and watchdog provider implementations @InternalApi
#1119
Conversation
…ns `@InternalApi` This is to address googleapis#829. Also add grpclb lib to bazel dependencies, as its absence was failing some of the tests well executing from bazel.
Codecov Report
@@ Coverage Diff @@
## master #1119 +/- ##
=========================================
Coverage 78.72% 78.72%
Complexity 1169 1169
=========================================
Files 204 204
Lines 5184 5184
Branches 416 416
=========================================
Hits 4081 4081
Misses 928 928
Partials 175 175
Continue to review full report at Codecov.
|
import java.util.concurrent.ScheduledExecutorService; | ||
import javax.annotation.Nonnull; | ||
import javax.annotation.Nullable; | ||
import org.threeten.bp.Duration; | ||
|
||
@BetaApi("The surface for streaming is not stable yet and may change in the future.") | ||
public class FixedWatchdogProvider implements WatchdogProvider { | ||
@InternalApi |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
Constructive feedback: please add some javadoc to the InternalApi tagged classes stating that they are internal and can change w/o notice. Less constructive feedback: I still don't think that the watchdog should be exposed as public api |
@BetaApi
and watchdog provider implementations `@InternalApi@BetaApi
and watchdog provider implementations @InternalApi
@igorbernstein2, @miraleung PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
This is to address #829.
Also add grpclb lib to bazel dependencies, as its absence was failing some of the tests well executed from bazel.
This PR basically repeats #881, with few more things in it (like making watchog final, as requested in the review there)