You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/05/24 03:25:38 UTC
[GitHub] [dolphinscheduler] jieguangzhou opened a new pull request, #10217: [Feature] [MLOps] support mlflow deploy with docker compose
jieguangzhou opened a new pull request, #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217
<!--Thanks very much for contributing to Apache DolphinScheduler. Please review https://dolphinscheduler.apache.org/en-us/community/development/pull-request.html before opening a pull request.-->
## Purpose of the pull request
Update MLflow task plugin to enable Dolphin users to schedule [MLflow](https://mlflow.org/).
This PR relates to https://github.com/apache/dolphinscheduler/issues/10149.
Because I want to continue enriching the capabilities of this plugin, leave the issue open for now.
- [x] Docker Compose: Use docker compose to run the container, Will replace the docker run above
## Brief change log
## Verify this pull request
<!--*(Please pick either of the following options)*-->
This pull request is code cleanup without any test coverage.
*(or)*
This pull request is already covered by existing tests, such as *(please describe tests)*.
(or)
This change added tests and can be verified as follows:
<!--*(example:)*
- *Added dolphinscheduler-dao tests for end-to-end.*
- *Added CronUtilsTest to verify the change.*
- *Manually verified the change by testing locally.* -->
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r884627466
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/resources/docker-compose.yml:
##########
@@ -0,0 +1,28 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+version: "3"
+
+services:
+ mlflow-model:
+ image: "${imageName}"
+ ports:
+ - "${deployPort}:8080"
+ deploy:
+ resources:
+ limits:
+ cpus: "${cpuLimit}"
+ memory: "${memoryLimit}"
Review Comment:
> > I think adding another MLOps tool for Monitor is the better way.
>
> It's better if we can monitor it.
I have no idea to add a monitor in the MLflow task plugin because I still don't think the MLflow task plugin should include monitoring docker health.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r892343069
##########
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
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r890846807
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/java/org/apache/dolphinscheduler/plugin/task/mlflow/MlflowConstants.java:
##########
@@ -86,9 +88,17 @@ 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 DOCKER_COMPOSE_CANCEL = "docker compose down";
Review Comment:
I've fixed it
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1140907997
@zhongjiajie I undid the code about canceling the application because I didn't think we needed to remove the running container here.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1140939310
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10217)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=duplicated_lines_density&view=list) No Duplication information
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1136782989
> Dear Jieguang,
> please check the review.
> Yours
I have fixed them, PTAL, thx.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1135468363
> > > Please associate the corresponding issue. @jieguangzhou
> >
> >
> > This PR relates to #10149. But I can't reopen it.
>
> I have processed it.
Thanks.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r892146278
##########
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 it's better to let users define the timeout instead of using hard code since the user's server may not be that fast.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] songjianet commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
songjianet commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1151854227
> > +1 @zhongjiajie Do you have better suggestions?
>
> I do not have
six six six
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1151848033
Thanks all.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1149703806
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10217)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=duplicated_lines_density&view=list) No Duplication information
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r882259877
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/java/org/apache/dolphinscheduler/plugin/task/mlflow/MlflowConstants.java:
##########
@@ -86,9 +88,17 @@ 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";
Review Comment:
docker-compose up -d?
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/java/org/apache/dolphinscheduler/plugin/task/mlflow/MlflowConstants.java:
##########
@@ -86,9 +88,17 @@ 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 DOCKER_COMPOSE_CANCEL = "docker compose down";
Review Comment:
docker-compose down?
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r882264700
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/java/org/apache/dolphinscheduler/plugin/task/mlflow/MlflowConstants.java:
##########
@@ -86,9 +88,17 @@ 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";
Review Comment:
The mechanism currently implemented is:
1. Move the docker-compose file to the working directory
2. Set the environment variable
3. run "docker compose up -d"
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r882268134
##########
docs/docs/en/guide/task/mlflow.md:
##########
@@ -92,71 +95,65 @@ First, introduce some general parameters of DolphinScheduler
**Task Parameter**
-- **mlflow server tracking uri** ：MLflow server uri, default http://localhost:5000.
-- **experiment name** ：Create the experiment where the task is running, if the experiment does not exist. If the name is empty, it is set to ` Default `, the same as MLflow.
-- **register model** ：Register the model or not. If register is selected, the following parameters are expanded.
- - **model name** : The registered model name is added to the original model version and registered as
+- **Register Model**: Register the model or not. If register is selected, the following parameters are expanded.
+ - **model name**: The registered model name is added to the original model version and registered as
Production.
-- **data path** : The absolute path of the file or folder. Ends with .csv for file or contain train.csv and
- test.csv for folder（In the suggested way, users should build their own test sets for model evaluation）。
-- **parameters** : Parameter when initializing the algorithm/AutoML model, which can be empty. For example
- parameters `n_estimators=200;learning_rate=0.2` for flaml 。The convention will be passed with '; 'shards
+- **Data Path**: The absolute path of the file or folder. Ends with .csv for file or contain train.csv and
+ test.csv for folder(In the suggested way, users should build their own test sets for model evaluation).
+- **Parameters**: Parameter when initializing the algorithm/AutoML model, which can be empty. For example
+ parameters `n_estimators=200;learning_rate=0.2` for flaml. The convention will be passed with '; 'shards
each parameter, using the name before the equal sign as the parameter name, and using the name after the equal
sign to get the corresponding parameter value through `python eval()`. The detailed parameter list is as follows:
- [flaml](https://microsoft.github.io/FLAML/docs/reference/automl#automl-objects)
- [autosklearn](https://automl.github.io/auto-sklearn/master/api.html)
-- **AutoML tool** : The AutoML tool used, currently
+- **AutoML tool**: The AutoML tool used, currently
supports [autosklearn](https://github.com/automl/auto-sklearn)
- and [flaml](https://github.com/microsoft/FLAML)
+ and [flaml](https://github.com/microsoft/FLAML).
Review Comment:
thanks, I will fix that
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r882261398
##########
docs/docs/en/guide/task/mlflow.md:
##########
@@ -92,71 +95,65 @@ First, introduce some general parameters of DolphinScheduler
**Task Parameter**
-- **mlflow server tracking uri** ：MLflow server uri, default http://localhost:5000.
-- **experiment name** ：Create the experiment where the task is running, if the experiment does not exist. If the name is empty, it is set to ` Default `, the same as MLflow.
-- **register model** ：Register the model or not. If register is selected, the following parameters are expanded.
- - **model name** : The registered model name is added to the original model version and registered as
+- **Register Model**: Register the model or not. If register is selected, the following parameters are expanded.
+ - **model name**: The registered model name is added to the original model version and registered as
Production.
-- **data path** : The absolute path of the file or folder. Ends with .csv for file or contain train.csv and
- test.csv for folder（In the suggested way, users should build their own test sets for model evaluation）。
-- **parameters** : Parameter when initializing the algorithm/AutoML model, which can be empty. For example
- parameters `n_estimators=200;learning_rate=0.2` for flaml 。The convention will be passed with '; 'shards
+- **Data Path**: The absolute path of the file or folder. Ends with .csv for file or contain train.csv and
+ test.csv for folder(In the suggested way, users should build their own test sets for model evaluation).
+- **Parameters**: Parameter when initializing the algorithm/AutoML model, which can be empty. For example
+ parameters `n_estimators=200;learning_rate=0.2` for flaml. The convention will be passed with '; 'shards
each parameter, using the name before the equal sign as the parameter name, and using the name after the equal
sign to get the corresponding parameter value through `python eval()`. The detailed parameter list is as follows:
- [flaml](https://microsoft.github.io/FLAML/docs/reference/automl#automl-objects)
- [autosklearn](https://automl.github.io/auto-sklearn/master/api.html)
-- **AutoML tool** : The AutoML tool used, currently
+- **AutoML tool**: The AutoML tool used, currently
supports [autosklearn](https://github.com/automl/auto-sklearn)
- and [flaml](https://github.com/microsoft/FLAML)
+ and [flaml](https://github.com/microsoft/FLAML).
Review Comment:
remove unused blank line?
##########
docs/docs/en/guide/task/mlflow.md:
##########
@@ -92,71 +95,65 @@ First, introduce some general parameters of DolphinScheduler
**Task Parameter**
-- **mlflow server tracking uri** ：MLflow server uri, default http://localhost:5000.
-- **experiment name** ：Create the experiment where the task is running, if the experiment does not exist. If the name is empty, it is set to ` Default `, the same as MLflow.
-- **register model** ：Register the model or not. If register is selected, the following parameters are expanded.
- - **model name** : The registered model name is added to the original model version and registered as
+- **Register Model**: Register the model or not. If register is selected, the following parameters are expanded.
+ - **model name**: The registered model name is added to the original model version and registered as
Production.
-- **data path** : The absolute path of the file or folder. Ends with .csv for file or contain train.csv and
- test.csv for folder（In the suggested way, users should build their own test sets for model evaluation）。
-- **parameters** : Parameter when initializing the algorithm/AutoML model, which can be empty. For example
- parameters `n_estimators=200;learning_rate=0.2` for flaml 。The convention will be passed with '; 'shards
+- **Data Path**: The absolute path of the file or folder. Ends with .csv for file or contain train.csv and
+ test.csv for folder(In the suggested way, users should build their own test sets for model evaluation).
+- **Parameters**: Parameter when initializing the algorithm/AutoML model, which can be empty. For example
+ parameters `n_estimators=200;learning_rate=0.2` for flaml. The convention will be passed with '; 'shards
each parameter, using the name before the equal sign as the parameter name, and using the name after the equal
sign to get the corresponding parameter value through `python eval()`. The detailed parameter list is as follows:
- [flaml](https://microsoft.github.io/FLAML/docs/reference/automl#automl-objects)
- [autosklearn](https://automl.github.io/auto-sklearn/master/api.html)
-- **AutoML tool** : The AutoML tool used, currently
+- **AutoML tool**: The AutoML tool used, currently
supports [autosklearn](https://github.com/automl/auto-sklearn)
- and [flaml](https://github.com/microsoft/FLAML)
+ and [flaml](https://github.com/microsoft/FLAML).
Review Comment:
remove unused blank line
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] codecov-commenter commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1135365764
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10217?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#10217](https://codecov.io/gh/apache/dolphinscheduler/pull/10217?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e933ae7) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/7fc3664ec366ffc7abbd8732d47e56f0f313bab7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7fc3664) will **increase** coverage by `0.03%`.
> The diff coverage is `73.91%`.
```diff
@@ Coverage Diff @@
## dev #10217 +/- ##
============================================
+ Coverage 40.96% 40.99% +0.03%
- Complexity 4735 4741 +6
============================================
Files 854 854
Lines 34565 34580 +15
Branches 3819 3819
============================================
+ Hits 14158 14175 +17
Misses 19051 19051
+ Partials 1356 1354 -2
```
| [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/10217?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...nscheduler/plugin/task/mlflow/MlflowConstants.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10217/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stbWxmbG93L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi90YXNrL21sZmxvdy9NbGZsb3dDb25zdGFudHMuamF2YQ==) | `0.00% <ø> (ø)` | |
| [...olphinscheduler/plugin/task/mlflow/MlflowTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10217/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stbWxmbG93L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi90YXNrL21sZmxvdy9NbGZsb3dUYXNrLmphdmE=) | `68.23% <57.14%> (+0.17%)` | :arrow_up: |
| [...scheduler/plugin/task/mlflow/MlflowParameters.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10217/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stbWxmbG93L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi90YXNrL21sZmxvdy9NbGZsb3dQYXJhbWV0ZXJzLmphdmE=) | `76.59% <100.00%> (+10.29%)` | :arrow_up: |
| [...org/apache/dolphinscheduler/remote/utils/Host.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10217/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL3V0aWxzL0hvc3QuamF2YQ==) | `37.77% <0.00%> (-2.23%)` | :arrow_down: |
| [...e/dolphinscheduler/remote/NettyRemotingClient.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10217/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL05ldHR5UmVtb3RpbmdDbGllbnQuamF2YQ==) | `52.11% <0.00%> (-1.41%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10217?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10217?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [7fc3664...e933ae7](https://codecov.io/gh/apache/dolphinscheduler/pull/10217?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] songjianet commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
songjianet commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1135467979
> > Please associate the corresponding issue. @jieguangzhou
>
> This PR relates to #10149. But I can't reopen it.
I have processed it.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r884488154
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/resources/docker-compose.yml:
##########
@@ -0,0 +1,28 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+version: "3"
+
+services:
+ mlflow-model:
+ image: "${imageName}"
+ ports:
+ - "${deployPort}:8080"
+ deploy:
+ resources:
+ limits:
+ cpus: "${cpuLimit}"
+ memory: "${memoryLimit}"
Review Comment:
WDYT @SbloodyS
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r884632858
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/resources/docker-compose.yml:
##########
@@ -0,0 +1,28 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+version: "3"
+
+services:
+ mlflow-model:
+ image: "${imageName}"
+ ports:
+ - "${deployPort}:8080"
+ deploy:
+ resources:
+ limits:
+ cpus: "${cpuLimit}"
+ memory: "${memoryLimit}"
Review Comment:
But I think it's reasonable to add more parameters if you need to start the Docker container
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1135466343
> Please associate the corresponding issue. @jieguangzhou
This PR relates to https://github.com/apache/dolphinscheduler/issues/10149.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] songjianet commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
songjianet commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1135465456
Please associate the corresponding issue. @jieguangzhou
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1141628983
LGTM overall, but we still have discuss in https://github.com/apache/dolphinscheduler/pull/10217#discussion_r882382374
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r882266160
##########
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:
Is it necessary to implement a cancellation mechanism? I think if it is not necessary, we do have not to cancel the docker container.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r886582010
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/resources/docker-compose.yml:
##########
@@ -0,0 +1,28 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+version: "3"
+
+services:
+ mlflow-model:
+ image: "${imageName}"
+ ports:
+ - "${deployPort}:8080"
+ deploy:
+ resources:
+ limits:
+ cpus: "${cpuLimit}"
+ memory: "${memoryLimit}"
Review Comment:
> I think your point is reasonable in most tasks. But in the model deployment task, I think state means something like this:
Success: Successfully publish the model.
Running: Deploying the model. In this case, In this case, it means creating the Docker and running the Docker container
Failure: Deployment failure. In this case, maybe an error in the command from MLflow docker build or an error in run command
I agree with the running part, if we should monitor the deployment docker container for it lifecycle, the deployment task should not ending, I think it is a little odd for users about "why my deployment task never end". I think the best way to do that is we only make sure the docker container up and start to service (to have specific logs in docker container).
BTW, I have to mention is the MlFlow allow users write same users define function and start service, when users write some error, it should certainly cause error, and sometime may exit the container, But IMO I think it is more like user herself issue instead of DolphinSchekduler's
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r890847472
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/java/org/apache/dolphinscheduler/plugin/task/mlflow/MlflowConstants.java:
##########
@@ -86,9 +88,17 @@ 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 DOCKER_COMPOSE_CANCEL = "docker compose down";
+
+ public static final String SET_DOCKER_COMPOSE_ENV = "export imageName=%s\n" +
+ "export deployPort=%s\n" +
+ "export cpuLimit=%s\n" +
+ "export memoryLimit=%s";
Review Comment:
Thanks for your suggestion. I've fixed it.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1150646939
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10217)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL) [45 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=new_coverage&view=list) [11.6% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=new_duplicated_lines_density&view=list) [1.1% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r893087528
##########
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've changed 1 minute to 5 minuites.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS merged pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
SbloodyS merged PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r883327164
##########
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:
I think our scenario is just for releasing the model online, and the subsequent operations are not the concern of this component. So, we should not remove the container.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1135375135
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10217)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=duplicated_lines_density&view=list) No Duplication information
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r885628160
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/resources/docker-compose.yml:
##########
@@ -0,0 +1,28 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+version: "3"
+
+services:
+ mlflow-model:
+ image: "${imageName}"
+ ports:
+ - "${deployPort}:8080"
+ deploy:
+ resources:
+ limits:
+ cpus: "${cpuLimit}"
+ memory: "${memoryLimit}"
Review Comment:
<img width="767" alt="image" src="https://user-images.githubusercontent.com/31528124/171181865-dd972e09-8f0a-4aae-ab34-b028ec4ddaa8.png">
Is it ok if I implement it like this?
I think the last feature(deployment with DOCKER RUN) of MLflow had the same problem. If I can fix it like it, i will fix the previous feature using the same method.@SbloodyS
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r885439972
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/resources/docker-compose.yml:
##########
@@ -0,0 +1,28 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+version: "3"
+
+services:
+ mlflow-model:
+ image: "${imageName}"
+ ports:
+ - "${deployPort}:8080"
+ deploy:
+ resources:
+ limits:
+ cpus: "${cpuLimit}"
+ memory: "${memoryLimit}"
Review Comment:
I'm not familiar about ML. I'm just asking the usual questions questions.
The command ```docker run``` sometimes would not throw exception if the docker container successfully start but healthcheck failed. After several runs it may cause many orphan containers in the user's host machine and it can't be known by ds or removed by ds. The users can only view the corresponding tasks through ```SSH``` on the worker's machine. Does this lose some features of DS? @jieguangzhou
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/resources/docker-compose.yml:
##########
@@ -0,0 +1,28 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+version: "3"
+
+services:
+ mlflow-model:
+ image: "${imageName}"
+ ports:
+ - "${deployPort}:8080"
+ deploy:
+ resources:
+ limits:
+ cpus: "${cpuLimit}"
+ memory: "${memoryLimit}"
Review Comment:
I'm not familiar about ML. I'm just asking the usual questions.
The command ```docker run``` sometimes would not throw exception if the docker container successfully start but healthcheck failed. After several runs it may cause many orphan containers in the user's host machine and it can't be known by ds or removed by ds. The users can only view the corresponding tasks through ```SSH``` on the worker's machine. Does this lose some features of DS? @jieguangzhou
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r884584907
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/resources/docker-compose.yml:
##########
@@ -0,0 +1,28 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+version: "3"
+
+services:
+ mlflow-model:
+ image: "${imageName}"
+ ports:
+ - "${deployPort}:8080"
+ deploy:
+ resources:
+ limits:
+ cpus: "${cpuLimit}"
+ memory: "${memoryLimit}"
Review Comment:
> I think adding another MLOps tool for Monitor is the better way.
It's better if we can monitor it.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1148284476
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10217)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=duplicated_lines_density&view=list) No Duplication information
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1136778678
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10217)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=new_coverage&view=list) [70.4% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r882382374
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/resources/docker-compose.yml:
##########
@@ -0,0 +1,28 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+version: "3"
+
+services:
+ mlflow-model:
+ image: "${imageName}"
+ ports:
+ - "${deployPort}:8080"
+ deploy:
+ resources:
+ limits:
+ cpus: "${cpuLimit}"
+ memory: "${memoryLimit}"
Review Comment:
I think we should add healthcheck here and use some methods such as ```https://www.testcontainers.org/``` that can be monitored. So that we can ensure the health of the container before performing subsequent operations.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r882267962
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/java/org/apache/dolphinscheduler/plugin/task/mlflow/MlflowConstants.java:
##########
@@ -86,9 +88,17 @@ 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 DOCKER_COMPOSE_CANCEL = "docker compose down";
Review Comment:
I think that should be "docker rm -f container-name"? But when we start the container, the task status will be changed to success, we can not cancel the task running.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r882261004
##########
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:
did you task cancel the task locally? I find out your task starts with `docker up -d` and it not alway running task which I think it should not be cancel
##########
docs/docs/en/guide/task/mlflow.md:
##########
@@ -92,71 +95,65 @@ First, introduce some general parameters of DolphinScheduler
**Task Parameter**
-- **mlflow server tracking uri** ：MLflow server uri, default http://localhost:5000.
-- **experiment name** ：Create the experiment where the task is running, if the experiment does not exist. If the name is empty, it is set to ` Default `, the same as MLflow.
-- **register model** ：Register the model or not. If register is selected, the following parameters are expanded.
- - **model name** : The registered model name is added to the original model version and registered as
+- **Register Model**: Register the model or not. If register is selected, the following parameters are expanded.
+ - **model name**: The registered model name is added to the original model version and registered as
Production.
-- **data path** : The absolute path of the file or folder. Ends with .csv for file or contain train.csv and
- test.csv for folder（In the suggested way, users should build their own test sets for model evaluation）。
-- **parameters** : Parameter when initializing the algorithm/AutoML model, which can be empty. For example
- parameters `n_estimators=200;learning_rate=0.2` for flaml 。The convention will be passed with '; 'shards
+- **Data Path**: The absolute path of the file or folder. Ends with .csv for file or contain train.csv and
+ test.csv for folder(In the suggested way, users should build their own test sets for model evaluation).
+- **Parameters**: Parameter when initializing the algorithm/AutoML model, which can be empty. For example
+ parameters `n_estimators=200;learning_rate=0.2` for flaml. The convention will be passed with '; 'shards
each parameter, using the name before the equal sign as the parameter name, and using the name after the equal
sign to get the corresponding parameter value through `python eval()`. The detailed parameter list is as follows:
- [flaml](https://microsoft.github.io/FLAML/docs/reference/automl#automl-objects)
- [autosklearn](https://automl.github.io/auto-sklearn/master/api.html)
-- **AutoML tool** : The AutoML tool used, currently
+- **AutoML tool**: The AutoML tool used, currently
supports [autosklearn](https://github.com/automl/auto-sklearn)
- and [flaml](https://github.com/microsoft/FLAML)
+ and [flaml](https://github.com/microsoft/FLAML).
Review Comment:
```suggestion
```
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r883501707
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/resources/docker-compose.yml:
##########
@@ -0,0 +1,28 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+version: "3"
+
+services:
+ mlflow-model:
+ image: "${imageName}"
+ ports:
+ - "${deployPort}:8080"
+ deploy:
+ resources:
+ limits:
+ cpus: "${cpuLimit}"
+ memory: "${memoryLimit}"
Review Comment:
the bad news is currently we do not have a monitor for the MLOPS task
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1138119777
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10217)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=new_coverage&view=list) [70.4% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r883659999
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/resources/docker-compose.yml:
##########
@@ -0,0 +1,28 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+version: "3"
+
+services:
+ mlflow-model:
+ image: "${imageName}"
+ ports:
+ - "${deployPort}:8080"
+ deploy:
+ resources:
+ limits:
+ cpus: "${cpuLimit}"
+ memory: "${memoryLimit}"
Review Comment:
I think adding another MLOps tool for Monitor is the better way.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] kezhenxu94 commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r888955693
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/java/org/apache/dolphinscheduler/plugin/task/mlflow/MlflowConstants.java:
##########
@@ -86,9 +88,17 @@ 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 DOCKER_COMPOSE_CANCEL = "docker compose down";
+
+ public static final String SET_DOCKER_COMPOSE_ENV = "export imageName=%s\n" +
+ "export deployPort=%s\n" +
+ "export cpuLimit=%s\n" +
+ "export memoryLimit=%s";
Review Comment:
Generally environment names should be all capitalized, and please add reasonable prefix (like `DS_TASK_MLFLOW_`) to avoid possible conflicts
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r885425912
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/resources/docker-compose.yml:
##########
@@ -0,0 +1,28 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+version: "3"
+
+services:
+ mlflow-model:
+ image: "${imageName}"
+ ports:
+ - "${deployPort}:8080"
+ deploy:
+ resources:
+ limits:
+ cpus: "${cpuLimit}"
+ memory: "${memoryLimit}"
Review Comment:
I think your point is reasonable in most tasks. But in the model deployment task, I think state means something like this:
Success: Successfully publish the model.
Running: Deploying the model. In this case, In this case, it means creating the Docker and running the Docker container
Failure: Deployment failure. In this case, maybe an error in the command from MLflow docker build or an error in run $docker compose$ command
So, If the model deployment is finished, we don't need to track the state anymore because it's done @SbloodyS @zhongjiajie
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r885269526
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/resources/docker-compose.yml:
##########
@@ -0,0 +1,28 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+version: "3"
+
+services:
+ mlflow-model:
+ image: "${imageName}"
+ ports:
+ - "${deployPort}:8080"
+ deploy:
+ resources:
+ limits:
+ cpus: "${cpuLimit}"
+ memory: "${memoryLimit}"
Review Comment:
It seems is not only the the mlops task issue is about all docker running task issue, So I think maybe we could add a new issue to track the process, about how to monitor the docker type task running, WDYT @SbloodyS
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r892137063
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/resources/docker-compose.yml:
##########
@@ -0,0 +1,28 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+version: "3"
+
+services:
+ mlflow-model:
+ image: "${imageName}"
+ ports:
+ - "${deployPort}:8080"
+ deploy:
+ resources:
+ limits:
+ cpus: "${cpuLimit}"
+ memory: "${memoryLimit}"
Review Comment:
Thank you for your advice. I've added a health check inside the docker container and also added a minute healthy outside the container until the container status becomes healthy. If not healthy, the state in DS will be set as Failure.
@zhongjiajie @SbloodyS
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r892346080
##########
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:
Or we can set the timeout to a long time even forever If the parameters of `Timeout failure` are compatible with this approach.
<img width="501" alt="image" src="https://user-images.githubusercontent.com/31528124/172624990-7c5c6dbc-4ea5-4987-a15d-955c2dabcd65.png">
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r892346080
##########
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:
Or we can set the timeout to forever If the parameters of `Timeout failure` are compatible with this approach.
<img width="501" alt="image" src="https://user-images.githubusercontent.com/31528124/172624990-7c5c6dbc-4ea5-4987-a15d-955c2dabcd65.png">
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1150691219
@zhongjiajie @SbloodyS @songjianet PTAL
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r892346080
##########
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:
Or we can set the timeout to forever If the parameters of Timeout failure are compatible with this approach.
<img width="501" alt="image" src="https://user-images.githubusercontent.com/31528124/172624990-7c5c6dbc-4ea5-4987-a15d-955c2dabcd65.png">
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r892346080
##########
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:
Or we can set the timeout to forever If the parameters of timeout failure are compatible with this approach.
<img width="501" alt="image" src="https://user-images.githubusercontent.com/31528124/172624990-7c5c6dbc-4ea5-4987-a15d-955c2dabcd65.png">
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1151846334
> +1 @zhongjiajie Do you have better suggestions?
I do not have
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] kezhenxu94 commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r888950442
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/java/org/apache/dolphinscheduler/plugin/task/mlflow/MlflowConstants.java:
##########
@@ -86,9 +88,17 @@ 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 DOCKER_COMPOSE_CANCEL = "docker compose down";
Review Comment:
> I think that should be "docker rm -f container-name"? But when we start the container, the task status will be changed to success, we can not cancel the task running.
You need `docker stop container-name` and then `docker rm container-name`, but `docker compose down` should be used here, it will remove all related resources (although you don't have those for now, but it's a good practice to use `docker compose down` if you use `docker compose up` to start the containers..
And I think @caishunfeng 's meaning is `docker-compose down`, note the dash (`-`) between `docker` and `compose`, `docker compose` is new recently in some newer Docker version so it might fail if users are running old Docker version
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1141640391
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10217)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10217&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=new_coverage&view=list) [95.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10217&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r885285721
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/resources/docker-compose.yml:
##########
@@ -0,0 +1,28 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+version: "3"
+
+services:
+ mlflow-model:
+ image: "${imageName}"
+ ports:
+ - "${deployPort}:8080"
+ deploy:
+ resources:
+ limits:
+ cpus: "${cpuLimit}"
+ memory: "${memoryLimit}"
Review Comment:
> I think we should add healthcheck here and use some methods such as `https://www.testcontainers.org/` that can be monitored. So that we can ensure the health of the container before performing subsequent operations.
Currently ```E2E``` using this package to start ```docker container``` from ```docker-compose.yaml``` and monitor it and destory it after all is done.
From the perspective of users using DS. Users do not pay attention to the task type. What they expect is that all task types have the same functionality, including stopping tasks. If the start, stop and running status of containers are not monitored, it is likely that abnormal debug caused by user environment problems will be very difficult. At the same time, it may also cause many orphan containers to affect the user's docker environment.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#issuecomment-1136009739
PTAL @songjianet @zhongjiajie
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] jieguangzhou commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
jieguangzhou commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r884632858
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-mlflow/src/main/resources/docker-compose.yml:
##########
@@ -0,0 +1,28 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+version: "3"
+
+services:
+ mlflow-model:
+ image: "${imageName}"
+ ports:
+ - "${deployPort}:8080"
+ deploy:
+ resources:
+ limits:
+ cpus: "${cpuLimit}"
+ memory: "${memoryLimit}"
Review Comment:
But I think it's reasonable to add more parameters if we need to start the Docker container
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r882269963
##########
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
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] Tianqi-Dotes commented on a diff in pull request #10217: [Feature] [MLOps] support mlflow deploy with docker compose
Posted by GitBox <gi...@apache.org>.
Tianqi-Dotes commented on code in PR #10217:
URL: https://github.com/apache/dolphinscheduler/pull/10217#discussion_r881088536
##########
docs/docs/en/guide/task/mlflow.md:
##########
@@ -20,7 +20,7 @@ The Mlflow plugin currently supports and will support the following:
- [ ] MLflow Models
- [x] MLFLOW: Use `MLflow models serve` to deploy a model service
- [x] Docker: Run the container after packaging the docker image
- - [ ] Docker Compose: Use docker compose to run the container, Will replace the docker run above
+ - [x] Docker Compose: Use docker compose to run the container, Will replace the docker run above
Review Comment:
Docker Compose: Use docker compose to run the container, Will replace the docker run above
-》
Docker Compose: Use docker compose to run the container, it will replace the docker run above
##########
docs/docs/en/guide/task/mlflow.md:
##########
@@ -150,6 +150,16 @@ The actual interface is as follows
- **model-uri** ：Model-uri of mlflow , support `models:/<model_name>/suffix` format and `runs:/` format. See https://mlflow.org/docs/latest/tracking.html#artifact-stores
- **Port** ：The port to listen on
+#### DOCKER COMPOSE
+
+
+
+- **mlflow server tracking uri** ：MLflow server uri, default http://localhost:5000.
Review Comment:
- **mlflow server tracking uri** ：MLflow server uri, default http://localhost:5000.
- ->
- **MLflow Server Tracking URI**: MLflow server URI, default is `http://localhost:5000`.
please replace and paste my corrections
##########
docs/docs/en/guide/task/mlflow.md:
##########
@@ -20,7 +20,7 @@ The Mlflow plugin currently supports and will support the following:
- [ ] MLflow Models
Review Comment:
please make corrections to all the `mlflow` to `MLflow`
##########
docs/docs/en/guide/task/mlflow.md:
##########
@@ -150,6 +150,16 @@ The actual interface is as follows
- **model-uri** ：Model-uri of mlflow , support `models:/<model_name>/suffix` format and `runs:/` format. See https://mlflow.org/docs/latest/tracking.html#artifact-stores
- **Port** ：The port to listen on
+#### DOCKER COMPOSE
+
+
+
+- **mlflow server tracking uri** ：MLflow server uri, default http://localhost:5000.
+- **model-uri** ：Model-uri of mlflow , support `models:/<model_name>/suffix` format and `runs:/` format. See https://mlflow.org/docs/latest/tracking.html#artifact-stores
Review Comment:
- **model-uri**
->
- **Model-uri** or
- **Model-URI**
##########
docs/docs/en/guide/task/mlflow.md:
##########
@@ -150,6 +150,16 @@ The actual interface is as follows
- **model-uri** ：Model-uri of mlflow , support `models:/<model_name>/suffix` format and `runs:/` format. See https://mlflow.org/docs/latest/tracking.html#artifact-stores
- **Port** ：The port to listen on
+#### DOCKER COMPOSE
+
+
+
+- **mlflow server tracking uri** ：MLflow server uri, default http://localhost:5000.
+- **model-uri** ：Model-uri of mlflow , support `models:/<model_name>/suffix` format and `runs:/` format. See https://mlflow.org/docs/latest/tracking.html#artifact-stores
+- **Port** ：The port to listen on
+- **max cpu limit** ：For example `1.0` or `0.5`, the same as docker compose.
Review Comment:
- **max cpu limit**
- ->
- **Max CPU Limit**
##########
docs/docs/en/guide/task/mlflow.md:
##########
@@ -150,6 +150,16 @@ The actual interface is as follows
- **model-uri** ：Model-uri of mlflow , support `models:/<model_name>/suffix` format and `runs:/` format. See https://mlflow.org/docs/latest/tracking.html#artifact-stores
- **Port** ：The port to listen on
+#### DOCKER COMPOSE
+
+
+
+- **mlflow server tracking uri** ：MLflow server uri, default http://localhost:5000.
+- **model-uri** ：Model-uri of mlflow , support `models:/<model_name>/suffix` format and `runs:/` format. See https://mlflow.org/docs/latest/tracking.html#artifact-stores
+- **Port** ：The port to listen on
+- **max cpu limit** ：For example `1.0` or `0.5`, the same as docker compose.
+- **max memory limit** ：For example `1G` or `500M`, the same as docker compose.
Review Comment:
- **max memory limit**
- >
- **Max Memory Limit**
##########
docs/docs/en/guide/task/mlflow.md:
##########
@@ -150,6 +150,16 @@ The actual interface is as follows
- **model-uri** ：Model-uri of mlflow , support `models:/<model_name>/suffix` format and `runs:/` format. See https://mlflow.org/docs/latest/tracking.html#artifact-stores
- **Port** ：The port to listen on
+#### DOCKER COMPOSE
+
+
+
+- **mlflow server tracking uri** ：MLflow server uri, default http://localhost:5000.
Review Comment:
please replace all the '：' to ':'
##########
docs/docs/en/guide/task/mlflow.md:
##########
@@ -150,6 +150,16 @@ The actual interface is as follows
- **model-uri** ：Model-uri of mlflow , support `models:/<model_name>/suffix` format and `runs:/` format. See https://mlflow.org/docs/latest/tracking.html#artifact-stores
- **Port** ：The port to listen on
+#### DOCKER COMPOSE
+
+
+
Review Comment:
please add '.' at the end of the bullets here, or remove all the ',' here.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org