-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[BEAM-14505] Add Dataflow streaming pipeline update support to the Go SDK #17747
Conversation
Can one of the admins verify this patch? |
1 similar comment
Can one of the admins verify this patch? |
Codecov Report
@@ Coverage Diff @@
## master #17747 +/- ##
==========================================
- Coverage 73.99% 73.99% -0.01%
==========================================
Files 695 695
Lines 91826 91843 +17
==========================================
+ Hits 67948 67958 +10
- Misses 22632 22639 +7
Partials 1246 1246
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Assigning reviewers. If you would like to opt out of this review, comment R: @riteshghorse for label go. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
R: @lostluck |
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
Jack, please update the PR description, it's incorrect: It's no longer a WIP and it does allow transform overrides (unless I missed my read). |
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
I overall don't love the way we have these, as package variables, but that's unrelated to this PR. We could clean up the tests if we had a config struct, and had the flags all using the *Var versions pointing to that struct. The struct is the plumbed through instead of direct access to the flags. (I guess we already have such a struct for JobOptions too)
Then we aren't accessing the flags everywhere, and testing different configurations becomes setting fields in the struct, which doesn't have ordering risk, presently mitigated by re-setting all the flags.
But that's for a different PR.
Nvm. I did it, since there was nothing else for you to change blocking a merge. |
Adds the ability to update streaming pipelines running on Dataflow through the
--update
flag. Includes adding the standard way to map old transforms to new ones, as described in https://cloud.google.com/dataflow/docs/guides/updating-a-pipelineThank 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-XXX
with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.CHANGES.md
with 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.