Add ErrorHandler DLQ API to Python #31856
Conversation
|
R: @johnjcasey |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
There was a problem hiding this comment.
while pending @johnjcasey review also went through once and mostly LGTM
| a fluent manner, disaggregating the error processing specification from | ||
| the main processing chain. | ||
|
|
||
| They is typically used as follows:: |
|
Thanks. Any other thoughts on this, @johnjcasey ? |
|
Another ping on this. |
|
would be nice to get this in by next release. For past contributor/reviewers on this topic, maybe @bzablocki (who reviewed #27145) could take another look if needed? |
| with beam.Pipeline() as p: | ||
| pcoll = p | beam.Create(['a', 'bb', 'cccc']) | ||
| with error_handling.ErrorHandler( | ||
| beam.Map(lambda x: "error: %s" % x[0])) as error_handler: |
There was a problem hiding this comment.
Why do we access here the first element in the array (Map(lambda x: "error: %s" % x[0]))) and in the test above we just access the entire element (Map(lambda x: "error: %s" % x))?
There was a problem hiding this comment.
PTransformWithErrors.with_error_handling() returns a PCollection of elements as errors, whereas the standard ParDo(...).with_error_handling() returns the Python equivalent of bad records that attach the bad elements to the exception thrown.
| assert_that(result, equal_to(['A', 'Bb']), label='CheckGood') | ||
| assert_that(error_pcoll, equal_to(['cccc']), label='CheckBad') | ||
|
|
||
| def test_error_on_collecting_error_handler_without_output_retrieval(self): |
There was a problem hiding this comment.
Shall we also add a unit test with the CollectingErrorHandler that is closed but not consumed?
There was a problem hiding this comment.
This is closed (due to the context) but not consumed.
| timeout, | ||
| error_handler) | ||
|
|
||
| def with_error_handler(self, error_handler, **exception_handling_kwargs): |
There was a problem hiding this comment.
Could we also have a unit test that would show how the **exception_handling_kwargs can be used?
Co-authored-by: Bartosz Zablocki <[email protected]>
It recently came to my attention that this was only added to Java.
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.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 or the workflows README to see a list of phrases to trigger workflows.