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 2022/06/02 20:46:21 UTC

[GitHub] [beam] kennknowles opened a new pull request, #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

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

   Re-activated nullness type checking for a portion of `org.apache.beam.sdk.values` namespace.
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [x] [**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.
    - [x] 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).
   
   To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
   
   GitHub Actions Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   [![Build python source distribution and wheels](https://github.com/apache/beam/workflows/Build%20python%20source%20distribution%20and%20wheels/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python tests](https://github.com/apache/beam/workflows/Python%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java tests](https://github.com/apache/beam/workflows/Java%20Tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
   
   See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
   


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] kennknowles commented on pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

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

   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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] kennknowles merged pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

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


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] kennknowles commented on pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

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

   Hrmm. One of the licenses that we download the server is down. bouncycastle.org


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] kennknowles commented on pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

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

   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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] kennknowles commented on pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

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

   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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] kennknowles commented on pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

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

   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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] kennknowles commented on pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

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

   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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] kennknowles commented on pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

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

   Run Java_Examples_Dataflow_Java17 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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] kennknowles commented on pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

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

   Still waiting to find time to resolve conflicts and address comments.


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] kileys commented on pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

Posted by GitBox <gi...@apache.org>.
kileys commented on PR #17819:
URL: https://github.com/apache/beam/pull/17819#issuecomment-1224562434

   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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] kileys commented on a diff in pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

Posted by GitBox <gi...@apache.org>.
kileys commented on code in PR #17819:
URL: https://github.com/apache/beam/pull/17819#discussion_r908731594


##########
sdks/java/core/src/main/java/org/apache/beam/sdk/values/PValues.java:
##########
@@ -99,16 +97,19 @@ public static Map<TupleTag<?>, PCollection<?>> fullyExpand(
                     PCollection.class.getSimpleName(),
                     valueComponent.getValue()));
           }
+          @Nullable

Review Comment:
   How come the other ones doesn't need the @Nullable?



##########
sdks/java/core/src/main/java/org/apache/beam/sdk/values/TypeDescriptor.java:
##########
@@ -180,7 +178,7 @@ public Class<? super T> getRawType() {
 
   /** Returns the component type if this type is an array type, otherwise returns {@code null}. */

Review Comment:
   Can this return null?



##########
sdks/java/core/src/main/java/org/apache/beam/sdk/util/Preconditions.java:
##########
@@ -472,4 +472,63 @@ public class Preconditions {
     }
     return reference;
   }
+
+  /**
+   * Ensures that a piece of state passed as a parameter to the calling method is not null.
+   *
+   * @param obj an object reference
+   * @param errorMessageTemplate a template for the exception message should the check fail. The
+   *     message is formed by replacing each {@code %s} placeholder in the template with an
+   *     argument. These are matched by position - the first {@code %s} gets {@code
+   *     errorMessageArgs[0]}, etc. Unmatched arguments will be appended to the formatted message in
+   *     square braces. Unmatched placeholders will be left as-is.
+   * @param errorMessageArgs the arguments to be substituted into the message template. Arguments
+   *     are converted to strings using {@link String#valueOf(Object)}.
+   * @return the non-null reference that was validated
+   * @throws IllegalStateException if {@code obj} is null
+   */
+  @CanIgnoreReturnValue
+  @EnsuresNonNull("#1")
+  public static <T extends @NonNull Object> T checkStateNotNull(
+      @Nullable T obj,
+      @Nullable String errorMessageTemplate,
+      @Nullable Object... errorMessageArgs) {
+    if (obj == null) {
+      throw new IllegalArgumentException(lenientFormat(errorMessageTemplate, errorMessageArgs));
+    }
+    return obj;
+  }
+
+  /**
+   * Ensures that a piece of state is not null.
+   *
+   * <p>See {@link #checkStateNotNull(Object, String, Object...)} for details.
+   */
+  @CanIgnoreReturnValue
+  @EnsuresNonNull("#1")
+  public static <T extends @NonNull Object> T checkStateNotNull(
+      @Nullable T obj, @Nullable String errorMessageTemplate, @Nullable Object p1) {
+    if (obj == null) {
+      throw new IllegalStateException(lenientFormat(errorMessageTemplate, p1));
+    }
+    return obj;
+  }
+
+  /**
+   * Ensures that an object reference passed as a parameter to the calling method is not null.
+   *
+   * <p>See {@link #checkArgumentNotNull(Object, String, Object...)} for details.
+   */
+  @CanIgnoreReturnValue
+  @EnsuresNonNull("#1")
+  public static <T extends @NonNull Object> T checkStateNotNull(
+      @Nullable T obj,
+      @Nullable String errorMessageTemplate,
+      @Nullable Object p1,
+      @Nullable Object p2) {
+    if (obj == null) {
+      throw new IllegalStateException(lenientFormat(errorMessageTemplate, p1));

Review Comment:
   p2 not used



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] kennknowles commented on pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

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

   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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] kennknowles commented on pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

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

   Our flakes are driving me nuts - confirmed some of these tests seem OK locally. I'll rebase and push just to get any recent fixes and have to kick jenkins less.


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] kennknowles commented on pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

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

   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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] kennknowles commented on pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

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

   On https://ci-beam.apache.org/job/beam_PreCommit_Java_Phrase/5298/ we see failures
   
   [org.apache.beam.sdk.io.gcp.datastore.RampupThrottlingFnTest.testRampupThrottler](https://ci-beam.apache.org/job/beam_PreCommit_Java_Phrase/5298/testReport/junit/org.apache.beam.sdk.io.gcp.datastore/RampupThrottlingFnTest/testRampupThrottler/)
   [org.apache.beam.sdk.io.gcp.spanner.SpannerIOWriteExceptionHandlingTest.testExceptionHandlingForWriteGrouped[0: DEADLINE_EXCEEDED]](https://ci-beam.apache.org/job/beam_PreCommit_Java_Phrase/5298/testReport/junit/org.apache.beam.sdk.io.gcp.spanner/SpannerIOWriteExceptionHandlingTest/testExceptionHandlingForWriteGrouped_0__DEADLINE_EXCEEDED_/)
   


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] kennknowles commented on a diff in pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

Posted by GitBox <gi...@apache.org>.
kennknowles commented on code in PR #17819:
URL: https://github.com/apache/beam/pull/17819#discussion_r951882036


##########
sdks/java/core/src/main/java/org/apache/beam/sdk/util/Preconditions.java:
##########
@@ -472,4 +472,63 @@ public class Preconditions {
     }
     return reference;
   }
+
+  /**
+   * Ensures that a piece of state passed as a parameter to the calling method is not null.
+   *
+   * @param obj an object reference
+   * @param errorMessageTemplate a template for the exception message should the check fail. The
+   *     message is formed by replacing each {@code %s} placeholder in the template with an
+   *     argument. These are matched by position - the first {@code %s} gets {@code
+   *     errorMessageArgs[0]}, etc. Unmatched arguments will be appended to the formatted message in
+   *     square braces. Unmatched placeholders will be left as-is.
+   * @param errorMessageArgs the arguments to be substituted into the message template. Arguments
+   *     are converted to strings using {@link String#valueOf(Object)}.
+   * @return the non-null reference that was validated
+   * @throws IllegalStateException if {@code obj} is null
+   */
+  @CanIgnoreReturnValue
+  @EnsuresNonNull("#1")
+  public static <T extends @NonNull Object> T checkStateNotNull(
+      @Nullable T obj,
+      @Nullable String errorMessageTemplate,
+      @Nullable Object... errorMessageArgs) {
+    if (obj == null) {
+      throw new IllegalArgumentException(lenientFormat(errorMessageTemplate, errorMessageArgs));
+    }
+    return obj;
+  }
+
+  /**
+   * Ensures that a piece of state is not null.
+   *
+   * <p>See {@link #checkStateNotNull(Object, String, Object...)} for details.
+   */
+  @CanIgnoreReturnValue
+  @EnsuresNonNull("#1")
+  public static <T extends @NonNull Object> T checkStateNotNull(
+      @Nullable T obj, @Nullable String errorMessageTemplate, @Nullable Object p1) {
+    if (obj == null) {
+      throw new IllegalStateException(lenientFormat(errorMessageTemplate, p1));
+    }
+    return obj;
+  }
+
+  /**
+   * Ensures that an object reference passed as a parameter to the calling method is not null.
+   *
+   * <p>See {@link #checkArgumentNotNull(Object, String, Object...)} for details.
+   */
+  @CanIgnoreReturnValue
+  @EnsuresNonNull("#1")
+  public static <T extends @NonNull Object> T checkStateNotNull(
+      @Nullable T obj,
+      @Nullable String errorMessageTemplate,
+      @Nullable Object p1,
+      @Nullable Object p2) {
+    if (obj == null) {
+      throw new IllegalStateException(lenientFormat(errorMessageTemplate, p1));

Review Comment:
   Fixed



##########
sdks/java/core/src/main/java/org/apache/beam/sdk/values/TypeDescriptor.java:
##########
@@ -180,7 +178,7 @@ public Class<? super T> getRawType() {
 
   /** Returns the component type if this type is an array type, otherwise returns {@code null}. */

Review Comment:
   Fixed



-- 
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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] robertwb commented on pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

Posted by GitBox <gi...@apache.org>.
robertwb commented on PR #17819:
URL: https://github.com/apache/beam/pull/17819#issuecomment-1185849034

   Any update on this PR?


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] kennknowles commented on pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

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

   amusingly, NPEs in elasticsearch test, plus flaking in pulsar test and file io


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] kennknowles commented on pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

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

   
   
   [Test Result](https://ci-beam.apache.org/job/beam_PreCommit_Java_Phrase/5299/testReport/) (1 failure / -1)
   [org.apache.beam.sdk.io.gcp.bigquery.BigQueryServicesImplTest.testInsertWithinRowCountLimits](https://ci-beam.apache.org/job/beam_PreCommit_Java_Phrase/5299/testReport/junit/org.apache.beam.sdk.io.gcp.bigquery/BigQueryServicesImplTest/testInsertWithinRowCountLimits/)


-- 
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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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

[GitHub] [beam] kennknowles commented on pull request #17819: [BEAM-10496] Eliminate some null errors from sdks/java/core

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

   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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

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