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 2021/02/24 18:34:35 UTC
[GitHub] [beam] kennknowles opened a new pull request #14072: Merge Fn API and runner v2 configurations for DataflowRunner
kennknowles opened a new pull request #14072:
URL: https://github.com/apache/beam/pull/14072
Two independent commits:
1. Make the DataflowRunner detect any runner v2 or beam_fn_api flag and treat them all the same.
2. Remove the Dataflow-specific flags from the core SDK, leaving just the runner-independent `beam_fn_api` flag.
------------------------
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).
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_VR_Dataflow_V2/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://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
Python | [](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_Python_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python_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_Dataflow/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 | Whitespace | Typescript
--- | --- | --- | --- | --- | --- | ---
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/) <br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Typescript_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)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #14072:
URL: https://github.com/apache/beam/pull/14072#issuecomment-786045554
Run Java_Examples_Dataflow 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] boyuanzz commented on a change in pull request #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
boyuanzz commented on a change in pull request #14072:
URL: https://github.com/apache/beam/pull/14072#discussion_r582539861
##########
File path: runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java
##########
@@ -978,12 +988,8 @@ public DataflowPipelineJob run(Pipeline pipeline) {
dataflowOptions.setPipelineUrl(stagedPipeline.getLocation());
if (!isNullOrEmpty(dataflowOptions.getDataflowWorkerJar())) {
Review comment:
I think we should only stage dataflow worker jar when `! useUnifiedWorker ()`. I can have a cleanup PR for that after this one is in.
----------------------------------------------------------------
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 #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #14072:
URL: https://github.com/apache/beam/pull/14072#issuecomment-785285512
I did not change the Dataflow worker because it still uses `beam_fn_api` flag to run as JRH. But the Java `DataflowRunner` should never start a JRH job, only Python. Correct?
----------------------------------------------------------------
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 #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
kennknowles merged pull request #14072:
URL: https://github.com/apache/beam/pull/14072
----------------------------------------------------------------
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] boyuanzz commented on a change in pull request #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
boyuanzz commented on a change in pull request #14072:
URL: https://github.com/apache/beam/pull/14072#discussion_r583229910
##########
File path: runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java
##########
@@ -915,6 +915,16 @@ private Debuggee registerDebuggee(CloudDebugger debuggerClient, String uniquifie
@Override
public DataflowPipelineJob run(Pipeline pipeline) {
+ Set<String> experiments =
Review comment:
One thing we can do is to always set experiments back to pipeline options when we change something 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 #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #14072:
URL: https://github.com/apache/beam/pull/14072#issuecomment-786280236
I removed the setting of experiments and it still fails. That isolates the change to using the `Set`.
----------------------------------------------------------------
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 #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #14072:
URL: https://github.com/apache/beam/pull/14072#discussion_r583288915
##########
File path: runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java
##########
@@ -915,6 +915,21 @@ private Debuggee registerDebuggee(CloudDebugger debuggerClient, String uniquifie
@Override
public DataflowPipelineJob run(Pipeline pipeline) {
+
+ if (useUnifiedWorker(options)) {
Review comment:
I reverted every commit that was red, and then added just this. It still fails in the same way. That's interesting...
----------------------------------------------------------------
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] boyuanzz commented on a change in pull request #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
boyuanzz commented on a change in pull request #14072:
URL: https://github.com/apache/beam/pull/14072#discussion_r582537792
##########
File path: runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java
##########
@@ -2332,7 +2326,9 @@ static String getContainerImageForJob(DataflowPipelineOptions options) {
}
static boolean useUnifiedWorker(DataflowPipelineOptions options) {
- return hasExperiment(options, "use_runner_v2") || hasExperiment(options, "use_unified_worker");
+ return hasExperiment(options, "beam_fn_api")
+ || hasExperiment(options, "use_runner_v2")
Review comment:
Thanks! That looks 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
[GitHub] [beam] kennknowles commented on pull request #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #14072:
URL: https://github.com/apache/beam/pull/14072#issuecomment-786044958
Interestingly, the initial commit was green, but this change to activate all experiments seems to break examples on runner v1.
----------------------------------------------------------------
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 #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #14072:
URL: https://github.com/apache/beam/pull/14072#discussion_r583289942
##########
File path: runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java
##########
@@ -915,6 +915,21 @@ private Debuggee registerDebuggee(CloudDebugger debuggerClient, String uniquifie
@Override
public DataflowPipelineJob run(Pipeline pipeline) {
+
+ if (useUnifiedWorker(options)) {
Review comment:
Nevermind, I was looking at stale results. It succeeds. Since the failures were in branches for `!useUnifiedWorker` this should not even execute. So the failure really had _something_ to do with it being a `Set` or how the variable was carried through the codepaths. Setting it back right away like you suggested would probably also have fixed 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] boyuanzz commented on pull request #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
boyuanzz commented on pull request #14072:
URL: https://github.com/apache/beam/pull/14072#issuecomment-786117725
Run Java_Examples_Dataflow 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 #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #14072:
URL: https://github.com/apache/beam/pull/14072#issuecomment-786052099
It definitely shows up as a strange bug in Dataflow service: https://pantheon.corp.google.com/dataflow/jobs/us-central1/2021-02-24_20_09_26-17288472525963235346;step=;expandBottomPanel=false?project=apache-beam-testing
- job "succeeded"
- every step red/failed
- no logs associated with steps
----------------------------------------------------------------
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 #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #14072:
URL: https://github.com/apache/beam/pull/14072#issuecomment-786055497
Actually that was just Jenkins parsing the wrong part of the log. But that job still looks bad.
The actual failure is https://pantheon.corp.google.com/dataflow/jobs/us-central1/2021-02-24_20_14_47-5573166312510156547;bottomTab=JOB_LOGS;expandBottomPanel=true;logsSeverity=ERROR?project=apache-beam-testing which matches the error message. The job could not be created at all. I think probably this is something trivial in the conversion of `experiments` to a set and back to a list.
----------------------------------------------------------------
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 #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #14072:
URL: https://github.com/apache/beam/pull/14072#discussion_r583289942
##########
File path: runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java
##########
@@ -915,6 +915,21 @@ private Debuggee registerDebuggee(CloudDebugger debuggerClient, String uniquifie
@Override
public DataflowPipelineJob run(Pipeline pipeline) {
+
+ if (useUnifiedWorker(options)) {
Review comment:
Nevermind, I was looking at stale results. It succeeds. Since the failures were in branches for `!useUnifiedWorker` this should not even execute. So the failure really had _something_ to do with it being a `Set`.
----------------------------------------------------------------
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] boyuanzz commented on pull request #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
boyuanzz commented on pull request #14072:
URL: https://github.com/apache/beam/pull/14072#issuecomment-786115388
Run Java examples on Dataflow Java 11
----------------------------------------------------------------
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] boyuanzz commented on pull request #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
boyuanzz commented on pull request #14072:
URL: https://github.com/apache/beam/pull/14072#issuecomment-786112978
The Java_Examples_Dataflow and Java_Examples_Dataflow_Java11 are tests for java legacy worker, which should not go through setting all runner_v2 flags code path.
----------------------------------------------------------------
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 #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #14072:
URL: https://github.com/apache/beam/pull/14072#issuecomment-786317853
All the Jenkins are green. GitHub actions are getting queued.
----------------------------------------------------------------
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 #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #14072:
URL: https://github.com/apache/beam/pull/14072#issuecomment-786085537
Experiments look good. So it is a problem somehow with setting all of them.
----------------------------------------------------------------
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] boyuanzz commented on pull request #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
boyuanzz commented on pull request #14072:
URL: https://github.com/apache/beam/pull/14072#issuecomment-786115213
Run Java Examples on Dataflow Runner V2
----------------------------------------------------------------
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] boyuanzz commented on a change in pull request #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
boyuanzz commented on a change in pull request #14072:
URL: https://github.com/apache/beam/pull/14072#discussion_r582520260
##########
File path: runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java
##########
@@ -2332,7 +2326,9 @@ static String getContainerImageForJob(DataflowPipelineOptions options) {
}
static boolean useUnifiedWorker(DataflowPipelineOptions options) {
- return hasExperiment(options, "use_runner_v2") || hasExperiment(options, "use_unified_worker");
+ return hasExperiment(options, "beam_fn_api")
+ || hasExperiment(options, "use_runner_v2")
Review comment:
IIRC, if we only specify `beam_fn_api`, we will go with JRH unless we have added runner v2 variants automatically somewhere. So maybe we should add runner_v2 variants as long as `beam_fn_api` is there before these checks.
----------------------------------------------------------------
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] boyuanzz commented on pull request #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
boyuanzz commented on pull request #14072:
URL: https://github.com/apache/beam/pull/14072#issuecomment-786130025
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] kennknowles commented on pull request #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #14072:
URL: https://github.com/apache/beam/pull/14072#issuecomment-785412402
OK looks like the second commit causes issues with ISM tests. Don't know if it causes issues with actual ISM in practice. I will drop it and start with the first easy commit.
----------------------------------------------------------------
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 #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #14072:
URL: https://github.com/apache/beam/pull/14072#discussion_r582529794
##########
File path: runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java
##########
@@ -2332,7 +2326,9 @@ static String getContainerImageForJob(DataflowPipelineOptions options) {
}
static boolean useUnifiedWorker(DataflowPipelineOptions options) {
- return hasExperiment(options, "use_runner_v2") || hasExperiment(options, "use_unified_worker");
+ return hasExperiment(options, "beam_fn_api")
+ || hasExperiment(options, "use_runner_v2")
Review comment:
Good point. So now I check for any of the experiments and set them all if any of them are set.
----------------------------------------------------------------
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 #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #14072:
URL: https://github.com/apache/beam/pull/14072#issuecomment-786050777
Run Java Examples on Dataflow Runner V2
----------------------------------------------------------------
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] boyuanzz commented on a change in pull request #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
boyuanzz commented on a change in pull request #14072:
URL: https://github.com/apache/beam/pull/14072#discussion_r582520260
##########
File path: runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java
##########
@@ -2332,7 +2326,9 @@ static String getContainerImageForJob(DataflowPipelineOptions options) {
}
static boolean useUnifiedWorker(DataflowPipelineOptions options) {
- return hasExperiment(options, "use_runner_v2") || hasExperiment(options, "use_unified_worker");
+ return hasExperiment(options, "beam_fn_api")
+ || hasExperiment(options, "use_runner_v2")
Review comment:
IIRC, if we only specify `beam_fn_api`, we will go with JRH unless we have added runner v2 variants automatically somewhere.
----------------------------------------------------------------
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 #14072: Merge Fn API and runner v2 configurations for DataflowRunner
Posted by GitBox <gi...@apache.org>.
kennknowles commented on pull request #14072:
URL: https://github.com/apache/beam/pull/14072#issuecomment-786240810
Yes it is pretty mysterious. I was thinking one of these:
- The value in `experiments` field was corrupted by conversion to set and back.
- There is some runner v1 code path that checked `!beam_fn_api` or `!use_unified_worker` and I have messed it up.
So far I have not found the problem.
----------------------------------------------------------------
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