[#24789][prism] internal/worker + tentative data #25478
Conversation
|
R: @jrmccluskey |
|
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 #25478 +/- ##
==========================================
- Coverage 72.93% 72.84% -0.10%
==========================================
Files 748 755 +7
Lines 99366 100421 +1055
==========================================
+ Hits 72476 73151 +675
- Misses 25523 25852 +329
- Partials 1367 1418 +51
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 |
|
Would you mind helping me usher in the Prism runner before the 2.46 cut? I'd love for it to be in a usable state in that release, and to get it so others can possible help complete it sooner. In particular for reviews at this stage, it would be great to get doc/ code ordering changes so it's not entirely confusing to readers. New functionality/bug fixes will become TODOs though, rather than completing them, since they may have strong follow on effects to the branch. |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
|
Of course, I'll have a look at this tomorrow! (UTC+1) |
There was a problem hiding this comment.
Looks great! At this point I don't have enough knowledge to provide a meaningful review with regards to functionality (but I'm interested to learn and like to read your code, so thanks for tagging me). I left a few comments for some minor things you may or may not want to address 😊 well done!
| func toSlogSev(sev fnpb.LogEntry_Severity_Enum) slog.Level { | ||
| switch sev { | ||
| case fnpb.LogEntry_Severity_TRACE: | ||
| return slog.Level(-8) // |
There was a problem hiding this comment.
| return slog.Level(-8) // | |
| return slog.Level(-8) |
|
These were excellent! Thank you very much. When there's nearly 8000 lines to get in (including documentation, tests and test functions), it really adds up, so splitting things up is hopefully going to make it easier to review. Unfortunately, we're now getting into the complicated stuff. |
* [prism] internal/worker + tentative data * [prism] B method comments. * [prism] worker PR comments --------- Co-authored-by: lostluck <13907733+lostluck@users.noreply.github.com>
Add the GRPC FnAPI implementation that sits between SDK harnesses and the runner on workers.
For Prism's initial implementation.
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.