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

   Fixes #22434
   
   `PipelineOptions.get_all_options(retain_unknown_options=True)` registers 
each unknown flag with argparse and then parses the original flags again. It 
decided whether an unknown flag takes a value by looking at the next entry in 
the list of unknown args. argparse takes the known options out of that list, so 
two entries that are next to each other there are not always next to each other 
in the original flags.
   
   For `--unknown_flag --job_name=foo bar` the unknown args are 
`['--unknown_flag', 'bar']`. `--unknown_flag` was registered as taking a 
string, and the second parse failed with `argument --unknown_flag: expected one 
argument`. `Pipeline.run()` calls 
`get_all_options(retain_unknown_options=True)`, so the pipeline failed as well.
   
   This change looks up where each unknown arg sits in the original flags, and 
only treats an unknown flag as taking a value when the value directly follows 
it there. Cases that already worked behave as before:
   
   - `--flag value` and `--flag=value` are kept as strings, `--flag` on its own 
as `True`
   - a value used by a known option or positional is not taken by an unknown 
flag
   - single dash flags that take a value are still dropped with a warning
   - parsing still stops at `--`
   
   One related case also changes: `--flag=value` followed by a stray argument 
now keeps its inline value. Before, the whole `--flag=value` token was 
registered as the flag name and given the stray argument (the input that broke 
the load tests in #22422).
   
   Tests: added cases to `pipeline_options_test.py` for the example in the 
issue, an unknown flag before a known option with a separate value, unknown and 
known flags mixed, `--flag=value` followed by a stray value, repeated unary and 
repeated valued flags, `--`, a single dash flag, and a known positional. Six of 
them fail without the change. Locally:
   
   - `pytest apache_beam/options/`: 247 passed, 14 skipped
   - `pytest apache_beam/pipeline_test.py 
apache_beam/transforms/display_test.py`: 72 passed, 2 skipped
   - `pytest apache_beam/runners/portability/portable_runner_test.py -k 
option`: 5 passed, 2 skipped
   - `yapf --diff` (0.43.0), `ruff check` (0.15.22) and `pyrefly check` (1.1.1) 
on the changed files: clean
   
   I updated `CHANGES.md` under Bugfixes for 2.78.0.
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us 
incorporate your contribution quickly and easily:
   
    - [ ] If this contribution is large, please file an Apache [Individual 
Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
    - [x] Replace these instructions with a description of your change and why 
you have made it.
    - [x] Mention the appropriate issue in your description (for example: 
`addresses #123` or `fixes #456`).
    - [x] Update `CHANGES.md` with noteworthy changes.
   


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

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to