SbloodyS commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r882377386
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/java/org/apache/dolphinscheduler/plugin/task/mlflow/MlflowTask.java:
##########
@@ -98,10 +98,15 @@ public void handle() throws Exception {
@Override
public void cancelApplication(boolean cancelApplication) throws Exception {
// cancel process
- shellCommandExecutor.cancelApplication();
+ if
(mlflowParameters.getDeployType().equals(MlflowConstants.MLFLOW_MODELS_DEPLOY_TYPE_DOCKER_COMPOSE))
{
+ String envCommand = mlflowParameters.getDockerComposeEnvCommand();
+ shellCommandExecutor.run(envCommand + "\n" +
MlflowConstants.DOCKER_COMPOSE_CANCEL);
Review Comment:
> we should better have a cancel operator here, but IMO, we can only cancel
some tasks running for a long time , such as `sleep 200` in bash which will
sleep 200s, and users want to stop them, they could call cancelApp, but you
here start the task by fixed `docker compose` command and whether success or
fail will exit the command quickly, Am I right? @caishunfeng
Agreed. We should cancel it by using ```docker rm -f``` as discussed in
https://github.com/apache/dolphinscheduler/pull/10217/files#r882267962.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
