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?
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.