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/20 23:50:49 UTC

[GitHub] [beam] ibzib opened a new pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.

ibzib opened a new pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.
URL: https://github.com/apache/beam/pull/11189
 
 
   See #11052 for context.
   
   ------------------------
   
   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] ibzib commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.

Posted by GitBox <gi...@apache.org>.
ibzib commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.
URL: https://github.com/apache/beam/pull/11189#discussion_r399405064
 
 

 ##########
 File path: sdks/python/apache_beam/options/pipeline_options.py
 ##########
 @@ -285,10 +289,29 @@ def get_all_options(
       cls._add_argparse_args(parser)  # pylint: disable=protected-access
     if add_extra_args_fn:
       add_extra_args_fn(parser)
+
     known_args, unknown_args = parser.parse_known_args(self._flags)
-    if unknown_args:
-      _LOGGER.warning("Discarding unparseable args: %s", unknown_args)
-    result = vars(known_args)
+    if retain_unknown_options:
+      i = 0
+      while i < len(unknown_args):
+        # Treat all unary flags as booleans, and all binary argument values as
+        # strings.
+        if i + 1 >= len(unknown_args) or unknown_args[i + 1].startswith('-'):
+          split = unknown_args[i].rsplit('=')
 
 Review comment:
   The split is only used to get the argument name, and the rest of the split isn't 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] mxm commented on issue #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.

Posted by GitBox <gi...@apache.org>.
mxm commented on issue #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.
URL: https://github.com/apache/beam/pull/11189#issuecomment-603368311
 
 
   Thanks for filing the bug. Unfortunately we have some post commits failing after adding Flink 1.10.

----------------------------------------------------------------
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] mxm commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.
URL: https://github.com/apache/beam/pull/11189#discussion_r396310204
 
 

 ##########
 File path: sdks/python/apache_beam/options/pipeline_options.py
 ##########
 @@ -285,10 +289,25 @@ def get_all_options(
       cls._add_argparse_args(parser)  # pylint: disable=protected-access
     if add_extra_args_fn:
       add_extra_args_fn(parser)
+
     known_args, unknown_args = parser.parse_known_args(self._flags)
-    if unknown_args:
-      _LOGGER.warning("Discarding unparseable args: %s", unknown_args)
-    result = vars(known_args)
+    if retain_unknown_options:
+      i = 0
+      while i < len(unknown_args):
+        # Treat all unary flags as booleans, and all binary argument values as
+        # strings.
+        if i + 1 >= len(unknown_args) or unknown_args[i + 1].startswith('--'):
 
 Review comment:
   Should we always check a parameter name starts with `--`, like done here for boolean 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

[GitHub] [beam] mxm commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.
URL: https://github.com/apache/beam/pull/11189#discussion_r399538469
 
 

 ##########
 File path: sdks/python/apache_beam/options/pipeline_options.py
 ##########
 @@ -285,10 +289,29 @@ def get_all_options(
       cls._add_argparse_args(parser)  # pylint: disable=protected-access
     if add_extra_args_fn:
       add_extra_args_fn(parser)
+
     known_args, unknown_args = parser.parse_known_args(self._flags)
-    if unknown_args:
-      _LOGGER.warning("Discarding unparseable args: %s", unknown_args)
-    result = vars(known_args)
+    if retain_unknown_options:
+      i = 0
+      while i < len(unknown_args):
+        # Treat all unary flags as booleans, and all binary argument values as
+        # strings.
+        if i + 1 >= len(unknown_args) or unknown_args[i + 1].startswith('-'):
+          split = unknown_args[i].rsplit('=')
 
 Review comment:
   Comment still applies. Splitting should be performed from the left, not the right. At most one split has to be performed.

----------------------------------------------------------------
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] ibzib commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.

Posted by GitBox <gi...@apache.org>.
ibzib commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.
URL: https://github.com/apache/beam/pull/11189#discussion_r399604848
 
 

 ##########
 File path: sdks/python/apache_beam/options/pipeline_options.py
 ##########
 @@ -285,10 +289,29 @@ def get_all_options(
       cls._add_argparse_args(parser)  # pylint: disable=protected-access
     if add_extra_args_fn:
       add_extra_args_fn(parser)
+
     known_args, unknown_args = parser.parse_known_args(self._flags)
-    if unknown_args:
-      _LOGGER.warning("Discarding unparseable args: %s", unknown_args)
-    result = vars(known_args)
+    if retain_unknown_options:
+      i = 0
+      while i < len(unknown_args):
+        # Treat all unary flags as booleans, and all binary argument values as
+        # strings.
+        if i + 1 >= len(unknown_args) or unknown_args[i + 1].startswith('-'):
+          split = unknown_args[i].rsplit('=')
 
 Review comment:
   Oh, I see what you mean now. I will commit your suggestion to avoid confusion. However, I don't think we should support unrecognized `append` arguments. I don't want to have to infer complex argument types. As for the specific case of `experiments`, that will not be a problem because `experiments` is already recognized by the parser.

----------------------------------------------------------------
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] mxm commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.
URL: https://github.com/apache/beam/pull/11189#discussion_r399107521
 
 

 ##########
 File path: sdks/python/apache_beam/options/pipeline_options.py
 ##########
 @@ -285,10 +289,29 @@ def get_all_options(
       cls._add_argparse_args(parser)  # pylint: disable=protected-access
     if add_extra_args_fn:
       add_extra_args_fn(parser)
+
     known_args, unknown_args = parser.parse_known_args(self._flags)
-    if unknown_args:
-      _LOGGER.warning("Discarding unparseable args: %s", unknown_args)
-    result = vars(known_args)
+    if retain_unknown_options:
+      i = 0
+      while i < len(unknown_args):
+        # Treat all unary flags as booleans, and all binary argument values as
+        # strings.
+        if i + 1 >= len(unknown_args) or unknown_args[i + 1].startswith('-'):
+          split = unknown_args[i].rsplit('=')
 
 Review comment:
   Why `rsplit`? Shouldn't this be: 
   ```suggestion
             split = unknown_args[i].split('=', 1)
   ```
   
   ?

----------------------------------------------------------------
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] ibzib commented on issue #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.

Posted by GitBox <gi...@apache.org>.
ibzib commented on issue #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.
URL: https://github.com/apache/beam/pull/11189#issuecomment-601960942
 
 
   Run PortableJar_Flink PostCommit

----------------------------------------------------------------
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] ibzib merged pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.

Posted by GitBox <gi...@apache.org>.
ibzib merged pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.
URL: https://github.com/apache/beam/pull/11189
 
 
   

----------------------------------------------------------------
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] ibzib commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.

Posted by GitBox <gi...@apache.org>.
ibzib commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.
URL: https://github.com/apache/beam/pull/11189#discussion_r396677870
 
 

 ##########
 File path: sdks/python/apache_beam/options/pipeline_options.py
 ##########
 @@ -285,10 +289,25 @@ def get_all_options(
       cls._add_argparse_args(parser)  # pylint: disable=protected-access
     if add_extra_args_fn:
       add_extra_args_fn(parser)
+
     known_args, unknown_args = parser.parse_known_args(self._flags)
-    if unknown_args:
-      _LOGGER.warning("Discarding unparseable args: %s", unknown_args)
-    result = vars(known_args)
+    if retain_unknown_options:
+      i = 0
+      while i < len(unknown_args):
+        # Treat all unary flags as booleans, and all binary argument values as
+        # strings.
+        if i + 1 >= len(unknown_args) or unknown_args[i + 1].startswith('--'):
+          parser.add_argument(unknown_args[i], action='store_true')
+          i += 1
+        else:
+          parser.add_argument(unknown_args[i], type=str)
+          i += 2
 
 Review comment:
   Good catch. I will have to add additional logic to handle 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] mxm commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.
URL: https://github.com/apache/beam/pull/11189#discussion_r399108204
 
 

 ##########
 File path: sdks/python/apache_beam/options/pipeline_options.py
 ##########
 @@ -285,10 +289,29 @@ def get_all_options(
       cls._add_argparse_args(parser)  # pylint: disable=protected-access
     if add_extra_args_fn:
       add_extra_args_fn(parser)
+
     known_args, unknown_args = parser.parse_known_args(self._flags)
-    if unknown_args:
-      _LOGGER.warning("Discarding unparseable args: %s", unknown_args)
-    result = vars(known_args)
+    if retain_unknown_options:
+      i = 0
+      while i < len(unknown_args):
+        # Treat all unary flags as booleans, and all binary argument values as
+        # strings.
+        if i + 1 >= len(unknown_args) or unknown_args[i + 1].startswith('-'):
+          split = unknown_args[i].rsplit('=')
 
 Review comment:
   Otherwise this will break options like `experiments=state_cache_size=1`.

----------------------------------------------------------------
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] ibzib commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.

Posted by GitBox <gi...@apache.org>.
ibzib commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.
URL: https://github.com/apache/beam/pull/11189#discussion_r396684486
 
 

 ##########
 File path: sdks/python/apache_beam/options/pipeline_options.py
 ##########
 @@ -285,10 +289,25 @@ def get_all_options(
       cls._add_argparse_args(parser)  # pylint: disable=protected-access
     if add_extra_args_fn:
       add_extra_args_fn(parser)
+
     known_args, unknown_args = parser.parse_known_args(self._flags)
-    if unknown_args:
-      _LOGGER.warning("Discarding unparseable args: %s", unknown_args)
-    result = vars(known_args)
+    if retain_unknown_options:
+      i = 0
+      while i < len(unknown_args):
+        # Treat all unary flags as booleans, and all binary argument values as
+        # strings.
+        if i + 1 >= len(unknown_args) or unknown_args[i + 1].startswith('--'):
 
 Review comment:
   I guess I should change it to `-`, not `--`. `-` is the default prefix for argparse, and AFAICT that is true for Beam as well, even though most (all?) pipeline options use `--`.
   https://docs.python.org/3/library/argparse.html#prefix-chars
   
   So then the question becomes "Should we always check a parameter name starts with `-`?" And the answer is we don't have to, because the call to parse_args will do that for us. `test_retain_unknown_options_unary_missing_prefix` tests 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] mxm commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.
URL: https://github.com/apache/beam/pull/11189#discussion_r397313859
 
 

 ##########
 File path: sdks/python/apache_beam/options/pipeline_options.py
 ##########
 @@ -285,10 +289,25 @@ def get_all_options(
       cls._add_argparse_args(parser)  # pylint: disable=protected-access
     if add_extra_args_fn:
       add_extra_args_fn(parser)
+
     known_args, unknown_args = parser.parse_known_args(self._flags)
-    if unknown_args:
-      _LOGGER.warning("Discarding unparseable args: %s", unknown_args)
-    result = vars(known_args)
+    if retain_unknown_options:
+      i = 0
+      while i < len(unknown_args):
+        # Treat all unary flags as booleans, and all binary argument values as
+        # strings.
+        if i + 1 >= len(unknown_args) or unknown_args[i + 1].startswith('--'):
 
 Review comment:
   That sounds 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] mxm commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.
URL: https://github.com/apache/beam/pull/11189#discussion_r396309448
 
 

 ##########
 File path: sdks/python/apache_beam/options/pipeline_options.py
 ##########
 @@ -285,10 +289,25 @@ def get_all_options(
       cls._add_argparse_args(parser)  # pylint: disable=protected-access
     if add_extra_args_fn:
       add_extra_args_fn(parser)
+
     known_args, unknown_args = parser.parse_known_args(self._flags)
-    if unknown_args:
-      _LOGGER.warning("Discarding unparseable args: %s", unknown_args)
-    result = vars(known_args)
+    if retain_unknown_options:
+      i = 0
+      while i < len(unknown_args):
+        # Treat all unary flags as booleans, and all binary argument values as
+        # strings.
+        if i + 1 >= len(unknown_args) or unknown_args[i + 1].startswith('--'):
+          parser.add_argument(unknown_args[i], action='store_true')
+          i += 1
+        else:
+          parser.add_argument(unknown_args[i], type=str)
+          i += 2
 
 Review comment:
   What about the `--arg=value` format?

----------------------------------------------------------------
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] ibzib commented on issue #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.

Posted by GitBox <gi...@apache.org>.
ibzib commented on issue #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.
URL: https://github.com/apache/beam/pull/11189#issuecomment-602808467
 
 
   @mxm thanks for the feedback, I addressed your comments.
   
   Unrelated: I wasn't aware that the Flink portable jar postcommit has been failing. I filed and self-assigned BEAM-9575.

----------------------------------------------------------------
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] mxm commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.

Posted by GitBox <gi...@apache.org>.
mxm commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.
URL: https://github.com/apache/beam/pull/11189#discussion_r400476144
 
 

 ##########
 File path: sdks/python/apache_beam/options/pipeline_options.py
 ##########
 @@ -285,10 +289,29 @@ def get_all_options(
       cls._add_argparse_args(parser)  # pylint: disable=protected-access
     if add_extra_args_fn:
       add_extra_args_fn(parser)
+
     known_args, unknown_args = parser.parse_known_args(self._flags)
-    if unknown_args:
-      _LOGGER.warning("Discarding unparseable args: %s", unknown_args)
-    result = vars(known_args)
+    if retain_unknown_options:
+      i = 0
+      while i < len(unknown_args):
+        # Treat all unary flags as booleans, and all binary argument values as
+        # strings.
+        if i + 1 >= len(unknown_args) or unknown_args[i + 1].startswith('-'):
+          split = unknown_args[i].rsplit('=')
 
 Review comment:
   I was only referring to options values containing `=` which can be the case independently of append options. That is fixed now. So all 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] ibzib commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.

Posted by GitBox <gi...@apache.org>.
ibzib commented on a change in pull request #11189: [BEAM-9446] Retain unknown arguments when using uber jar job server.
URL: https://github.com/apache/beam/pull/11189#discussion_r400448482
 
 

 ##########
 File path: sdks/python/apache_beam/options/pipeline_options.py
 ##########
 @@ -285,10 +289,29 @@ def get_all_options(
       cls._add_argparse_args(parser)  # pylint: disable=protected-access
     if add_extra_args_fn:
       add_extra_args_fn(parser)
+
     known_args, unknown_args = parser.parse_known_args(self._flags)
-    if unknown_args:
-      _LOGGER.warning("Discarding unparseable args: %s", unknown_args)
-    result = vars(known_args)
+    if retain_unknown_options:
+      i = 0
+      while i < len(unknown_args):
+        # Treat all unary flags as booleans, and all binary argument values as
+        # strings.
+        if i + 1 >= len(unknown_args) or unknown_args[i + 1].startswith('-'):
+          split = unknown_args[i].rsplit('=')
 
 Review comment:
   I will merge this now. If we need to support `append` arguments for some reason, we should be able to add that 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