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/08/12 06:52:23 UTC
[GitHub] [beam] jayendra13 opened a new pull request #12540: [BEAM-10620] Eliminate nullability errors from :sdks:java:extensions:…
jayendra13 opened a new pull request #12540:
URL: https://github.com/apache/beam/pull/12540
…sql:perf-tests
Fixing nullability errors in :sdks:java:extensions:sql:perf-tests
------------------------
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`).
- [ ] 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.
- [ ] 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 | [](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
Java | [](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
Python | [](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
XLang | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](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.
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jayendra13 commented on a change in pull request #12540: [BEAM-10620] Eliminate nullability errors from :sdks:java:extensions:sql:perf-tests
Posted by GitBox <gi...@apache.org>.
jayendra13 commented on a change in pull request #12540:
URL: https://github.com/apache/beam/pull/12540#discussion_r487353928
##########
File path: sdks/java/io/common/src/test/java/org/apache/beam/sdk/io/common/IOTestPipelineOptions.java
##########
@@ -45,19 +45,16 @@
void setBigQueryTable(@Nullable String tableName);
@Description("InfluxDB measurement to publish results to.")
- @Nullable
String getInfluxMeasurement();
void setInfluxMeasurement(@Nullable String measurement);
@Description("InfluxDB host.")
- @Nullable
String getInfluxHost();
void setInfluxHost(@Nullable String host);
@Description("InfluxDB database.")
- @Nullable
Review comment:
I removed those nullable as it was expecting these fields non-null [here](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java#L31) via, [this](https://github.com/jayendra13/beam/blob/10620/sdks/java/extensions/sql/perf-tests/src/test/java/org/apache/beam/sdk/extensions/sql/meta/provider/bigquery/BigQueryIOPushDownIT.java#L114). I removed the `Nullable` from here thinking of it as a test code and [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as a non-test code.
Annotating the corresponding fields at [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as `Nullable` also should work.
Should I go with that?
----------------------------------------------------------------
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] jayendra13 commented on a change in pull request #12540: [BEAM-10620] Eliminate nullability errors from :sdks:java:extensions:sql:perf-tests
Posted by GitBox <gi...@apache.org>.
jayendra13 commented on a change in pull request #12540:
URL: https://github.com/apache/beam/pull/12540#discussion_r487353928
##########
File path: sdks/java/io/common/src/test/java/org/apache/beam/sdk/io/common/IOTestPipelineOptions.java
##########
@@ -45,19 +45,16 @@
void setBigQueryTable(@Nullable String tableName);
@Description("InfluxDB measurement to publish results to.")
- @Nullable
String getInfluxMeasurement();
void setInfluxMeasurement(@Nullable String measurement);
@Description("InfluxDB host.")
- @Nullable
String getInfluxHost();
void setInfluxHost(@Nullable String host);
@Description("InfluxDB database.")
- @Nullable
Review comment:
I removed those nullable as it was expecting these fields non-null [here](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java#L31) via, [this](https://github.com/jayendra13/beam/blob/10620/sdks/java/extensions/sql/perf-tests/src/test/java/org/apache/beam/sdk/extensions/sql/meta/provider/bigquery/BigQueryIOPushDownIT.java#L114). I removed the `Nullable` from here thinking of it as a test code and [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as a non-test code.
Annotating the corresponding fields at [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as `Nullable` also should work.
Should I go with that?
----------------------------------------------------------------
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 #12540: [BEAM-10620] Eliminate nullability errors from :sdks:java:extensions:sql:perf-tests
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #12540:
URL: https://github.com/apache/beam/pull/12540#discussion_r489808601
##########
File path: sdks/java/io/common/src/test/java/org/apache/beam/sdk/io/common/IOTestPipelineOptions.java
##########
@@ -45,19 +45,16 @@
void setBigQueryTable(@Nullable String tableName);
@Description("InfluxDB measurement to publish results to.")
- @Nullable
String getInfluxMeasurement();
void setInfluxMeasurement(@Nullable String measurement);
@Description("InfluxDB host.")
- @Nullable
String getInfluxHost();
void setInfluxHost(@Nullable String host);
@Description("InfluxDB database.")
- @Nullable
Review comment:
Yes, exactly. I wonder if we can alter the core `PipelineOptions` code that generates the proxy objects to ensure that the nullability matches... (if you want an extra fun follow up project...)
----------------------------------------------------------------
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] jayendra13 commented on a change in pull request #12540: [BEAM-10620] Eliminate nullability errors from :sdks:java:extensions:sql:perf-tests
Posted by GitBox <gi...@apache.org>.
jayendra13 commented on a change in pull request #12540:
URL: https://github.com/apache/beam/pull/12540#discussion_r488690404
##########
File path: sdks/java/io/common/src/test/java/org/apache/beam/sdk/io/common/IOTestPipelineOptions.java
##########
@@ -45,19 +45,16 @@
void setBigQueryTable(@Nullable String tableName);
@Description("InfluxDB measurement to publish results to.")
- @Nullable
String getInfluxMeasurement();
void setInfluxMeasurement(@Nullable String measurement);
@Description("InfluxDB host.")
- @Nullable
String getInfluxHost();
void setInfluxHost(@Nullable String host);
@Description("InfluxDB database.")
- @Nullable
Review comment:
So these three getters (for measurements, host and database) are fine, and we should also change these three setters to take `non-null` arguments. Correct me if I am wrong.
----------------------------------------------------------------
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 #12540: [BEAM-10620] Eliminate nullability errors from :sdks:java:extensions:sql:perf-tests
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #12540:
URL: https://github.com/apache/beam/pull/12540#discussion_r485882803
##########
File path: sdks/java/io/common/src/test/java/org/apache/beam/sdk/io/common/IOTestPipelineOptions.java
##########
@@ -45,19 +45,16 @@
void setBigQueryTable(@Nullable String tableName);
@Description("InfluxDB measurement to publish results to.")
- @Nullable
String getInfluxMeasurement();
void setInfluxMeasurement(@Nullable String measurement);
@Description("InfluxDB host.")
- @Nullable
String getInfluxHost();
void setInfluxHost(@Nullable String host);
@Description("InfluxDB database.")
- @Nullable
Review comment:
This change is curious. The `setInfluxDatabase` method accepts `null`, so this value should also be nullable. Should you change the setter as well? Or is there something else going on?
----------------------------------------------------------------
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] jayendra13 commented on a change in pull request #12540: [BEAM-10620] Eliminate nullability errors from :sdks:java:extensions:sql:perf-tests
Posted by GitBox <gi...@apache.org>.
jayendra13 commented on a change in pull request #12540:
URL: https://github.com/apache/beam/pull/12540#discussion_r487353928
##########
File path: sdks/java/io/common/src/test/java/org/apache/beam/sdk/io/common/IOTestPipelineOptions.java
##########
@@ -45,19 +45,16 @@
void setBigQueryTable(@Nullable String tableName);
@Description("InfluxDB measurement to publish results to.")
- @Nullable
String getInfluxMeasurement();
void setInfluxMeasurement(@Nullable String measurement);
@Description("InfluxDB host.")
- @Nullable
String getInfluxHost();
void setInfluxHost(@Nullable String host);
@Description("InfluxDB database.")
- @Nullable
Review comment:
I removed those nullable as it was expecting these fields non-null [here](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java#L31) via, [this](https://github.com/jayendra13/beam/blob/10620/sdks/java/extensions/sql/perf-tests/src/test/java/org/apache/beam/sdk/extensions/sql/meta/provider/bigquery/BigQueryIOPushDownIT.java#L114). I removed the `Nullable` from here thinking of it as a test code and [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as a non-test code.
Annotating the corresponding fields at [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as `Nullable` also should work.
Should I go with that?
##########
File path: sdks/java/io/common/src/test/java/org/apache/beam/sdk/io/common/IOTestPipelineOptions.java
##########
@@ -45,19 +45,16 @@
void setBigQueryTable(@Nullable String tableName);
@Description("InfluxDB measurement to publish results to.")
- @Nullable
String getInfluxMeasurement();
void setInfluxMeasurement(@Nullable String measurement);
@Description("InfluxDB host.")
- @Nullable
String getInfluxHost();
void setInfluxHost(@Nullable String host);
@Description("InfluxDB database.")
- @Nullable
Review comment:
I removed those nullable as it was expecting these fields non-null [here](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java#L31) via, [this](https://github.com/jayendra13/beam/blob/10620/sdks/java/extensions/sql/perf-tests/src/test/java/org/apache/beam/sdk/extensions/sql/meta/provider/bigquery/BigQueryIOPushDownIT.java#L114). I removed the `Nullable` from here thinking of it as a test code and [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as a non-test code.
Annotating the corresponding fields at [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as `Nullable` also should work.
Should I go with that?
##########
File path: sdks/java/io/common/src/test/java/org/apache/beam/sdk/io/common/IOTestPipelineOptions.java
##########
@@ -45,19 +45,16 @@
void setBigQueryTable(@Nullable String tableName);
@Description("InfluxDB measurement to publish results to.")
- @Nullable
String getInfluxMeasurement();
void setInfluxMeasurement(@Nullable String measurement);
@Description("InfluxDB host.")
- @Nullable
String getInfluxHost();
void setInfluxHost(@Nullable String host);
@Description("InfluxDB database.")
- @Nullable
Review comment:
I removed those nullable as it was expecting these fields non-null [here](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java#L31) via, [this](https://github.com/jayendra13/beam/blob/10620/sdks/java/extensions/sql/perf-tests/src/test/java/org/apache/beam/sdk/extensions/sql/meta/provider/bigquery/BigQueryIOPushDownIT.java#L114). I removed the `Nullable` from here thinking of it as a test code and [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as a non-test code.
Annotating the corresponding fields at [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as `Nullable` also should work.
Should I go with that?
##########
File path: sdks/java/io/common/src/test/java/org/apache/beam/sdk/io/common/IOTestPipelineOptions.java
##########
@@ -45,19 +45,16 @@
void setBigQueryTable(@Nullable String tableName);
@Description("InfluxDB measurement to publish results to.")
- @Nullable
String getInfluxMeasurement();
void setInfluxMeasurement(@Nullable String measurement);
@Description("InfluxDB host.")
- @Nullable
String getInfluxHost();
void setInfluxHost(@Nullable String host);
@Description("InfluxDB database.")
- @Nullable
Review comment:
I removed those nullable as it was expecting these fields non-null [here](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java#L31) via, [this](https://github.com/jayendra13/beam/blob/10620/sdks/java/extensions/sql/perf-tests/src/test/java/org/apache/beam/sdk/extensions/sql/meta/provider/bigquery/BigQueryIOPushDownIT.java#L114). I removed the `Nullable` from here thinking of it as a test code and [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as a non-test code.
Annotating the corresponding fields at [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as `Nullable` also should work.
Should I go with that?
##########
File path: sdks/java/io/common/src/test/java/org/apache/beam/sdk/io/common/IOTestPipelineOptions.java
##########
@@ -45,19 +45,16 @@
void setBigQueryTable(@Nullable String tableName);
@Description("InfluxDB measurement to publish results to.")
- @Nullable
String getInfluxMeasurement();
void setInfluxMeasurement(@Nullable String measurement);
@Description("InfluxDB host.")
- @Nullable
String getInfluxHost();
void setInfluxHost(@Nullable String host);
@Description("InfluxDB database.")
- @Nullable
Review comment:
I removed those nullable as it was expecting these fields non-null [here](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java#L31) via, [this](https://github.com/jayendra13/beam/blob/10620/sdks/java/extensions/sql/perf-tests/src/test/java/org/apache/beam/sdk/extensions/sql/meta/provider/bigquery/BigQueryIOPushDownIT.java#L114). I removed the `Nullable` from here thinking of it as a test code and [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as a non-test code.
Annotating the corresponding fields at [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as `Nullable` also should work.
Should I go with that?
##########
File path: sdks/java/io/common/src/test/java/org/apache/beam/sdk/io/common/IOTestPipelineOptions.java
##########
@@ -45,19 +45,16 @@
void setBigQueryTable(@Nullable String tableName);
@Description("InfluxDB measurement to publish results to.")
- @Nullable
String getInfluxMeasurement();
void setInfluxMeasurement(@Nullable String measurement);
@Description("InfluxDB host.")
- @Nullable
String getInfluxHost();
void setInfluxHost(@Nullable String host);
@Description("InfluxDB database.")
- @Nullable
Review comment:
I removed those nullable as it was expecting these fields non-null [here](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java#L31) via, [this](https://github.com/jayendra13/beam/blob/10620/sdks/java/extensions/sql/perf-tests/src/test/java/org/apache/beam/sdk/extensions/sql/meta/provider/bigquery/BigQueryIOPushDownIT.java#L114). I removed the `Nullable` from here thinking of it as a test code and [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as a non-test code.
Annotating the corresponding fields at [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as `Nullable` also should work.
Should I go with that?
##########
File path: sdks/java/io/common/src/test/java/org/apache/beam/sdk/io/common/IOTestPipelineOptions.java
##########
@@ -45,19 +45,16 @@
void setBigQueryTable(@Nullable String tableName);
@Description("InfluxDB measurement to publish results to.")
- @Nullable
String getInfluxMeasurement();
void setInfluxMeasurement(@Nullable String measurement);
@Description("InfluxDB host.")
- @Nullable
String getInfluxHost();
void setInfluxHost(@Nullable String host);
@Description("InfluxDB database.")
- @Nullable
Review comment:
I removed those nullable as it was expecting these fields non-null [here](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java#L31) via, [this](https://github.com/jayendra13/beam/blob/10620/sdks/java/extensions/sql/perf-tests/src/test/java/org/apache/beam/sdk/extensions/sql/meta/provider/bigquery/BigQueryIOPushDownIT.java#L114). I removed the `Nullable` from here thinking of it as a test code and [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as a non-test code.
Annotating the corresponding fields at [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as `Nullable` also should work.
Should I go with that?
----------------------------------------------------------------
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] jayendra13 commented on pull request #12540: [BEAM-10620] Eliminate nullability errors from :sdks:java:extensions:…
Posted by GitBox <gi...@apache.org>.
jayendra13 commented on pull request #12540:
URL: https://github.com/apache/beam/pull/12540#issuecomment-674606540
R: @kennknowles
----------------------------------------------------------------
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] jayendra13 commented on pull request #12540: [BEAM-10620] Eliminate nullability errors from :sdks:java:extensions:…
Posted by GitBox <gi...@apache.org>.
jayendra13 commented on pull request #12540:
URL: https://github.com/apache/beam/pull/12540#issuecomment-672727690
Run Java_Examples_Dataflow_Java11 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] jayendra13 commented on pull request #12540: [BEAM-10620] Eliminate nullability errors from :sdks:java:extensions:sql:perf-tests
Posted by GitBox <gi...@apache.org>.
jayendra13 commented on pull request #12540:
URL: https://github.com/apache/beam/pull/12540#issuecomment-685256262
@kennknowles Should I tag someone else ?
----------------------------------------------------------------
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 #12540: [BEAM-10620] Eliminate nullability errors from :sdks:java:extensions:sql:perf-tests
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #12540:
URL: https://github.com/apache/beam/pull/12540#issuecomment-689784631
Apologies - I was offline for a week and just coming back and catching up.
----------------------------------------------------------------
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 #12540: [BEAM-10620] Eliminate nullability errors from :sdks:java:extensions:sql:perf-tests
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #12540:
URL: https://github.com/apache/beam/pull/12540#discussion_r488104689
##########
File path: sdks/java/io/common/src/test/java/org/apache/beam/sdk/io/common/IOTestPipelineOptions.java
##########
@@ -45,19 +45,16 @@
void setBigQueryTable(@Nullable String tableName);
@Description("InfluxDB measurement to publish results to.")
- @Nullable
String getInfluxMeasurement();
void setInfluxMeasurement(@Nullable String measurement);
@Description("InfluxDB host.")
- @Nullable
String getInfluxHost();
void setInfluxHost(@Nullable String host);
@Description("InfluxDB database.")
- @Nullable
Review comment:
Ah that makes sense. What you should do, I think, is alter `setInfluxDatabase` so the parameter is non-nullable. Otherwise somehow the pipeline options would be magically turning `null` into non-`null`.
----------------------------------------------------------------
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] jayendra13 commented on a change in pull request #12540: [BEAM-10620] Eliminate nullability errors from :sdks:java:extensions:sql:perf-tests
Posted by GitBox <gi...@apache.org>.
jayendra13 commented on a change in pull request #12540:
URL: https://github.com/apache/beam/pull/12540#discussion_r487353928
##########
File path: sdks/java/io/common/src/test/java/org/apache/beam/sdk/io/common/IOTestPipelineOptions.java
##########
@@ -45,19 +45,16 @@
void setBigQueryTable(@Nullable String tableName);
@Description("InfluxDB measurement to publish results to.")
- @Nullable
String getInfluxMeasurement();
void setInfluxMeasurement(@Nullable String measurement);
@Description("InfluxDB host.")
- @Nullable
String getInfluxHost();
void setInfluxHost(@Nullable String host);
@Description("InfluxDB database.")
- @Nullable
Review comment:
I removed those nullable as it was expecting these fields non-null [here](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java#L31) via, [this](https://github.com/jayendra13/beam/blob/10620/sdks/java/extensions/sql/perf-tests/src/test/java/org/apache/beam/sdk/extensions/sql/meta/provider/bigquery/BigQueryIOPushDownIT.java#L114). I removed the `Nullable` from here thinking of it as a test code and [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as a non-test code.
Annotating the corresponding fields at [InfluxDBSettings.java](https://github.com/jayendra13/beam/blob/10620/sdks/java/testing/test-utils/src/main/java/org/apache/beam/sdk/testutils/publishing/InfluxDBSettings.java) as `Nullable` also should work.
Should I go with that?
----------------------------------------------------------------
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 #12540: [BEAM-10620] Eliminate nullability errors from :sdks:java:extensions:sql:perf-tests
Posted by GitBox <gi...@apache.org>.
kennknowles merged pull request #12540:
URL: https://github.com/apache/beam/pull/12540
----------------------------------------------------------------
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