SbloodyS commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r892353404
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/java/org/apache/dolphinscheduler/plugin/task/mlflow/MlflowConstants.java:
##########
@@ -86,9 +88,22 @@ private MlflowConstants() {
public static final String MLFLOW_BUILD_DOCKER = "mlflow models
build-docker -m %s -n %s --enable-mlserver";
-
public static final String DOCKER_RREMOVE_CONTAINER = "docker rm -f %s";
public static final String DOCKER_RUN = "docker run --name=%s -p=%s:8080
%s";
+ public static final String DOCKER_COMPOSE_RUN = "docker-compose up -d";
+
+ public static final String SET_DOCKER_COMPOSE_ENV = "export
DS_TASK_MLFLOW_IMAGE_NAME=%s\n" +
+ "export DS_TASK_MLFLOW_CONTAINER_NAME=%s\n" +
+ "export DS_TASK_MLFLOW_DEPLOY_PORT=%s\n" +
+ "export DS_TASK_MLFLOW_CPU_LIMIT=%s\n" +
+ "export DS_TASK_MLFLOW_MEMORY_LIMIT=%s";
+
+ public static final String DOCKER_HEALTH_CHECK_COMMAND = "for i in $(seq 1
20); " +
Review Comment:
> I think we should prefer to reduce the number of non-essential parameters
to make the task plugin easier to use.
>
> But the opinion above is nice. I think we can extend the timeout time,
from 1 min to 5min or something else. If the MLflow model service container can
not start healthy in 5 min. It's almost certain that something went wrong.
>
> What do you think? @zhongjiajie @SbloodyS
Sounds good to me.
--
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]
