-
Notifications
You must be signed in to change notification settings - Fork 25.2k
[ML] Migrate stream to core error parsing #120722
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
Conversation
Avoid trapping Throwable by rethrowing it on another thread, allowing us to reuse the `generateFailureXContent` for Exceptions and match the new 9.0 format.
Hi @prwhelan, I've created a changelog YAML for you. |
Pinging @elastic/ml-core (Team:ML) |
e = (Exception) t; | ||
} else { | ||
// if not exception, then error, and we should not let it escape. rethrow on another thread, and inform the user we're stopping. | ||
ExceptionsHelper.maybeDieOnAnotherThread(t); |
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.
Just curious, do we need this because we're in a listener? Or why is it that we want to throw the error on a different thread?
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.
My thought process was to throw on another thread and forward the error to the user, though now that I think about it maybe we should invert that (throw on this thread and forward the error on another thread), but perhaps I'm over thinking it.
This is what we did for Bedrock here - #115868
Avoid trapping Throwable by rethrowing it on another thread, allowing us to reuse the `generateFailureXContent` for Exceptions and match the new 9.0 format.
Avoid trapping Throwable by rethrowing it on another thread, allowing us to reuse the
generateFailureXContent
for Exceptions and match the new 9.0 format.