Also use config pattern for yaml providers.#27884
Conversation
|
R: @damccorm |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
Codecov Report
@@ Coverage Diff @@
## master #27884 +/- ##
==========================================
- Coverage 70.86% 70.59% -0.27%
==========================================
Files 861 857 -4
Lines 105010 103951 -1059
==========================================
- Hits 74413 73382 -1031
+ Misses 29038 29010 -28
Partials 1559 1559
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 25 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| 'appendix' | ||
| ] | ||
| })) | ||
| lambda: subprocess_server.JavaJarServer.path_to_maven_jar(**config)) |
There was a problem hiding this comment.
Should we have some validation here or elsewhere to make sure that we're passing in a valid value (one of
'artifact_id',
'group_id',
'version',
'repository',
'classifier',
'appendix'
)? I'm a little worried that the error we return here if passing in a bad config would be hard for users to understand
There was a problem hiding this comment.
Good point. Fixed it up so the errors will be earlier and cleaner.
There is a bit of redundancy here, but I think the tradeoff is worth it.
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.