-
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
[DO NOT SUBMIT] Umbrella PR for setting prism as the default Go SDK runner instead of the direct runner. #27550
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Jul 19, 2023
This was referenced Jul 20, 2023
Run Go PostCommit |
This was referenced Jul 27, 2023
3 tasks
lostluck
added a commit
that referenced
this pull request
Aug 7, 2023
* Make the prism runner the default Go SDK runner. * Break cycle with ptest. * [DO NOT SUBMIT] Most changes needec to set prism as default Go SDK runner. * rm commented out code. * Avoid unnecessary logs on normal path. * Fix top. * Adjust Go versions? * [prism] Update symbol lookup to not be unit test specific. * [prism] Support for reshuffles. * [prism] move reshuffle test out of unimplemented. * [prism] Add CoGBK test to unimplemented set. * [prism] graduate additional tests. * delint * [prog] guide updates * [prism] Support CoGBKs and wafer thin fusion. * Make window close strict. * quick first pass * chang cleanup * remove unnecessary churn changes * rm execute line * Update sdks/go/pkg/beam/runners/vet/vet.go Co-authored-by: Ritesh Ghorse <[email protected]> --------- Co-authored-by: lostluck <[email protected]> Co-authored-by: Ritesh Ghorse <[email protected]>
Closing because the contents of this PR have been submitted in other PRs. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Umbrella PR to make prism the default runner. Will be sending out smaller slices of this separately for review.
This fixes up several paper cuts WRT the prism runner vs the direct runner in Go SDK code. As well as fixing up a few issues WRT built-ins running on portable runners.
Critically, it avoids discrepancies between the direct runner implementation and the expectations of other (portable) runners. While this does make things slightly harder to get started, it homogenizes the experience when moving to production beam runners from testing, which will make documenting correct procedures consistent, and provide consistent test implementations.
Sets prism to be the default runner for the Go SDK.
direct
package with prism.Update the SDK to require Go 1.20
Adds a prism stanza for the integration tests
Ensure all unit tested DoFns are registered. Update some things to use the generic registration package. Outstanding work to do a full conversion to the register package.
Add calls to ptest.Main in TestMains for package unit tests that use ptest.
The datastore and fhirio packages stay on the "direct" runner because they don't have portable testing implementations. Those issues have been filed seperately. datastore ([Task][Go SDK]: Update datastoreio tests to not rely on direct runner. #27549) and fhirio ([Task][Go SDK]: Update fhirio tests to not rely on direct runner. #27547), This only pertains to their ability to test, not executed.
Fixed top.accum to handle when it's not in a fused stage.
Update the debug string for ParDo execution nodes to list side inputs.
Update the debug string side inputs to list coder.
Clarify harness ProcessBundle handling as the error source when the failure from materializing ProcessBundleDescriptors to exec.Plans.
Ensure data handling errors that end up at harness are actually reported as errors that fail a bundle.
Update the universal runner to track the last error message over the stream, and return that as the pipeline failure message. Alternatively, wait until the stream terminates or an error message is received after the JobState is Failed. This allows prism (and other portable runners) to produce the failure cause cleanly to the launching task (if it's still up).
Have the universal runner wrapper avoid the Go binary compile step when the execution is set to Loopback mode. Generates a temporary empty file and sets that as the "worker_binary". This avoids waiting on compiles between each test, when the binary would not even be used anyway.
Quieted down some info logs.
Prism Fixes
Additional TODOs
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.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.