Add helpers to interact with pipeline options in boot entrypoints - #39595
Add helpers to interact with pipeline options in boot entrypoints#39595tvalentyn wants to merge 4 commits into
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #39595 +/- ##
============================================
- Coverage 58.16% 58.15% -0.02%
Complexity 13085 13085
============================================
Files 2521 2520 -1
Lines 264530 264915 +385
Branches 10788 10788
============================================
+ Hits 153876 154065 +189
- Misses 104879 105063 +184
- Partials 5775 5787 +12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…k for Dataflow and Portable runners.
694bd5d to
99596c1
Compare
tvalentyn
changed the title
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
R: @shunping who is familiar with the issue, @jrmccluskey for any golang feedback -- thanks! |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Assign a set of reviewers. |
jrmccluskey
left a comment
jrmccluskey
left a comment
There was a problem hiding this comment.
Largely some go style nits, none of these are blockers
| if exps, err := po.GetStringSlice("experiments"); err == nil { | ||
| if expsMap, err := parseExperiments(exps); err == nil { | ||
| po.experiments = expsMap | ||
| } | ||
| } | ||
| return po, nil |
There was a problem hiding this comment.
This is the only place in the function where the code potentially receiving and handling an error, but it does wind up swallowing it. If the error here is relevant it should be surfaced, otherwise the function doesn't need to return an error at all.
| return po, nil | ||
| } | ||
|
|
||
| func parseExperiments(slice []string) (map[string]string, error) { |
There was a problem hiding this comment.
Same thing here, there's never an error case to return. Is there an interface you're programming towards here?
| os.Remove("pipeline_options.json") | ||
| } | ||
|
|
||
| func TestPipelineOptions(t *testing.T) { |
There was a problem hiding this comment.
In general the benefit to using table-driven test structures is to avoid re-writing similar/the same testing logic repeatedly, making the test definition here take a complex validate function makes the test much harder to follow.
3 participants
Depending on the runner, the pipeline options payload retrieved from the Provisioning API (
info.GetPipelineOptions()) is structured in two different formats.Dataflow runner produces the proto that looks like the following :
Portable Runner on the other hand has the following structure with keys having certain prefix and suffiix surrounding the option name
Go prism runner additionally adds go_options:
Go Dataflow runner uses both go_options and options, where some pipeline options passed on the command line,are passed in go_options but not in top-level pipeline_options.options, for example
dataflow_service_optionsis nested under go_options->dataflow_service_options , but not present otherwise:This PR attempts to provide a helper that can access pipeline options for any Python/Java/Go entrypoint on any runner, which should allow us to clean up messy usages and ad-hoc helper functions in the entrypoint code.
fixes: #39613
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 or the workflows README to see a list of phrases to trigger workflows.