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

[BEAM-4948, BEAM-6267, BEAM-5559, BEAM-7289] Update the version of guava to 26.0-jre for all our vendored artifacts containing guava #8899

Merged
merged 4 commits into from
Jul 17, 2019

Conversation

lukecwik
Copy link
Member


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) and mention them in a comment (R: @username).
  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- Build Status --- --- Build Status
Java Build Status Build Status Build Status Build Status
Build Status
Build Status
Build Status Build Status Build Status
Build Status
Python Build Status
Build Status
--- Build Status
Build Status
Build Status --- --- Build Status

Pre-Commit Tests Status (on master branch)

--- Java Python Go Website
Non-portable Build Status Build Status Build Status Build Status
Portable --- Build Status --- ---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

@lukecwik
Copy link
Member Author

This is now ready for review since the vendored artifacts have been released.

@lukecwik
Copy link
Member Author

R: @iemejia @pabloem

@lukecwik
Copy link
Member Author

CC: @nymanjens

@lukecwik
Copy link
Member Author

Currently working through annotation issues with Guava upgrade as discussed in google/guava#920 and how they are incompatible with findbugs/spotbugs.

@lukecwik
Copy link
Member Author

Run Go PreCommit

@lukecwik
Copy link
Member Author

Run JavaPortabilityApi PreCommit

@lukecwik
Copy link
Member Author

Run Portable_Python PreCommit

@lukecwik
Copy link
Member Author

Run Python_PVR_Flink PreCommit

@lukecwik
Copy link
Member Author

Added @nonnull and suppressed the warning to address the spotbugs failures

Copy link
Member

@iemejia iemejia left a comment

Choose a reason for hiding this comment

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

LGTM This is so harrd to get in with ongoing rebases and others that better to do it quickly.

@pabloem
Copy link
Member

pabloem commented Jul 17, 2019

Thanks Luke! LGTM.

@iemejia
Copy link
Member

iemejia commented Jul 17, 2019

Please self merge. Have two minor comments:

  1. We used to supress spotbugs warnings via a filters exclusion file, probably worth to keep that for consistency, but we can do that after in a subsequent PR.
  2. I really did not understand why it now complains to add a @Nonnull annotation, that's a bit of a bummer if we need to do this explicit, but specially I did not get why it does not complain in other parts (luckily maybe).

@lukecwik lukecwik merged commit c2f0d28 into apache:master Jul 17, 2019
@lukecwik
Copy link
Member Author

The issue is that Guava migrated to the checkerframework @Nullable instead of the javax version which made spotbugs perform its nullness checks. For example, the Guava Function class has the parameter marked as @Nullable which means that the function must correctly handle null inputs which some of our previous Function implementations were not. So I could either update them to handle null inputs or mark them as @Nonnull. The issue with the latter is that we are now narrowing the definition from a Function that took nullable input to one that didn't which required a different FB suppression.

@udim
Copy link
Member

udim commented Jul 19, 2019

Precommits are broken: https://issues.apache.org/jira/browse/BEAM-7784

@udim
Copy link
Member

udim commented Jul 19, 2019

FYI, there are 19 files under sdks/java/maven-archetypes/ that still reference v20_0.

udim added a commit to udim/beam that referenced this pull request Jul 19, 2019
@udim udim mentioned this pull request Jul 19, 2019
3 tasks
lostluck pushed a commit to lostluck/beam that referenced this pull request Jul 23, 2019
…ava to 26.0-jre for all our vendored artifacts containing guava (apache#8899)

* fixup! Address missing auto value annotations for spotbugs and also j2objc annotations during compilation

* fixup! Fix used but undeclared deps for annotation deps, include minor clean-up of build module.

* fixup! Address spotbugs issues with incompatible nullability annotations in Guava 26.0-jre

Further details in google/guava#920

* [BEAM-4948, BEAM-6267, BEAM-5559, BEAM-7289] Update the version of guava to 26.0-jre for all our vendored artifacts containing guava
lostluck pushed a commit to lostluck/beam that referenced this pull request Jul 23, 2019
lostluck pushed a commit to lostluck/beam that referenced this pull request Jul 23, 2019
…ava to 26.0-jre for all our vendored artifacts containing guava (apache#8899)

* fixup! Address missing auto value annotations for spotbugs and also j2objc annotations during compilation

* fixup! Fix used but undeclared deps for annotation deps, include minor clean-up of build module.

* fixup! Address spotbugs issues with incompatible nullability annotations in Guava 26.0-jre

Further details in google/guava#920

* [BEAM-4948, BEAM-6267, BEAM-5559, BEAM-7289] Update the version of guava to 26.0-jre for all our vendored artifacts containing guava
lostluck pushed a commit to lostluck/beam that referenced this pull request Jul 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants