[Feature] Enable users to switch and install conda env in jupyter task #10337

EricGao888 · 2022-06-02T04:43:52Z

Purpose of the pull request

Enable users to switch and install conda env in jupyter task. If the conda environment user needs for his/her jupyter task is not too large like bigger than 100M, the user could upload the packed conda environment to resource center and select it when creating jupyter task, jupyter task plugin will automatically install the conda env on target worker before executing jupyter notes.
Also, users could upload multiple packed conda environments to resource center and select the one needed for a specific task.
This feature will save users quite a lot of time from installing multiple environments on multiple workers and greatly better the experience of using jupyter task plugin

This PR closes: [Feature][Task Plugin] Enable users to switch and install conda env in jupyter task #10302

Brief change log

Already described above.

Verify this pull request

Verified by unit tests and manual tests.

Amy0104

The front end part LGTM.

EricGao888 · 2022-06-06T02:15:41Z

Discussed with @SbloodyS, it seems using source may cause security issue. I'm trying to find a workaround to avoid using source.

SbloodyS · 2022-06-09T08:41:43Z

...-jupyter/src/main/java/org/apache/dolphinscheduler/plugin/task/jupyter/JupyterConstants.java

+            "tar -xzf %s -C jupyter_env && " +
+            "source jupyter_env/bin/activate";


Is it better to let users to define this path if the user's path is different from this?

Currently, env tar will be downloaded from resource center before task execution and removed when task execution completed.

dolphinscheduler/dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/runner/TaskExecuteThread.java

Lines 141 to 145 in f90f0f8

// copy hdfs/minio file to local

List<Pair<String, String>> fileDownloads = downloadCheck(taskExecutionContext.getExecutePath(), taskExecutionContext.getResources());

if (!fileDownloads.isEmpty()){

downloadResource(taskExecutionContext.getExecutePath(), logger, fileDownloads);

}

dolphinscheduler/dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/runner/TaskExecuteThread.java

Lines 199 to 203 in f90f0f8

} finally {

TaskExecutionContextCacheManager.removeByTaskInstanceId(taskExecutionContext.getTaskInstanceId());

taskCallbackService.sendTaskExecuteResponseCommand(taskExecutionContext);

clearTaskExecPath();

}

However, I think it is a good idea to give a choice to define the path and install the env on workers without getting removed after task execution. In this way, the same env will not need to be downloaded every time.

WDYT @SbloodyS @zhongjiajie

It's better. But what i mean is if the user's the compressed tar package path after decompression is inconsistent with this hard code path. If we hard code this. We should put this to the docs.

It's better. But what i mean is if the user's the compressed tar package path after decompression is inconsistent with this hard code path.

Seems if users use conda pack to pack their environment, when they run tar they will get the same directory structure.

I'm not quite sure about it. Could you take a look? @zhongjiajie

It's better. But what i mean is if the user's the compressed tar package path after decompression is inconsistent with this hard code path.

Seems if users use conda pack to pack their environment, when they run tar they will get the same directory structure.

I test locally, and it is 👍. but should we add some docs to tell use run conda pack to create the conda tarball?

It's better. But what i mean is if the user's the compressed tar package path after decompression is inconsistent with this hard code path.

Seems if users use conda pack to pack their environment, when they run tar they will get the same directory structure.

I test locally, and it is 👍. but should we add some docs to tell use run conda pack to create the conda tarball?

@zhongjiajie Sure, that's a good idea. I will add some docs to instruct users on conda pack. BTW, is there any substitute for source jupyter_env/bin/activate? @SbloodyS and I discussed about this and it is possible that some companies ban developers from using source cmd.

However, I think it is a good idea to give a choice to define the path and install the env on workers without getting removed after task execution. In this way, the same env will not need to be downloaded every time.

but I do not think so, I think they can use shell task to do that. It is odd if we do not remove resources after the task execute finish, using two cases:

we only support not deleting resources after task is done: although the worker has already installed the env, but we still have resource configurated in task definition, So it means worker will re-install the conda env to worker. Or maybe users have to change the task definition and remove the resource after the first time they ran the task.(it is odd)

We have given users an option about resources install and delete. then have to change the option after they ran the task first time too.

However, I think it is a good idea to give a choice to define the path and install the env on workers without getting removed after task execution. In this way, the same env will not need to be downloaded every time.

but I do not think so, I think they can use shell task to do that. It is odd if we do not remove resources after the task execute finish, using two cases:

we only support not deleting resources after task is done: although the worker has already installed the env, but we still have resource configurated in task definition, So it means worker will re-install the conda env to worker. Or maybe users have to change the task definition and remove the resource after the first time they ran the task.(it is odd)

We have given users an option about resources install and delete. then have to change the option after they ran the task first time too.

Alright. This makes sense to me. Since the tasks are usually triggered multiple times, considering persisting the env could be complicated and might cause unexpected issues. If users want to persist the env, it is better to use a shell task and only trigger it once at the very beginning.

yup, that what I mean

codecov-commenter · 2022-06-15T04:25:13Z

Codecov Report

Merging #10337 (2720f4d) into dev (4be3b87) will increase coverage by 0.00%.
The diff coverage is 88.88%.

@@            Coverage Diff            @@
##                dev   #10337   +/-   ##
=========================================
  Coverage     40.87%   40.87%           
- Complexity     4851     4853    +2     
=========================================
  Files           886      886           
  Lines         36032    36039    +7     
  Branches       3998     3999    +1     
=========================================
+ Hits          14727    14731    +4     
- Misses        19848    19854    +6     
+ Partials       1457     1454    -3

Impacted Files	Coverage Δ
...cheduler/plugin/task/jupyter/JupyterConstants.java	`0.00% <ø> (ø)`
...heduler/plugin/task/jupyter/JupyterParameters.java	`91.17% <75.00%> (-2.16%)`	⬇️
...phinscheduler/plugin/task/jupyter/JupyterTask.java	`66.66% <100.00%> (+1.19%)`	⬆️
...org/apache/dolphinscheduler/remote/utils/Host.java	`43.47% <0.00%> (-2.18%)`	⬇️
...e/dolphinscheduler/remote/NettyRemotingClient.java	`51.38% <0.00%> (-1.39%)`	⬇️
...er/master/dispatch/host/assign/RandomSelector.java	`83.33% <0.00%> (+5.55%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4be3b87...2720f4d. Read the comment docs.

EricGao888 · 2022-06-15T04:32:41Z

Related docs added in the latest commit : )

zhongjiajie · 2022-06-15T12:35:21Z

and we still have discussion in #10337 (comment), I wonder whether conda have some command like import same as docker's docker import to import and then conda activate?

EricGao888 · 2022-06-15T12:41:56Z

and we still have discussion in #10337 (comment), I wonder whether conda have some command like import same as docker's docker import to import and then conda activate?

I haven't found such things so far. However, using source for unpacked conda env seems to be the only way I could find on Conda-Pack official page.

zhongjiajie · 2022-06-18T05:11:29Z

and we still have discussion in #10337 (comment), I wonder whether conda have some command like import same as docker's docker import to import and then conda activate?

I haven't found such things so far. However, using source for unpacked conda env seems to be the only way I could find on Conda-Pack official page.

Yeah, I can not find either, I think maybe we should use source to activate the env template, WDYT @EricGao888 @SbloodyS

EricGao888 · 2022-06-18T05:18:57Z

and we still have discussion in #10337 (comment), I wonder whether conda have some command like import same as docker's docker import to import and then conda activate?

I haven't found such things so far. However, using source for unpacked conda env seems to be the only way I could find on Conda-Pack official page.

Yeah, I can not find either, I think maybe we should use source to activate the env template, WDYT @EricGao888 @SbloodyS

Yes, IMHO, there is no perfect solution here to avoid using source while providing good user experience at the same time. This is a trade-off. I think we could give user this option but add some notice about source command in docs. If they have concerns about it, they could choose not to use it.

zhongjiajie · 2022-06-18T05:37:11Z

SGTM, So please add some docs to hint users about thesource command

… jupyter task (apache#10302)

…jupyter task (apache#10302)

…in jupyter task plugin (apache#10302)

sonarcloud · 2022-06-18T12:33:58Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

88.9% Coverage
0.0% Duplication

EricGao888 · 2022-06-18T12:38:40Z

SGTM, So please add some docs to hint users about thesource command

Caveats to source command usage have been added in docs in the latest commit, PTAL @zhongjiajie @SbloodyS Thx!

SbloodyS

The backend part LGTM

zhongjiajie

LGTM, and frontend jus simply add resource usage, merge

apache#10337)

EricGao888 force-pushed the Fix-10302 branch from 00cce68 to 542da4a Compare June 2, 2022 04:55

SbloodyS assigned EricGao888 Jun 2, 2022

SbloodyS added UI ui and front end related improvement make more easy to user or prompt friendly backend labels Jun 2, 2022

Amy0104 reviewed Jun 6, 2022

View reviewed changes

EricGao888 force-pushed the Fix-10302 branch from 542da4a to a797850 Compare June 8, 2022 12:29

EricGao888 requested review from songjianet, caishunfeng, SbloodyS and zhuangchong as code owners June 8, 2022 12:29

SbloodyS reviewed Jun 9, 2022

View reviewed changes

EricGao888 force-pushed the Fix-10302 branch from a797850 to a5ece00 Compare June 15, 2022 04:10

EricGao888 requested a review from Tianqi-Dotes as a code owner June 15, 2022 04:10

EricGao888 requested a review from zhongjiajie June 15, 2022 04:33

EricGao888 added 3 commits June 18, 2022 16:29

[Feature][Task Plugin]Enable users to switch and install conda env in…

f4f470a

… jupyter task (apache#10302)

[Feature][Task Plugin] Add docs for python dependency management in …

c06e95c

…jupyter task (apache#10302)

[Feature][Task Plugin] Add notice in docs about source command usage …

2720f4d

…in jupyter task plugin (apache#10302)

EricGao888 force-pushed the Fix-10302 branch from a5ece00 to 2720f4d Compare June 18, 2022 12:12

SbloodyS approved these changes Jun 18, 2022

View reviewed changes

zhongjiajie approved these changes Jun 20, 2022

View reviewed changes

zhongjiajie merged commit 64cee03 into apache:dev Jun 20, 2022

zhongjiajie changed the title ~~[Feature][Task Plugin]Enable users to switch and install conda env in jupyter task~~ [Feature] Enable users to switch and install conda env in jupyter task Jun 20, 2022

zhongjiajie added this to the 3.1.0-alpha milestone Jun 20, 2022

hstdream pushed a commit to hstdream/dolphinscheduler that referenced this pull request Jun 23, 2022

[Feature] Enable users to switch and install conda env in jupyter task (

e085292

apache#10337)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] Enable users to switch and install conda env in jupyter task #10337

[Feature] Enable users to switch and install conda env in jupyter task #10337

EricGao888 commented Jun 2, 2022

Amy0104 left a comment

EricGao888 commented Jun 6, 2022

SbloodyS Jun 9, 2022

EricGao888 Jun 9, 2022

SbloodyS Jun 9, 2022 •

edited

Loading

EricGao888 Jun 9, 2022

SbloodyS Jun 9, 2022

zhongjiajie Jun 14, 2022

EricGao888 Jun 14, 2022

zhongjiajie Jun 14, 2022

EricGao888 Jun 14, 2022

zhongjiajie Jun 15, 2022

codecov-commenter commented Jun 15, 2022 •

edited

Loading

EricGao888 commented Jun 15, 2022

zhongjiajie commented Jun 15, 2022

EricGao888 commented Jun 15, 2022

zhongjiajie commented Jun 18, 2022

EricGao888 commented Jun 18, 2022

zhongjiajie commented Jun 18, 2022

sonarcloud bot commented Jun 18, 2022

EricGao888 commented Jun 18, 2022 •

edited

Loading

SbloodyS left a comment

zhongjiajie left a comment

		"tar -xzf %s -C jupyter_env && " +
		"source jupyter_env/bin/activate";

	// copy hdfs/minio file to local
	List<Pair<String, String>> fileDownloads = downloadCheck(taskExecutionContext.getExecutePath(), taskExecutionContext.getResources());
	if (!fileDownloads.isEmpty()){
	downloadResource(taskExecutionContext.getExecutePath(), logger, fileDownloads);
	}

	} finally {
	TaskExecutionContextCacheManager.removeByTaskInstanceId(taskExecutionContext.getTaskInstanceId());
	taskCallbackService.sendTaskExecuteResponseCommand(taskExecutionContext);
	clearTaskExecPath();
	}

[Feature] Enable users to switch and install conda env in jupyter task #10337

[Feature] Enable users to switch and install conda env in jupyter task #10337

Conversation

EricGao888 commented Jun 2, 2022

Purpose of the pull request

Brief change log

Verify this pull request

Amy0104 left a comment

Choose a reason for hiding this comment

EricGao888 commented Jun 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SbloodyS Jun 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 15, 2022 • edited Loading

Codecov Report

EricGao888 commented Jun 15, 2022

zhongjiajie commented Jun 15, 2022

EricGao888 commented Jun 15, 2022

zhongjiajie commented Jun 18, 2022

EricGao888 commented Jun 18, 2022

zhongjiajie commented Jun 18, 2022

sonarcloud bot commented Jun 18, 2022

EricGao888 commented Jun 18, 2022 • edited Loading

SbloodyS left a comment

Choose a reason for hiding this comment

zhongjiajie left a comment

Choose a reason for hiding this comment

SbloodyS Jun 9, 2022 •

edited

Loading

codecov-commenter commented Jun 15, 2022 •

edited

Loading

EricGao888 commented Jun 18, 2022 •

edited

Loading