-
Notifications
You must be signed in to change notification settings - Fork 25.6k
[ML] Remove tasktype any from supportedStreamingTasks #121460
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
Changes from all commits
13217ea
d6bbd6b
d524718
3a78c34
331eaac
07bf2aa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,11 +38,27 @@ public abstract class BaseResponseHandler implements ResponseHandler { | |
| protected final String requestType; | ||
| private final ResponseParser parseFunction; | ||
| private final Function<HttpResult, ErrorResponse> errorParseFunction; | ||
| private final boolean canHandleStreamingResponses; | ||
|
|
||
| public BaseResponseHandler(String requestType, ResponseParser parseFunction, Function<HttpResult, ErrorResponse> errorParseFunction) { | ||
| this(requestType, parseFunction, errorParseFunction, false); | ||
| } | ||
|
|
||
| public BaseResponseHandler( | ||
| String requestType, | ||
| ResponseParser parseFunction, | ||
| Function<HttpResult, ErrorResponse> errorParseFunction, | ||
| boolean canHandleStreamingResponses | ||
| ) { | ||
| this.requestType = Objects.requireNonNull(requestType); | ||
| this.parseFunction = Objects.requireNonNull(parseFunction); | ||
| this.errorParseFunction = Objects.requireNonNull(errorParseFunction); | ||
| this.canHandleStreamingResponses = canHandleStreamingResponses; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean canHandleStreamingResponses() { | ||
| return canHandleStreamingResponses; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved here to the base class. |
||
| } | ||
|
|
||
| @Override | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,12 @@ | |
| import org.elasticsearch.xpack.inference.logging.ThrottlerManager; | ||
|
|
||
| public abstract class AmazonBedrockResponseHandler implements ResponseHandler { | ||
|
|
||
| @Override | ||
| public boolean canHandleStreamingResponses() { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a bit unfortunate. This response handler is a workaround and isn't really used because this service uses its own client. AmazonBedrock does support streaming the responses but its handled in a separate place.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Eh we need to separate Bedrock from everything after Sender anyway, since it has its own internal threadpool and all the stuff we wrap around the http client |
||
| return false; | ||
| } | ||
|
|
||
| @Override | ||
| public final void validateResponse(ThrottlerManager throttlerManager, Logger logger, Request request, HttpResult result) | ||
| throws RetryException { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,4 +58,8 @@ public InferenceServiceResults parseResult(Request request, HttpResult result) t | |
| } | ||
| } | ||
|
|
||
| @Override | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved this class to the test directory since that's the only place it's referenced. |
||
| public boolean canHandleStreamingResponses() { | ||
| return false; | ||
| } | ||
| } | ||
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.
Use the model's task type to determine streaming (can never be any).