-
Notifications
You must be signed in to change notification settings - Fork 4.5k
[BEAM-11104] Add ProcessContinuation type to Go SDK #17265
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Can one of the admins verify this patch? |
3 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
Codecov Report
@@ Coverage Diff @@
## master #17265 +/- ##
==========================================
- Coverage 74.16% 74.09% -0.07%
==========================================
Files 675 679 +4
Lines 88896 89087 +191
==========================================
+ Hits 65926 66010 +84
- Misses 21835 21926 +91
- Partials 1135 1151 +16
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Not sure what fmt is catching here, it's not a file I touched and running fmt on the offending file (core/typex/special.go) is not changing anything. |
|
PR to fix the fmt break at #17266 |
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
|
assign set of reviewers |
|
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). |
|
R: @lostluck for final approval |
|
The bot was a little overeager, happy to have @lostluck weigh in though :) I put up a one liner fix for the bot issue fwiw |
|
Please add a link to the design doc in the top description. Makes things a bit more accessible than indirecting to the JIRA (which isn't always as discoverable as one might hope). |
lostluck
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty good TBH. A few comments/changes on this pass.
lostluck
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM at this point, other than the suggested edits. Errors don't have to be pithy and short. We can add more information, like links to the jira, which when resolved will indicate which release starts supporting the feature.
Co-authored-by: Robert Burke <lostluck@users.noreply.github.com>
lostluck
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Adds the ProcessContinuation interface to the Go SDK and adds handling for it to be a valid return value in funcx/fn.go. Also adds a unit test case for the new return type.
Design Doc: https://docs.google.com/document/d/1_JbzjY9JR07ZK5v7PcZevUfzHPsqwzfV7W6AouNpMPk/edit?usp=sharing
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).[BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replaceBEAM-XXXwith the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.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.