-
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
DLQ support in RunInference #26261
DLQ support in RunInference #26261
Conversation
…damccorm/runInferenceDLQ
Assigning reviewers. If you would like to opt out of this review, comment R: @tvalentyn for label python. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
| 'BeamML_RunInference' >> run_inference_pardo) | ||
|
||
def with_exception_handling( | ||
self, *, exc_class=Exception, use_subprocess=False, threshold=1): |
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.
nit: Do we want to provide main_tag
and dead_letter_tag
? since we already mentioned by default good
tag and bad
tag so not sure how useful it will be.
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.
I originally had it, but intentionally omitted it since those are primarily useful in the context of having >2 outputs
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.
Sg
Run Python_Coverage PreCommit |
Codecov Report
@@ Coverage Diff @@
## master #26261 +/- ##
==========================================
+ Coverage 71.21% 71.81% +0.59%
==========================================
Files 787 752 -35
Lines 103330 101386 -1944
==========================================
- Hits 73588 72809 -779
+ Misses 28246 27095 -1151
+ Partials 1496 1482 -14
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 48 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Args: | ||
exc_class: An exception class, or tuple of exception classes, to catch. | ||
Optional, defaults to 'Exception'. | ||
use_subprocess: Whether to execute the DoFn logic in a subprocess. This |
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.
JFYI, use_subprocess currently has some known issues / limitations, which may surface here as well.
Adds support for
use_exception_handling
inRunInference
. We don't get this for free becauseuse_exception_handling
is a function onParDo
, not onPTransform
(which is the superclass ofRunInference
)Fixes #24209
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.