-
Notifications
You must be signed in to change notification settings - Fork 129
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
support train ML model in either sync or async way #124
Conversation
Signed-off-by: Yaliang Wu <ylwu@amazon.com>
20b2dcc
to
7291511
Compare
@@ -63,6 +65,7 @@ | |||
@Setter | |||
private String error; | |||
private User user; // TODO: support document level access control later | |||
private boolean async; | |||
|
|||
@Builder | |||
public MLTask( |
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.
We can consider moving this class to the ml-common in the next PR.
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.
Yes, will send out a separate refactoring PR
if (request.isAsync()) { | ||
mlTaskManager.createMLTask(mlTask, ActionListener.wrap(r -> { | ||
String taskId = r.getId(); | ||
mlTask.setTaskId(taskId); | ||
if (mlTask.isAsync()) { |
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.
Is there any user case that the request is Sync, but the mlTask is Async? I am just wondering the difference between the two.
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.
Async property of MLTask
is consistent with request, check line 116 of this class.
For async task, we will cache task in memory and persist task in index and it will return task id directly.
For sync task, we just cache task in memory. Will train model and return the model id.
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.
Approved with a few questions in the comments.
Signed-off-by: Yaliang Wu ylwu@amazon.com
Description
Support training ML model in either sync or async way.
For example, PPL user should be able to train ML model in sync way; and user can use async way if the model training will be time consuming.
Main changes:
async
URL parameter in train API. Add?async=true
if need to train model in async way, by default will train model in sync way.GetRequest
to get model in predict task runnerCheck List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.