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/04/08 22:35:18 UTC

[GitHub] [beam] y1chi opened a new pull request #11355: [BEAM-9727] Automatically set required experiment flags for dataflow …

y1chi opened a new pull request #11355: [BEAM-9727] Automatically set required experiment flags for dataflow …
URL: https://github.com/apache/beam/pull/11355
 
 
   …runner v2.
   
   **Please** add a meaningful description for your change here
   
   ------------------------
   
   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 | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
   Java | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
   Python | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
   XLang | --- | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/)
   
   Pre-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   --- |Java | Python | Go | Website
   --- | --- | --- | --- | ---
   Non-portable | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/) 
   Portable | --- | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon)](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 #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.

Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.
URL: https://github.com/apache/beam/pull/11355#issuecomment-611731056
 
 
   retest this please

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] lukecwik commented on issue #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.

Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.
URL: https://github.com/apache/beam/pull/11355#issuecomment-611692494
 
 
   It doens't look like Ankur's comments have been addressed.

----------------------------------------------------------------
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] angoenka commented on a change in pull request #11355: [BEAM-9727] Automatically set required experiment flags for dataflow …

Posted by GitBox <gi...@apache.org>.
angoenka commented on a change in pull request #11355: [BEAM-9727] Automatically set required experiment flags for dataflow …
URL: https://github.com/apache/beam/pull/11355#discussion_r405880080
 
 

 ##########
 File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
 ##########
 @@ -566,6 +572,17 @@ def run_pipeline(self, pipeline, options):
     result.metric_results = self._metrics
     return result
 
+  def _maybe_add_unified_worker_missing_options(self, options):
+    # set default beam_fn_api and use_unified_worker experiment if
+    # 'use_runner_v2' experiment flag exists, no-op otherwise.
+    experiments = options.view_as(DebugOptions).experiments or []
+    if 'use_runner_v2' in experiments:
+      if not 'beam_fn_api' in experiments:
 
 Review comment:
   @pabloem Do we need to add BQ option here as well?

----------------------------------------------------------------
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] y1chi commented on issue #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.

Posted by GitBox <gi...@apache.org>.
y1chi commented on issue #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.
URL: https://github.com/apache/beam/pull/11355#issuecomment-611764830
 
 
   > Consider adding the logic here instead:
   > https://github.com/apache/beam/blob/79b2d87b59819ee55fb8600e8a845c6ba5b98d64/sdks/python/apache_beam/pipeline.py#L206
   > 
   > This would add the experiment slightly earlier then when the first ptransform is applied.
   
   I initially considered to add it into pipeline.py following this check, but feel that the logic is too dataflow-specific to be added here.

----------------------------------------------------------------
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] y1chi commented on issue #11355: [BEAM-9727] Automatically set required experiment flags for dataflow …

Posted by GitBox <gi...@apache.org>.
y1chi commented on issue #11355: [BEAM-9727] Automatically set required experiment flags for dataflow …
URL: https://github.com/apache/beam/pull/11355#issuecomment-611229442
 
 
   R: @angoenka 

----------------------------------------------------------------
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] angoenka commented on a change in pull request #11355: [BEAM-9727] Automatically set required experiment flags for dataflow …

Posted by GitBox <gi...@apache.org>.
angoenka commented on a change in pull request #11355: [BEAM-9727] Automatically set required experiment flags for dataflow …
URL: https://github.com/apache/beam/pull/11355#discussion_r405879963
 
 

 ##########
 File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
 ##########
 @@ -566,6 +572,17 @@ def run_pipeline(self, pipeline, options):
     result.metric_results = self._metrics
     return result
 
+  def _maybe_add_unified_worker_missing_options(self, options):
+    # set default beam_fn_api and use_unified_worker experiment if
+    # 'use_runner_v2' experiment flag exists, no-op otherwise.
+    experiments = options.view_as(DebugOptions).experiments or []
 
 Review comment:
   Debug options have has convenience method for lookup and adding experiments. We can use those instead of raw 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


With regards,
Apache Git Services

[GitHub] [beam] pabloem commented on a change in pull request #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.

Posted by GitBox <gi...@apache.org>.
pabloem commented on a change in pull request #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.
URL: https://github.com/apache/beam/pull/11355#discussion_r406408569
 
 

 ##########
 File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
 ##########
 @@ -566,6 +572,17 @@ def run_pipeline(self, pipeline, options):
     result.metric_results = self._metrics
     return result
 
+  def _maybe_add_unified_worker_missing_options(self, options):
+    # set default beam_fn_api and use_unified_worker experiment if
+    # 'use_runner_v2' experiment flag exists, no-op otherwise.
+    experiments = options.view_as(DebugOptions).experiments or []
+    if 'use_runner_v2' in experiments:
+      if not 'beam_fn_api' in experiments:
 
 Review comment:
   I think it makes sense to add it. `'use_beam_bq_sink'`.

----------------------------------------------------------------
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] ananvay commented on a change in pull request #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.

Posted by GitBox <gi...@apache.org>.
ananvay commented on a change in pull request #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.
URL: https://github.com/apache/beam/pull/11355#discussion_r406430923
 
 

 ##########
 File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
 ##########
 @@ -566,6 +572,17 @@ def run_pipeline(self, pipeline, options):
     result.metric_results = self._metrics
     return result
 
+  def _maybe_add_unified_worker_missing_options(self, options):
+    # set default beam_fn_api and use_unified_worker experiment if
+    # 'use_runner_v2' experiment flag exists, no-op otherwise.
+    experiments = options.view_as(DebugOptions).experiments or []
+    if 'use_runner_v2' in experiments:
+      if not 'beam_fn_api' in experiments:
 
 Review comment:
   okay, let's do it at least for the bq_sink then, as Pablo commented earlier.

----------------------------------------------------------------
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 #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.

Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.
URL: https://github.com/apache/beam/pull/11355#issuecomment-611785115
 
 
   > > Consider adding the logic here instead:
   > > https://github.com/apache/beam/blob/79b2d87b59819ee55fb8600e8a845c6ba5b98d64/sdks/python/apache_beam/pipeline.py#L206
   > > 
   > > This would add the experiment slightly earlier then when the first ptransform is applied.
   > 
   > I initially considered to add it into pipeline.py following this check, but feel that the logic is too dataflow-specific to be added here.
   
   sgtm, in the worst case we can tell users to use a longer list of experiments if we find a problem later.

----------------------------------------------------------------
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 merged pull request #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.

Posted by GitBox <gi...@apache.org>.
lukecwik merged pull request #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.
URL: https://github.com/apache/beam/pull/11355
 
 
   

----------------------------------------------------------------
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] pabloem commented on a change in pull request #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.

Posted by GitBox <gi...@apache.org>.
pabloem commented on a change in pull request #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.
URL: https://github.com/apache/beam/pull/11355#discussion_r406423223
 
 

 ##########
 File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
 ##########
 @@ -566,6 +572,17 @@ def run_pipeline(self, pipeline, options):
     result.metric_results = self._metrics
     return result
 
+  def _maybe_add_unified_worker_missing_options(self, options):
+    # set default beam_fn_api and use_unified_worker experiment if
+    # 'use_runner_v2' experiment flag exists, no-op otherwise.
+    experiments = options.view_as(DebugOptions).experiments or []
+    if 'use_runner_v2' in experiments:
+      if not 'beam_fn_api' in experiments:
 
 Review comment:
   The switch for BQ source is a little trickier. I am working on a follow up PR for 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


With regards,
Apache Git Services

[GitHub] [beam] ananvay commented on issue #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.

Posted by GitBox <gi...@apache.org>.
ananvay commented on issue #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.
URL: https://github.com/apache/beam/pull/11355#issuecomment-611776464
 
 
   LGTM

----------------------------------------------------------------
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] angoenka commented on a change in pull request #11355: [BEAM-9727] Automatically set required experiment flags for dataflow …

Posted by GitBox <gi...@apache.org>.
angoenka commented on a change in pull request #11355: [BEAM-9727] Automatically set required experiment flags for dataflow …
URL: https://github.com/apache/beam/pull/11355#discussion_r405880602
 
 

 ##########
 File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
 ##########
 @@ -566,6 +572,17 @@ def run_pipeline(self, pipeline, options):
     result.metric_results = self._metrics
     return result
 
+  def _maybe_add_unified_worker_missing_options(self, options):
+    # set default beam_fn_api and use_unified_worker experiment if
+    # 'use_runner_v2' experiment flag exists, no-op otherwise.
+    experiments = options.view_as(DebugOptions).experiments or []
+    if 'use_runner_v2' in experiments:
 
 Review comment:
   We have a method apiclient.use_unified_worker. We can use that instead of checking it explicitly.
   Also, we can simplify/remove that method if we sanitize the options here.

----------------------------------------------------------------
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 #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.

Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.
URL: https://github.com/apache/beam/pull/11355#issuecomment-611754766
 
 
   retest this please

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] ananvay commented on a change in pull request #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.

Posted by GitBox <gi...@apache.org>.
ananvay commented on a change in pull request #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.
URL: https://github.com/apache/beam/pull/11355#discussion_r406409214
 
 

 ##########
 File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner.py
 ##########
 @@ -566,6 +572,17 @@ def run_pipeline(self, pipeline, options):
     result.metric_results = self._metrics
     return result
 
+  def _maybe_add_unified_worker_missing_options(self, options):
+    # set default beam_fn_api and use_unified_worker experiment if
+    # 'use_runner_v2' experiment flag exists, no-op otherwise.
+    experiments = options.view_as(DebugOptions).experiments or []
+    if 'use_runner_v2' in experiments:
+      if not 'beam_fn_api' in experiments:
 
 Review comment:
   can we do it for batch bq source as well? I don't think the UW can support the native BQ source...

----------------------------------------------------------------
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 #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.

Posted by GitBox <gi...@apache.org>.
lukecwik commented on issue #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.
URL: https://github.com/apache/beam/pull/11355#issuecomment-611730889
 
 
   trigger tests

----------------------------------------------------------------
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] ananvay commented on issue #11355: [BEAM-9727] Automatically set required experiment flags for dataflow …

Posted by GitBox <gi...@apache.org>.
ananvay commented on issue #11355: [BEAM-9727] Automatically set required experiment flags for dataflow …
URL: https://github.com/apache/beam/pull/11355#issuecomment-611690586
 
 
   R @lukecwik @robertwb 

----------------------------------------------------------------
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 #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.

Posted by GitBox <gi...@apache.org>.
lukecwik commented on a change in pull request #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.
URL: https://github.com/apache/beam/pull/11355#discussion_r406455862
 
 

 ##########
 File path: sdks/python/apache_beam/runners/dataflow/internal/apiclient.py
 ##########
 @@ -1014,16 +1014,13 @@ def _use_fnapi(pipeline_options):
 
 
 def _use_unified_worker(pipeline_options):
-  if not _use_fnapi(pipeline_options):
-    return False
   debug_options = pipeline_options.view_as(DebugOptions)
   use_unified_worker_flag = 'use_unified_worker'
+  use_runner_v2_flag = 'use_runner_v2'
 
-  if debug_options.lookup_experiment(use_unified_worker_flag):
-    return debug_options.lookup_experiment(use_unified_worker_flag)
-
-  if debug_options.lookup_experiment('use_runner_v2'):
-    debug_options.add_experiment(use_unified_worker_flag)
+  if (debug_options.lookup_experiment(use_runner_v2_flag) and not
+  debug_options.lookup_experiment(use_unified_worker_flag)):
 
 Review comment:
   formatting issue

----------------------------------------------------------------
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] y1chi commented on issue #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.

Posted by GitBox <gi...@apache.org>.
y1chi commented on issue #11355: [BEAM-9727] Automatically set required experiment flags for dataflow runner v2.
URL: https://github.com/apache/beam/pull/11355#issuecomment-611705515
 
 
   > Thanks a lot Yichi! Could we also ensure this change works with the default bq_sink flag in #11309? Basically, we should be able to run a batch python pipeline with BQ sink with just --use_runner_v2 option. Ideally, with your current change and the one in PR-11309, we should be able to run it e2e w/o any extra flags.
   
   I've validated with an example pipeline using BigQuery sink, and asserted on the beam fn api flag in apply_WriteToBigQuery where the flag is 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] ananvay commented on issue #11355: [BEAM-9727] Automatically set required experiment flags for dataflow …

Posted by GitBox <gi...@apache.org>.
ananvay commented on issue #11355: [BEAM-9727] Automatically set required experiment flags for dataflow …
URL: https://github.com/apache/beam/pull/11355#issuecomment-611684738
 
 
   Thanks a lot Yichi! Could we also ensure this change works with the default bq_sink flag in https://github.com/apache/beam/pull/11309? Basically, we should be able to run a batch python pipeline with BQ sink with just --use_runner_v2 option. Ideally, with your current change and the one in PR-11309, we should be able to run it e2e w/o any extra flags.

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