You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "riteshghorse (via GitHub)" <gi...@apache.org> on 2023/03/22 01:38:43 UTC

[GitHub] [beam] riteshghorse opened a new pull request, #25920: [Python] Ignore unknown args with single dash

riteshghorse opened a new pull request, #25920:
URL: https://github.com/apache/beam/pull/25920

   **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:
   
    - [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
    - [ ] 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/get-started-contributing/#make-the-reviewers-job-easier).
   
   To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
   
   GitHub Actions Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   [![Build python source distribution and wheels](https://github.com/apache/beam/workflows/Build%20python%20source%20distribution%20and%20wheels/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python tests](https://github.com/apache/beam/workflows/Python%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java tests](https://github.com/apache/beam/workflows/Java%20Tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Go tests](https://github.com/apache/beam/workflows/Go%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Go+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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] github-actions[bot] commented on pull request #25920: [Python] Ignore unknown args with single dash

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25920:
URL: https://github.com/apache/beam/pull/25920#issuecomment-1478805084

   Stopping reviewer notifications for this pull request: requested by reviewer


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn commented on pull request #25920: [Python] Ignore unknown args with single dash

Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on PR #25920:
URL: https://github.com/apache/beam/pull/25920#issuecomment-1480043566

   Run Python_Runners 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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn commented on a diff in pull request #25920: [Python] Ignore unknown args with single dash

Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on code in PR #25920:
URL: https://github.com/apache/beam/pull/25920#discussion_r1146456293


##########
CHANGES.md:
##########
@@ -68,6 +68,7 @@
 ## Breaking Changes
 
 * If a main session fails to load, the pipeline will now fail at worker startup. ([#25401](https://github.com/apache/beam/issues/25401)).
+* Python Pipelines will now ignore flags with single dash.  ([#25943](https://github.com/apache/beam/issues/25943)).

Review Comment:
   ```suggestion
   * Python pipeline options will now ignore unparsed command line flags prefixed with a single dash. ([#25943](https://github.com/apache/beam/issues/25943)).
   ```



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn commented on a diff in pull request #25920: [Python] Ignore unknown args with single dash

Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on code in PR #25920:
URL: https://github.com/apache/beam/pull/25920#discussion_r1322122087


##########
sdks/python/apache_beam/options/pipeline_options.py:
##########
@@ -347,9 +347,16 @@ def get_all_options(
           else:
             parser.add_argument(split[0], type=str)
           i += 1
-        else:
+        elif unknown_args[i].startswith('--'):
           parser.add_argument(unknown_args[i], type=str)
           i += 2
+        else:
+          # skip all binary flags used with '-' and not '--'.
+          # ex: using -f instead of --f (or --flexrs_goal) will prevent
+          # argument validation before job submission and can be incorrectly

Review Comment:
   somehow we are not getting these warnings.



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] riteshghorse commented on a diff in pull request #25920: [Python] Ignore unknown args with single dash

Posted by "riteshghorse (via GitHub)" <gi...@apache.org>.
riteshghorse commented on code in PR #25920:
URL: https://github.com/apache/beam/pull/25920#discussion_r1146319513


##########
sdks/python/apache_beam/options/pipeline_options.py:
##########
@@ -347,9 +347,16 @@ def get_all_options(
           else:
             parser.add_argument(split[0], type=str)
           i += 1
-        else:
+        elif unknown_args[i].startswith('--'):
           parser.add_argument(unknown_args[i], type=str)
           i += 2
+        else:
+          # skip all binary flags used with '-' and not '--'.
+          # ex: using -f instead of --f (or --flexrs_goal) will prevent
+          # argument validation before job submission and can be incorrectly

Review Comment:
   Added warning log



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn commented on pull request #25920: [Python] Ignore unknown args with single dash

Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on PR #25920:
URL: https://github.com/apache/beam/pull/25920#issuecomment-1480043285

   Run Python_Coverage 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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] liferoad commented on a diff in pull request #25920: [Python] Ignore unknown args with single dash

Posted by "liferoad (via GitHub)" <gi...@apache.org>.
liferoad commented on code in PR #25920:
URL: https://github.com/apache/beam/pull/25920#discussion_r1145323776


##########
sdks/python/apache_beam/options/pipeline_options.py:
##########
@@ -347,9 +347,16 @@ def get_all_options(
           else:
             parser.add_argument(split[0], type=str)
           i += 1
-        else:
+        elif unknown_args[i].startswith('--'):
           parser.add_argument(unknown_args[i], type=str)
           i += 2
+        else:
+          # skip all binary flags used with '-' and not '--'.
+          # ex: using -f instead of --f (or --flexrs_goal) will prevent
+          # argument validation before job submission and can be incorrectly

Review Comment:
   shall we log these as warning? @tvalentyn 



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] codecov[bot] commented on pull request #25920: [Python] Ignore unknown args with single dash

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #25920:
URL: https://github.com/apache/beam/pull/25920#issuecomment-1480092435

   ## [Codecov](https://codecov.io/gh/apache/beam/pull/25920?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#25920](https://codecov.io/gh/apache/beam/pull/25920?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0c237c2) into [master](https://codecov.io/gh/apache/beam/commit/ca0787642a6b3804a742326147281c99ae8d08d2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ca07876) will **not change** coverage.
   > The diff coverage is `n/a`.
   
   ```diff
   @@           Coverage Diff           @@
   ##           master   #25920   +/-   ##
   =======================================
     Coverage   71.42%   71.42%           
   =======================================
     Files         778      778           
     Lines      102420   102420           
   =======================================
     Hits        73152    73152           
     Misses      27813    27813           
     Partials     1455     1455           
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/beam/pull/25920?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...dks/python/apache\_beam/options/pipeline\_options.py](https://codecov.io/gh/apache/beam/pull/25920?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vb3B0aW9ucy9waXBlbGluZV9vcHRpb25zLnB5) | `93.97% <ø> (ø)` | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn commented on a diff in pull request #25920: [Python] Ignore unknown args with single dash

Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on code in PR #25920:
URL: https://github.com/apache/beam/pull/25920#discussion_r1322122087


##########
sdks/python/apache_beam/options/pipeline_options.py:
##########
@@ -347,9 +347,16 @@ def get_all_options(
           else:
             parser.add_argument(split[0], type=str)
           i += 1
-        else:
+        elif unknown_args[i].startswith('--'):
           parser.add_argument(unknown_args[i], type=str)
           i += 2
+        else:
+          # skip all binary flags used with '-' and not '--'.
+          # ex: using -f instead of --f (or --flexrs_goal) will prevent
+          # argument validation before job submission and can be incorrectly

Review Comment:
   somehow we are not getting these warnings.



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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] riteshghorse commented on pull request #25920: [Python] Ignore unknown args with single dash

Posted by "riteshghorse (via GitHub)" <gi...@apache.org>.
riteshghorse commented on PR #25920:
URL: https://github.com/apache/beam/pull/25920#issuecomment-1478804506

   stop reviewer notifications


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn commented on pull request #25920: [Python] Ignore unknown args with single dash

Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on PR #25920:
URL: https://github.com/apache/beam/pull/25920#issuecomment-1480044463

   Please add a note to changes.md


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn merged pull request #25920: [Python] Ignore unknown args with single dash

Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn merged PR #25920:
URL: https://github.com/apache/beam/pull/25920


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] tvalentyn commented on pull request #25920: [Python] Ignore unknown args with single dash

Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on PR #25920:
URL: https://github.com/apache/beam/pull/25920#issuecomment-1481504049

   Thank you!


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

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org