[BEAM-117] Runners should be resilient to DisplayData failure #315
Conversation
This file contains hidden or 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
Display data is collected from PTransforms at Pipeline construction time. Collecting display data runs user code from provided transforms and fn's. These components should be designed not to throw during pipeline construction, however we also shouldn't fail a pipeline if this code does fail. This PR adds resiliency to the DataflowPipelineTranslator, where we collect display data for the Dataflow runner, and also a RunnableOnService test to verify that all runners are resilient to display data failures. Other runners are not yet using display data, but will get this validation for free when they do.
Contributor
Author
|
R: @dhalperi |
| */ | ||
| @Test | ||
| @Category(RunnableOnService.class) | ||
| public void testRunnersResilientToDispalyDataExceptions() { |
Contributor
|
LGTM -- let me know when tests are green and typos are fixed. See if @bjchambers wants to sign off as well? |
Contributor
Author
|
I've addressed all feedback so far. Please take another look. @dhalperi / @bjchambers |
Contributor
|
Contributor
Author
@dhalperi 🍏 📗 💚 !!! |
Contributor
|
Per discussion with bchambers@ -- looks like you want to revisit this to put a message in display data that there was an exception getting the display data? |
Contributor
Author
|
I've addressed all feedback so far. Please take another look. @dhalperi |
Contributor
Author
|
Backport: GoogleCloudPlatform/DataflowJavaSDK#230 |
dhalperi
pushed a commit
to dhalperi/beam
that referenced
this pull request
Aug 23, 2016
* Make IdentityWindowFn and NeverTrigger available This will be used as part of the new PAssert IdentityWindowFn remains package-private to restrict usage. (cherry picked from commit c9a1efa) * fixup! Checkstyle validation
iemejia
pushed a commit
to iemejia/beam
that referenced
this pull request
Jan 12, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
[BEAM-<Jira issue #>] Description of pull requestmvn clean verify. (Even better, enableTravis-CI on your fork and ensure the whole test matrix passes).
<Jira issue #>in the title with the actual Jira issuenumber, if there is one.
Individual Contributor License Agreement.
Display data is collected from PTransforms at Pipeline construction
time. Collecting display data runs user code from provided transforms
and fn's. These components should be designed not to throw during
pipeline construction, however we also shouldn't fail a pipeline
if this code does fail.
This PR adds resiliency to the DataflowPipelineTranslator, where
we collect display data for the Dataflow runner, and also a
RunnableOnService test to verify that all runners are resilient to
display data failures. Other runners are not yet using display data,
but will get this validation for free when they do.