You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/07/02 04:53:32 UTC

[GitHub] [beam] kennknowles opened a new pull request #12162: [BEAM-10402] Enable checker framework and eliminate nullability errors in some modules, disable it in other modules

kennknowles opened a new pull request #12162:
URL: https://github.com/apache/beam/pull/12162


   I enabled the checker framework and immediately discovered over 100 nullability errors in the main Java SDK. Opening a PR as the simple way to publicize the breadth of errors in Beam. I will fix errors in one or more modules then disable the framework in the remaining modules.
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
    - [x] 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.
    - [ ] Update `CHANGES.md` with noteworthy changes.
    - [x] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
   
   Post-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
   Java | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
   Python | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
   XLang | --- | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
   
   Pre-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   --- |Java | Python | Go | Website
   --- | --- | --- | --- | ---
   Non-portable | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
   Portable | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
   
   See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kennknowles commented on pull request #12162: [BEAM-10402] Enable checker framework, disabled everywhere since every module has errors. Eliminate some errors (but no complete modules)

Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12162:
URL: https://github.com/apache/beam/pull/12162#issuecomment-654434497


   There is no 3.4.x or 3.5.0 release of that artifact. The failed target also succeeded for me locally.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kennknowles commented on pull request #12162: [BEAM-10402] Enable checker framework, disabled everywhere since every module has errors. Eliminate some errors (but no complete modules)

Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12162:
URL: https://github.com/apache/beam/pull/12162#issuecomment-654523685


   run python precommit


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kennknowles commented on pull request #12162: [BEAM-10402] Enable checker framework, disabled everywhere since every module has errors. Eliminate some errors (but no complete modules)

Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12162:
URL: https://github.com/apache/beam/pull/12162#issuecomment-654365801






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kennknowles commented on pull request #12162: [BEAM-10402] Enable checker framework, disabled everywhere since every module has errors. Eliminate some errors (but no complete modules)

Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12162:
URL: https://github.com/apache/beam/pull/12162#issuecomment-653295836


   retest this please


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] TheNeuralBit commented on pull request #12162: [BEAM-10402] Enable checker framework, disabled everywhere since every module has errors. Eliminate some errors (but no complete modules)

Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on pull request #12162:
URL: https://github.com/apache/beam/pull/12162#issuecomment-654321921


   does the @DefaultAnnotation(NonNull.class) still work?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kennknowles merged pull request #12162: [BEAM-10402] Enable checker framework, disabled everywhere since every module has errors. Eliminate some errors (but no complete modules)

Posted by GitBox <gi...@apache.org>.
kennknowles merged pull request #12162:
URL: https://github.com/apache/beam/pull/12162


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kennknowles commented on a change in pull request #12162: [BEAM-10402] Enable checker framework, disabled everywhere since every module has errors. Eliminate some errors (but no complete modules)

Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #12162:
URL: https://github.com/apache/beam/pull/12162#discussion_r449273320



##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/metrics/MetricFiltering.java
##########
@@ -19,6 +19,7 @@
 
 import java.util.Set;
 import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.base.Objects;
+import org.checkerframework.checker.nullness.qual.Nullable;

Review comment:
       `javax.annotations.Nullable` has incorrect qualifiers, so it cannot be applied in all contexts where it is applicable.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kennknowles commented on pull request #12162: [BEAM-10402] Enable checker framework, disabled everywhere since every module has errors. Eliminate some errors (but no complete modules)

Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12162:
URL: https://github.com/apache/beam/pull/12162#issuecomment-654479077


   OK actually with `:examples:kotlin:dependencyReport` we can reproduce the error in the report. But for some reason it doesn't fail the license report tasks locally.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kennknowles commented on pull request #12162: [BEAM-10402] Enable checker framework, disabled everywhere since every module has errors. Eliminate some errors (but no complete modules)

Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12162:
URL: https://github.com/apache/beam/pull/12162#issuecomment-654515881






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kennknowles commented on pull request #12162: [BEAM-10402] Enable checker framework, disabled everywhere since every module has errors. Eliminate some errors (but no complete modules)

Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12162:
URL: https://github.com/apache/beam/pull/12162#issuecomment-654367733


   Ah, those failures are a good catch, actually. The versions of the analyzer are not pinned, and they released 3.5.0 on July 1. As of this moment it is not yet present at https://repo.maven.apache.org/maven2/org/checkerframework/jdk8/ (actually 3.3.0 is the latest there)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kennknowles commented on pull request #12162: [BEAM-10402] Enable checker framework, disabled everywhere since every module has errors. Eliminate some errors (but no complete modules)

Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12162:
URL: https://github.com/apache/beam/pull/12162#issuecomment-654366722


   Run Python2_PVR_Flink PreCommit


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kennknowles commented on pull request #12162: [BEAM-10402] Enable checker framework, disabled everywhere since every module has errors. Eliminate some errors (but no complete modules)

Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12162:
URL: https://github.com/apache/beam/pull/12162#issuecomment-654474785


   There really is something different about Jenkins. I cannot reproduce the failure in any local context. In the failing context, some part of checkerframework gradle plugin cannot determine the version of JDK8 to use so defaults to the version of checker it is using, which is 3.5.0. But the versions do not track together so this breaks.
   
   It is unfortunate, because as it turns out some of our smaller, perhaps trivial, modules, do not have errors. It would be nice to leave it enabled for them. But I would rather get the infrastructure merged in place so I can focus on re-enabling, so I am disabling it in things like the examples and job servers, which otherwise are OK.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kennknowles commented on pull request #12162: [BEAM-10402] Enable checker framework, disabled everywhere since every module has errors. Eliminate some errors (but no complete modules)

Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12162:
URL: https://github.com/apache/beam/pull/12162#issuecomment-654559015


   Run Java PreCommit


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kennknowles commented on pull request #12162: [BEAM-10402] Enable checker framework, disabled everywhere since every module has errors. Eliminate some errors (but no complete modules)

Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12162:
URL: https://github.com/apache/beam/pull/12162#issuecomment-654366549


   Maybe wait a week or two after getting this more established, then we can remove the spotbugs legacy annotations and turn off spotbugs nullness since it is a heuristic with false positives and false negatives, of which I believe checker has neither.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kennknowles edited a comment on pull request #12162: [BEAM-10402] Enable checker framework, disabled everywhere since every module has errors. Eliminate some errors (but no complete modules)

Posted by GitBox <gi...@apache.org>.
kennknowles edited a comment on pull request #12162:
URL: https://github.com/apache/beam/pull/12162#issuecomment-653252161


   @TheNeuralBit I finally got to it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kennknowles commented on pull request #12162: [BEAM-10402] Enable checker framework, disabled everywhere since every module has errors. Eliminate some errors (but no complete modules)

Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12162:
URL: https://github.com/apache/beam/pull/12162#issuecomment-653252161


   @TheNeuralBit I finally got to it in my free time.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] kennknowles commented on pull request #12162: [BEAM-10402] Enable checker framework, disabled everywhere since every module has errors. Eliminate some errors (but no complete modules)

Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12162:
URL: https://github.com/apache/beam/pull/12162#issuecomment-654558922


   A problem in Samza's state backend on https://ci-beam.apache.org/job/beam_PreCommit_Java_Phrase/2428/. We really need to separate test runs for different artifacts...


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org