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

Remove InternalApi from Watchdog, and add InternalApi to FixedWatchdogProvider #881

Closed
wants to merge 6 commits into from
Closed

Remove InternalApi from Watchdog, and add InternalApi to FixedWatchdogProvider #881

wants to merge 6 commits into from

Conversation

yihanzhen
Copy link
Contributor

@yihanzhen yihanzhen commented Feb 24, 2020 •
edited

Fix #829

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 24, 2020
@codecov
Copy link

codecov bot commented Feb 24, 2020 •
edited

Codecov Report

Merging #881 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #881   +/-   ##
=========================================
  Coverage     78.63%   78.63%           
  Complexity     1163     1163           
=========================================
  Files           203      203           
  Lines          5143     5143           
  Branches        413      413           
=========================================
  Hits           4044     4044           
  Misses          925      925           
  Partials        174      174
Impacted Files Coverage Δ Complexity Δ
...a/com/google/api/gax/tracing/OpencensusTracer.java 87.50% <0.00%> (ø) 32.00% <0.00%> (ø%)
.../java/com/google/longrunning/OperationsClient.java 73.33% <0.00%> (ø) 16.00% <0.00%> (ø%)
...api/gax/grpc/InstantiatingGrpcChannelProvider.java 79.89% <0.00%> (ø) 35.00% <0.00%> (ø%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a5b940...1b3470e. Read the comment docs.

igorbernstein2
igorbernstein2 reviewed Feb 24, 2020
View reviewed changes
gax/src/main/java/com/google/api/gax/rpc/Watchdog.java
@@ -59,7 +58,6 @@
* had no outstanding demand. Duration.ZERO disables the timeout.
* </ul>
*/
@InternalApi
Copy link
Contributor

Choose a reason for hiding this comment

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

EndUsers should really not be setting Watchdogs directly. However if you insist on continuing on with this PR, at least mark this as BetaApi, since its api was never reviewed for public consumption and please mark it as final so that users don't try to extend it

Copy link
Contributor Author

@yihanzhen yihanzhen Feb 24, 2020 •
edited

Choose a reason for hiding this comment

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

But isn't marking a class as @BetaApi considered breaking change?

Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding is that since its marked InternalOnly, we are allowed to change this class in any way we see fit.

@yihanzhen
Copy link
Contributor Author

I can't figure out why Java 7 tests failed... @vam-google passing this to you.

@vam-google vam-google mentioned this pull request Jun 4, 2020
Merged
vam-google added a commit that referenced this pull request Jun 5, 2020
@vam-google
fix: Make watchdog @BetaApi and watchdog provider implementations `…
593924b 
…@InternalApi` (#1119)

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)
@vam-google
Copy link
Contributor

Closing as obsolete (implemented as parto of #1119)

@vam-google vam-google closed this Jun 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@igorbernstein2 igorbernstein2 igorbernstein2 left review comments

@vam-google vam-google Awaiting requested review from vam-google

Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Watchdog is internal but uses aren't
4 participants
@yihanzhen @vam-google @igorbernstein2 @googlebot