You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@beam.apache.org by GitBox <gi...@apache.org> on 2020/03/18 23:22:09 UTC
[GitHub] [beam] robertwb opened a new pull request #11165: [BEAM-9340]
Populate requirements for Java.
robertwb opened a new pull request #11165: [BEAM-9340] Populate requirements for Java.
URL: https://github.com/apache/beam/pull/11165
------------------------
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 | Apex | Dataflow | Flink | Gearpump | Samza | Spark
--- | --- | --- | --- | --- | --- | --- | ---
Go | [](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
Java | [](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
Python | [](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | --- | [](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
XLang | --- | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/)
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](https://builds.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
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11165: [BEAM-9340] Populate
requirements for Java.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11165: [BEAM-9340] Populate requirements for Java.
URL: https://github.com/apache/beam/pull/11165#issuecomment-602707990
It would be good to extend [PipelineValidator](https://github.com/apache/beam/blob/33845727ec635eb07f95db12809e6ce217af8b33/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/graph/PipelineValidator.java#L42) to make sure that SDKs are submitting their jobs correctly including the set of requirements.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on a change in pull request #11165:
[BEAM-9340] Populate requirements for Java.
Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #11165: [BEAM-9340] Populate requirements for Java.
URL: https://github.com/apache/beam/pull/11165#discussion_r395845584
##########
File path: runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/PTransformTranslation.java
##########
@@ -124,6 +124,15 @@
public static final String SPLITTABLE_PROCESS_SIZED_ELEMENTS_AND_RESTRICTIONS_URN =
"beam:transform:sdf_process_sized_element_and_restrictions:v1";
+ public static final String REQUIRES_STATEFUL_PROCESSING_URN =
+ getUrn(RunnerApi.StandardRequirements.Enum.REQUIRES_STATEFUL_PROCESSING);
Review comment:
That's unfortunate, but I see the pattern. Have to be vigilant to prevent bugs. (Unlikely that these'll be used in switch statements, but consistency is good.)
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on a change in pull request #11165:
[BEAM-9340] Populate requirements for Java.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #11165: [BEAM-9340] Populate requirements for Java.
URL: https://github.com/apache/beam/pull/11165#discussion_r395306659
##########
File path: runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/PTransformTranslation.java
##########
@@ -124,6 +124,15 @@
public static final String SPLITTABLE_PROCESS_SIZED_ELEMENTS_AND_RESTRICTIONS_URN =
"beam:transform:sdf_process_sized_element_and_restrictions:v1";
+ public static final String REQUIRES_STATEFUL_PROCESSING_URN =
+ getUrn(RunnerApi.StandardRequirements.Enum.REQUIRES_STATEFUL_PROCESSING);
Review comment:
Please define string constants and then validate that they match in the static initializer. This allows for the string constants to be used in switch/case statements since they are compile time constants.
I have pr/11162 which duplicates this effort, which ever goes in second needs to get updated.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on issue #11165: [BEAM-9340] Populate
requirements for Java.
Posted by GitBox <gi...@apache.org>.
robertwb commented on issue #11165: [BEAM-9340] Populate requirements for Java.
URL: https://github.com/apache/beam/pull/11165#issuecomment-603527045
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on issue #11165: [BEAM-9340] Populate
requirements for Java.
Posted by GitBox <gi...@apache.org>.
robertwb commented on issue #11165: [BEAM-9340] Populate requirements for Java.
URL: https://github.com/apache/beam/pull/11165#issuecomment-601872470
I rebased on top of your 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on issue #11165: [BEAM-9340] Populate
requirements for Java.
Posted by GitBox <gi...@apache.org>.
robertwb commented on issue #11165: [BEAM-9340] Populate requirements for Java.
URL: https://github.com/apache/beam/pull/11165#issuecomment-603527163
Good point about the validator. I'll do that in a follow-up 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on issue #11165: [BEAM-9340] Populate
requirements for Java.
Posted by GitBox <gi...@apache.org>.
robertwb commented on issue #11165: [BEAM-9340] Populate requirements for Java.
URL: https://github.com/apache/beam/pull/11165#issuecomment-603962869
Now org.apache.beam.sdk.io.cassandra.CassandraIOTest.classMethod is failing. These are unrelated and didn't fail before. Merging.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on issue #11165: [BEAM-9340] Populate
requirements for Java.
Posted by GitBox <gi...@apache.org>.
robertwb commented on issue #11165: [BEAM-9340] Populate requirements for Java.
URL: https://github.com/apache/beam/pull/11165#issuecomment-601947065
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb merged pull request #11165: [BEAM-9340] Populate
requirements for Java.
Posted by GitBox <gi...@apache.org>.
robertwb merged pull request #11165: [BEAM-9340] Populate requirements for Java.
URL: https://github.com/apache/beam/pull/11165
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on issue #11165: [BEAM-9340] Populate
requirements for Java.
Posted by GitBox <gi...@apache.org>.
robertwb commented on issue #11165: [BEAM-9340] Populate requirements for Java.
URL: https://github.com/apache/beam/pull/11165#issuecomment-601946329
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
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on a change in pull request #11165:
[BEAM-9340] Populate requirements for Java.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #11165: [BEAM-9340] Populate requirements for Java.
URL: https://github.com/apache/beam/pull/11165#discussion_r395308799
##########
File path: runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/EnvironmentsTest.java
##########
@@ -115,7 +115,7 @@ public void getEnvironmentPTransform() throws IOException {
ParDo.of(
new DoFn<String, String>() {
@ProcessElement
- public void process(ProcessContext ctxt) {}
+ public void process(ProcessContext ctxt) {}CreateExecutableStageNodeFunction.java
Review comment:
```suggestion
public void process(ProcessContext ctxt) {}
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on issue #11165: [BEAM-9340] Populate
requirements for Java.
Posted by GitBox <gi...@apache.org>.
robertwb commented on issue #11165: [BEAM-9340] Populate requirements for Java.
URL: https://github.com/apache/beam/pull/11165#issuecomment-601946285
This passes locally. org.apache.beam.sdk.transforms.ParDoLifecycleTest.testTeardownCalledAfterExceptionInFinishBundleStateful seems unrelated. Retrying again.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on a change in pull request #11165:
[BEAM-9340] Populate requirements for Java.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #11165: [BEAM-9340] Populate requirements for Java.
URL: https://github.com/apache/beam/pull/11165#discussion_r395310246
##########
File path: runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/SdkComponents.java
##########
@@ -86,25 +91,30 @@ public static SdkComponents create(RunnerApi.Components components) {
}
public static SdkComponents create(PipelineOptions options) {
- SdkComponents sdkComponents = new SdkComponents(RunnerApi.Components.getDefaultInstance(), "");
+ SdkComponents sdkComponents =
+ new SdkComponents(RunnerApi.Components.getDefaultInstance(), null, "");
PortablePipelineOptions portablePipelineOptions = options.as(PortablePipelineOptions.class);
sdkComponents.registerEnvironment(
Environments.createOrGetDefaultEnvironment(portablePipelineOptions));
return sdkComponents;
}
- private SdkComponents(RunnerApi.Components components, String newIdPrefix) {
+ private SdkComponents(
+ Components components, Collection<String> requirements, String newIdPrefix) {
this.newIdPrefix = newIdPrefix;
+ this.requirements = new HashSet<>();
if (components == null) {
- return;
+ if (requirements != null) {
+ this.requirements.addAll(requirements);
+ }
+ } else {
+ mergeFrom(components, requirements);
}
-
- mergeFrom(components);
}
/** Merge Components proto into this SdkComponents instance. */
- public void mergeFrom(RunnerApi.Components components) {
+ public void mergeFrom(RunnerApi.Components components, Collection<String> requirements) {
Review comment:
```suggestion
public void mergeFrom(RunnerApi.Components components, @Nullable Collection<String> requirements) {
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on issue #11165: [BEAM-9340] Populate
requirements for Java.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11165: [BEAM-9340] Populate requirements for Java.
URL: https://github.com/apache/beam/pull/11165#issuecomment-602705842
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
With regards,
Apache Git Services
[GitHub] [beam] lukecwik edited a comment on issue #11165: [BEAM-9340]
Populate requirements for Java.
Posted by GitBox <gi...@apache.org>.
lukecwik edited a comment on issue #11165: [BEAM-9340] Populate requirements for Java.
URL: https://github.com/apache/beam/pull/11165#issuecomment-602707990
It would be good to extend [PipelineValidator](https://github.com/apache/beam/blob/33845727ec635eb07f95db12809e6ce217af8b33/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/graph/PipelineValidator.java#L42) to make sure that SDKs are submitting their jobs correctly including the set of requirements by inspecting the ParDoPayload.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on issue #11165: [BEAM-9340] Populate
requirements for Java.
Posted by GitBox <gi...@apache.org>.
robertwb commented on issue #11165: [BEAM-9340] Populate requirements for Java.
URL: https://github.com/apache/beam/pull/11165#issuecomment-601921071
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
With regards,
Apache Git Services
[GitHub] [beam] lukecwik commented on a change in pull request #11165:
[BEAM-9340] Populate requirements for Java.
Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #11165: [BEAM-9340] Populate requirements for Java.
URL: https://github.com/apache/beam/pull/11165#discussion_r395310152
##########
File path: runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/SdkComponents.java
##########
@@ -86,25 +91,30 @@ public static SdkComponents create(RunnerApi.Components components) {
}
public static SdkComponents create(PipelineOptions options) {
- SdkComponents sdkComponents = new SdkComponents(RunnerApi.Components.getDefaultInstance(), "");
+ SdkComponents sdkComponents =
+ new SdkComponents(RunnerApi.Components.getDefaultInstance(), null, "");
PortablePipelineOptions portablePipelineOptions = options.as(PortablePipelineOptions.class);
sdkComponents.registerEnvironment(
Environments.createOrGetDefaultEnvironment(portablePipelineOptions));
return sdkComponents;
}
- private SdkComponents(RunnerApi.Components components, String newIdPrefix) {
+ private SdkComponents(
+ Components components, Collection<String> requirements, String newIdPrefix) {
Review comment:
```suggestion
Components components, @Nullable Collection<String> requirements, String newIdPrefix) {
```
----------------------------------------------------------------
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
With regards,
Apache Git Services