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

[gax-java] fix: add back javax.annotation dependency #1129

Merged
merged 1 commit into from Jun 16, 2020
Merged

Conversation

miraleung
Copy link
Contributor

This dependency was removed in PR #1000 in favor of transitively pulling it in from another project. However, it doesn't build in Kokoro when updating the WORKSPACE hash in g3/tp, which is currently set just past v1.55.0. We get errors like this:

/<path>/<project>/BUILD.bazel:71:1: no such package '@javax_annotation_javax_annotation_api//jar': The repository '@javax_annotation_javax_annotation_api' could not be resolved and referenced by  <project>:<project>_java_gapic

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

codecov bot commented Jun 15, 2020

Codecov Report

Merging #1129 into master will increase coverage by 0.65%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1129      +/-   ##
============================================
+ Coverage     78.72%   79.37%   +0.65%     
- Complexity     1169     1245      +76     
============================================
  Files           204      204              
  Lines          5184     5446     +262     
  Branches        416      471      +55     
============================================
+ Hits           4081     4323     +242     
- Misses          928      931       +3     
- Partials        175      192      +17     
Impacted Files Coverage Δ Complexity Δ
...gle/api/gax/rpc/FixedTransportChannelProvider.java 96.55% <0.00%> (-3.45%) 25.00% <0.00%> (+10.00%) ⬇️
.../com/google/api/gax/rpc/FixedWatchdogProvider.java 100.00% <0.00%> (ø) 19.00% <0.00%> (+9.00%)
...gle/api/gax/rpc/InstantiatingWatchdogProvider.java 92.10% <0.00%> (+1.19%) 29.00% <0.00%> (+13.00%)
...ain/java/com/google/api/gax/rpc/ClientContext.java 82.41% <0.00%> (+1.53%) 17.00% <0.00%> (+8.00%)
...api/gax/grpc/InstantiatingGrpcChannelProvider.java 82.04% <0.00%> (+3.28%) 48.00% <0.00%> (+15.00%)
...httpjson/InstantiatingHttpJsonChannelProvider.java 77.17% <0.00%> (+6.20%) 30.00% <0.00%> (+11.00%)
...src/main/java/com/google/api/gax/rpc/Watchdog.java 87.76% <0.00%> (+7.42%) 18.00% <0.00%> (+7.00%)
...in/java/com/google/api/gax/core/GaxProperties.java 59.25% <0.00%> (+9.25%) 9.00% <0.00%> (+3.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 acf1fec...8796110. Read the comment docs.

@miraleung miraleung requested a review from noahdietz June 15, 2020 20:38
@noahdietz noahdietz removed their request for review June 15, 2020 20:48
@noahdietz
Copy link
Contributor

I'm not sure I know enough about why the dep was removed to approve replacing it. I'll let @vam-google review

@miraleung miraleung requested a review from noahdietz June 15, 2020 20:54
Copy link
Contributor

@vam-google vam-google left a comment

Choose a reason for hiding this comment

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

Looks like the dependency was removed from grpc dependencies (replaced with tomcat version) recently in grpc/grpc-java@6a50a63#diff-515bc54a0cbb4b12fb4a7c465758b011L151, so we can't consume it as a transitive anymore.

The commit above was tagged for 1.30 release. So once we updated gax to depend on grpc 1.30 it started failing. This error is not kokoro-specific and is reproducible on workstations.

The fix is LGTM. Another way to fix it would be to use @org_apache_tomcat_annotations_api dependency instead (like what gRPC started doing).

@miraleung
Copy link
Contributor Author

Summarizing our offline discussion: We decided not to switch to the tomcat dependency, because that change will need to be propagated to all dependent clients.

@miraleung miraleung merged commit 77a4cc3 into master Jun 16, 2020
@miraleung miraleung deleted the fix/deps branch June 16, 2020 23:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants