damccorm commented on code in PR #22132:
URL: https://github.com/apache/beam/pull/22132#discussion_r915838753
##########
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:
> Also, we don't necessarily know what the defaults are for values that
don't come from flags.
I guess my point is, will we ever know what the defaults are? If so, its
probably worth keeping?
I still think we could consolidate - even though the set of pipeline options
is the union of result and overrides, we're only adding the overrides in the
previous loop so I think `for k in overrides` would be valid in this case.
Regardless, its at most a small code cleanliness win (if that), so I'm good
leaving it as is.
--
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]
