[#24789][prism] add preprocessor and test #25520
Conversation
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
Codecov Report
@@ Coverage Diff @@
## master #25520 +/- ##
==========================================
- Coverage 72.77% 72.62% -0.15%
==========================================
Files 749 759 +10
Lines 99434 100638 +1204
==========================================
+ Hits 72362 73092 +730
- Misses 25709 26135 +426
- Partials 1363 1411 +48
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 |
|
Assigning reviewers. If you would like to opt out of this review, comment R: @damccorm for label go. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
|
The lack of regular updates this week or with how codcovv3 is configured is messing with the coverage numbers. Even according to the bot's output, the non-test file is 90+% covered. |
|
Mostly just in case you're interested. |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
| } | ||
| } | ||
|
|
||
| // transformPreparer is an interface for handling different urns in the prepropcessor |
There was a problem hiding this comment.
| // transformPreparer is an interface for handling different urns in the prepropcessor | |
| // transformPreparer is an interface for handling different urns in the preprocessor |
| // transformPreparer is an interface for handling different urns in the prepropcessor | ||
| // largely for exchanging transforms for others, to be added to the complete set of | ||
| // components in the pipeline. | ||
| type transformPreparer interface { |
There was a problem hiding this comment.
In terms of code organization, it may read better if the transformPreparer type definition is moved above the preprocessor, so that the preprocessor type definition and its related functions/methods are grouped together?
There was a problem hiding this comment.
Agreed! Thanks. I do prefer having things pre-declared when a different order doesn't dramatically improve readability.
Adds the handler for the graph pre-processor.
For Prism's initial implementation, it produces single transform stages, so no fusion.
See #24789.
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.