-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[#24339] Make Slices use iterable coder instead of custom coder. #24346
Conversation
Codecov Report
@@ Coverage Diff @@
## master #24346 +/- ##
==========================================
+ Coverage 73.34% 73.36% +0.01%
==========================================
Files 718 718
Lines 96966 97034 +68
==========================================
+ Hits 71120 71187 +67
+ Misses 24515 24503 -12
- Partials 1331 1344 +13
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: @jrmccluskey for label go. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
Run Go PostCommit |
This should unblock RunInference. |
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
Looks like not having a comprehensive test runner for all the different versions and variants of what runners can do strikes again! Should be simple enough though. Specifically, Dataflow appears to turn something into the "state backed" coder, and I'm missing the appropriate handling for it in some instances. |
Run Go PostCommit |
Run Go Flink ValidatesRunner |
Run Go Flink ValidatesRunner |
Run Go PostCommit |
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.
minor nits but LGTM. Thanks for unblocking!
Thanks for the review! |
Changes the default encoding of Go slices to use the beam iterable coder format.
Technically this is the same format as before, but without a length prefix.
Changing coder implementations are generally a breaking change, however this only applies to streaming updates or when trying to re-use the internal pipeline coders. The benefit of cross language compatibility longer term, in a format expected by go users, is perceived to be a larger benefit.
Fixes #24339.
Additionally:
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username
).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.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.