Fix incorrect parsing of None value parameters. #24948
Conversation
|
Assigning reviewers. If you would like to opt out of this review, comment R: @AnandInguva for label python. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
Codecov Report
@@ Coverage Diff @@
## master #24948 +/- ##
==========================================
- Coverage 73.07% 73.06% -0.02%
==========================================
Files 735 735
Lines 98133 98135 +2
==========================================
- Hits 71710 71701 -9
- Misses 25063 25074 +11
Partials 1360 1360
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| @@ -284,6 +284,10 @@ def from_dictionary(cls, options): | |||
| flags.append('--%s=%s' % (k, i)) | |||
| elif isinstance(v, dict): | |||
| flags.append('--%s=%s' % (k, json.dumps(v))) | |||
| elif v is None: | |||
There was a problem hiding this comment.
Can you also write a small test for this None type args?
There was a problem hiding this comment.
Added a unit test case and confirmed it fails without and passes with my change in my local env.
|
R: @tvalentyn for final approval |
|
PythonDocs test is unrelated and fix is provided at #25000 |
Co-authored-by: Anand Inguva <[email protected]>
|
R : @damccorm @jrmccluskey for final review ( Valentyn on vacation) |
|
Run Python PreCommit |
When a parameter is set as None it gets parsed as the string "None". By not setting the flag in pipeline_options.py we allow it to be set to the default value, which correctly resolves to None.
R: @robertwb
R: @lukecwik
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
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, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.