chamikaramj commented on a change in pull request #14557:
URL: https://github.com/apache/beam/pull/14557#discussion_r616209786



##########
File path: sdks/python/apache_beam/runners/dataflow/dataflow_runner_test.py
##########
@@ -206,7 +206,7 @@ def test_create_runner(self):
 
   def test_environment_override_translation(self):
     self.default_properties.append('--experiments=beam_fn_api')
-    self.default_properties.append('--worker_harness_container_image=FOO')

Review comment:
       Probably add a new unit test for "sdk_container_image" instead to make 
sure "worker_harness_container_image" works as well (at least for runner v1).

##########
File path: sdks/python/apache_beam/options/pipeline_options.py
##########
@@ -859,13 +859,14 @@ def _add_argparse_args(cls, parser):
             'subnetwork name. For more information, see '
             'https://cloud.google.com/compute/docs/vpc/'))
     parser.add_argument(
+        '--sdk_container_image',
         '--worker_harness_container_image',
+        dest='sdk_container_image',
         default=None,
         help=(
             'Docker registry location of container image to use for the '
             'worker harness. Default is the container for the version of the '
-            'SDK. Note: currently, only approved Google Cloud Dataflow '
-            'container images may be used here.'))
+            'SDK. Note this field is only valid for portable pipelines.'))

Review comment:
       Probably documentation has to be further modified or 
"sdk_container_image" has to be made a separate option.
   
   * This works for Dataflow runner v1 as well.
   * For portable runners and Dataflow runner v1 what we update is the SDK 
Harness not "worker harness".
   

##########
File path: sdks/python/apache_beam/runners/dataflow/internal/apiclient.py
##########
@@ -1096,8 +1096,8 @@ def get_container_image_from_options(pipeline_options):
       str: Container image for remote execution.
   """
   worker_options = pipeline_options.view_as(WorkerOptions)
-  if worker_options.worker_harness_container_image:

Review comment:
       Yeah, you are right.




-- 
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:
[email protected]


Reply via email to