robertwb commented on code in PR #22132:
URL: https://github.com/apache/beam/pull/22132#discussion_r917041980
##########
sdks/python/apache_beam/options/pipeline_options.py:
##########
@@ -348,12 +348,15 @@ def get_all_options(
del result[k]
if overrides:
- _LOGGER.warning("Discarding invalid overrides: %s", overrides)
+ if retain_unknown_options:
+ result.update(overrides)
Review Comment:
Correct, we need to keep things if we don't know (for sure) what the default
is.
We can't easily consolidate because as well as things in `overrides` that
don't work for `parser.get_default` (or at least are not safe to use the
guessed default) there may be things already in `result` that are not in
`overrides` that we want to remove because they have default values.
I agree this code is all very messy. Were I to do it again I would probably
try better to detangle the argument parsing code from PipelineOptions.
--
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]
