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/08/06 15:37:46 UTC
[GitHub] [dolphinscheduler] CFlyGoo opened a new pull request, #11334: [Feature-10562][Task Plugin] Add local env and global env priority support
CFlyGoo opened a new pull request, #11334:
URL: https://github.com/apache/dolphinscheduler/pull/11334
<!--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
<!--(For example: This pull request adds checkstyle plugin).-->
* Load environment config and env file at the same time.
* The priority of the environment config is higher than the priority of the env file. When an environment variable does not exist in the environment config, it will be supplemented by the env file (if it exists).
* The environment config can also overwrite/get the environment variables existing in the env file.
This close #10562
## Brief change log
<!--*(for example:)*
- *Add maven-checkstyle-plugin to root pom.xml*
-->
Add local env and global env priority support.
## Verify this pull request
This pull request is code cleanup without any test coverage.
--
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] CFlyGoo commented on a diff in pull request #11334: [Feature-10562][Task Plugin] Add local env and global env priority support
Posted by GitBox <gi...@apache.org>.
CFlyGoo commented on code in PR #11334:
URL: https://github.com/apache/dolphinscheduler/pull/11334#discussion_r940851499
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/ShellCommandExecutor.java:
##########
@@ -85,39 +85,37 @@ protected void createCommandFileIfNotExists(String execCommand, String commandFi
logger.info("tenantCode user:{}, task dir:{}", taskRequest.getTenantCode(),
taskRequest.getTaskAppId());
- // create if non existence
- if (!Files.exists(Paths.get(commandFile))) {
- logger.info("create command file:{}", commandFile);
-
- StringBuilder sb = new StringBuilder();
- if (SystemUtils.IS_OS_WINDOWS) {
- sb.append("@echo off\n");
- sb.append("cd /d %~dp0\n");
- if (!Strings.isNullOrEmpty(taskRequest.getEnvironmentConfig())) {
- sb.append(taskRequest.getEnvironmentConfig()).append("\n");
- } else {
- if (taskRequest.getEnvFile() != null) {
- sb.append("call ").append(taskRequest.getEnvFile()).append("\n");
- }
- }
- } else {
- sb.append("#!/bin/sh\n");
- sb.append("BASEDIR=$(cd `dirname $0`; pwd)\n");
- sb.append("cd $BASEDIR\n");
- if (!Strings.isNullOrEmpty(taskRequest.getEnvironmentConfig())) {
- sb.append(taskRequest.getEnvironmentConfig()).append("\n");
- } else {
- if (taskRequest.getEnvFile() != null) {
- sb.append("source ").append(taskRequest.getEnvFile()).append("\n");
- }
- }
- }
- sb.append(execCommand);
- logger.info("command : {}", sb);
-
- // write data to file
- FileUtils.writeStringToFile(new File(commandFile), sb.toString(), StandardCharsets.UTF_8);
+ if (Files.exists(Paths.get(commandFile))) {
+ return;
Review Comment:
This is old logic to avoid duplicating the creation of command file, such as rerun the task. So I think it should be kept. Will this have any side effects? @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 #11334: [Feature-10562][Task Plugin] Add local env and global env priority support
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #11334:
URL: https://github.com/apache/dolphinscheduler/pull/11334#discussion_r940817591
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/ShellCommandExecutor.java:
##########
@@ -85,39 +85,37 @@ protected void createCommandFileIfNotExists(String execCommand, String commandFi
logger.info("tenantCode user:{}, task dir:{}", taskRequest.getTenantCode(),
taskRequest.getTaskAppId());
- // create if non existence
- if (!Files.exists(Paths.get(commandFile))) {
- logger.info("create command file:{}", commandFile);
-
- StringBuilder sb = new StringBuilder();
- if (SystemUtils.IS_OS_WINDOWS) {
- sb.append("@echo off\n");
- sb.append("cd /d %~dp0\n");
- if (!Strings.isNullOrEmpty(taskRequest.getEnvironmentConfig())) {
- sb.append(taskRequest.getEnvironmentConfig()).append("\n");
- } else {
- if (taskRequest.getEnvFile() != null) {
- sb.append("call ").append(taskRequest.getEnvFile()).append("\n");
- }
- }
- } else {
- sb.append("#!/bin/sh\n");
- sb.append("BASEDIR=$(cd `dirname $0`; pwd)\n");
- sb.append("cd $BASEDIR\n");
- if (!Strings.isNullOrEmpty(taskRequest.getEnvironmentConfig())) {
- sb.append(taskRequest.getEnvironmentConfig()).append("\n");
- } else {
- if (taskRequest.getEnvFile() != null) {
- sb.append("source ").append(taskRequest.getEnvFile()).append("\n");
- }
- }
- }
- sb.append(execCommand);
- logger.info("command : {}", sb);
-
- // write data to file
- FileUtils.writeStringToFile(new File(commandFile), sb.toString(), StandardCharsets.UTF_8);
+ if (Files.exists(Paths.get(commandFile))) {
+ return;
Review Comment:
This does not realize what the comments says in https://github.com/apache/dolphinscheduler/issues/10562#issuecomment-1175877731
--
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 #11334: [Feature-10562][Task Plugin] Add local env and global env priority support
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #11334:
URL: https://github.com/apache/dolphinscheduler/pull/11334#discussion_r945409938
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/ShellCommandExecutor.java:
##########
@@ -85,39 +85,37 @@ protected void createCommandFileIfNotExists(String execCommand, String commandFi
logger.info("tenantCode user:{}, task dir:{}", taskRequest.getTenantCode(),
taskRequest.getTaskAppId());
- // create if non existence
- if (!Files.exists(Paths.get(commandFile))) {
- logger.info("create command file:{}", commandFile);
-
- StringBuilder sb = new StringBuilder();
- if (SystemUtils.IS_OS_WINDOWS) {
- sb.append("@echo off\n");
- sb.append("cd /d %~dp0\n");
- if (!Strings.isNullOrEmpty(taskRequest.getEnvironmentConfig())) {
- sb.append(taskRequest.getEnvironmentConfig()).append("\n");
- } else {
- if (taskRequest.getEnvFile() != null) {
- sb.append("call ").append(taskRequest.getEnvFile()).append("\n");
- }
- }
- } else {
- sb.append("#!/bin/sh\n");
- sb.append("BASEDIR=$(cd `dirname $0`; pwd)\n");
- sb.append("cd $BASEDIR\n");
- if (!Strings.isNullOrEmpty(taskRequest.getEnvironmentConfig())) {
- sb.append(taskRequest.getEnvironmentConfig()).append("\n");
- } else {
- if (taskRequest.getEnvFile() != null) {
- sb.append("source ").append(taskRequest.getEnvFile()).append("\n");
- }
- }
- }
- sb.append(execCommand);
- logger.info("command : {}", sb);
-
- // write data to file
- FileUtils.writeStringToFile(new File(commandFile), sb.toString(), StandardCharsets.UTF_8);
+ if (Files.exists(Paths.get(commandFile))) {
+ return;
Review Comment:
> This is old logic to avoid duplicating the creation of command file, such as rerun the task. So I think it should be kept. Will this have any side effects? @SbloodyS
If the user modifies the contents of the command and rerun the task. This will cause an execution exception.
--
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] CFlyGoo commented on pull request #11334: [Feature-10562][Task Plugin] Add local env and global env priority support
Posted by GitBox <gi...@apache.org>.
CFlyGoo commented on PR #11334:
URL: https://github.com/apache/dolphinscheduler/pull/11334#issuecomment-1214582564
Without `Environment Config`
Load both `dolphinscheduler_env.sh` and `Environment Config`
The above picture is the log screenshot of my local test.
The `dolphinscheduler_env.sh` is always loaded as a `global `. If the `Environment config` is exists, the `Environment config` is loaded as a `local variable`. `Environment Config` always can gets/overrides configuration in `dolphinscheduer_env.sh`.
Isn't this the expected effect of #10562? @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] CFlyGoo commented on a diff in pull request #11334: [Feature-10562][Task Plugin] Add local env and global env priority support
Posted by GitBox <gi...@apache.org>.
CFlyGoo commented on code in PR #11334:
URL: https://github.com/apache/dolphinscheduler/pull/11334#discussion_r940851499
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/ShellCommandExecutor.java:
##########
@@ -85,39 +85,37 @@ protected void createCommandFileIfNotExists(String execCommand, String commandFi
logger.info("tenantCode user:{}, task dir:{}", taskRequest.getTenantCode(),
taskRequest.getTaskAppId());
- // create if non existence
- if (!Files.exists(Paths.get(commandFile))) {
- logger.info("create command file:{}", commandFile);
-
- StringBuilder sb = new StringBuilder();
- if (SystemUtils.IS_OS_WINDOWS) {
- sb.append("@echo off\n");
- sb.append("cd /d %~dp0\n");
- if (!Strings.isNullOrEmpty(taskRequest.getEnvironmentConfig())) {
- sb.append(taskRequest.getEnvironmentConfig()).append("\n");
- } else {
- if (taskRequest.getEnvFile() != null) {
- sb.append("call ").append(taskRequest.getEnvFile()).append("\n");
- }
- }
- } else {
- sb.append("#!/bin/sh\n");
- sb.append("BASEDIR=$(cd `dirname $0`; pwd)\n");
- sb.append("cd $BASEDIR\n");
- if (!Strings.isNullOrEmpty(taskRequest.getEnvironmentConfig())) {
- sb.append(taskRequest.getEnvironmentConfig()).append("\n");
- } else {
- if (taskRequest.getEnvFile() != null) {
- sb.append("source ").append(taskRequest.getEnvFile()).append("\n");
- }
- }
- }
- sb.append(execCommand);
- logger.info("command : {}", sb);
-
- // write data to file
- FileUtils.writeStringToFile(new File(commandFile), sb.toString(), StandardCharsets.UTF_8);
+ if (Files.exists(Paths.get(commandFile))) {
+ return;
Review Comment:
This is old logic to avoid duplicating the creation of command file, such as rerun the task. So I think it should be kept.
--
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 #11334: [Feature-10562][Task Plugin] Add local env and global env priority support
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11334:
URL: https://github.com/apache/dolphinscheduler/pull/11334#issuecomment-1207583072
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11334)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11334&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11334&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11334&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11334&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] codecov-commenter commented on pull request #11334: [Feature-10562][Task Plugin] Add local env and global env priority support
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #11334:
URL: https://github.com/apache/dolphinscheduler/pull/11334#issuecomment-1207579055
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/11334?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 [#11334](https://codecov.io/gh/apache/dolphinscheduler/pull/11334?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (225fd1d) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/3aa9f2ea25ca42112141aad85140a72b0963e2c3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3aa9f2e) will **decrease** coverage by `0.02%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## dev #11334 +/- ##
============================================
- Coverage 40.26% 40.24% -0.03%
+ Complexity 4843 4839 -4
============================================
Files 974 974
Lines 37323 37315 -8
Branches 4142 4133 -9
============================================
- Hits 15029 15016 -13
- Misses 20748 20751 +3
- Partials 1546 1548 +2
```
| [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/11334?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...cheduler/plugin/task/api/ShellCommandExecutor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11334/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-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi90YXNrL2FwaS9TaGVsbENvbW1hbmRFeGVjdXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...erver/master/processor/queue/TaskEventService.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11334/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-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9wcm9jZXNzb3IvcXVldWUvVGFza0V2ZW50U2VydmljZS5qYXZh) | `69.64% <0.00%> (-10.72%)` | :arrow_down: |
| [...er/master/dispatch/host/assign/RandomSelector.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11334/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-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9kaXNwYXRjaC9ob3N0L2Fzc2lnbi9SYW5kb21TZWxlY3Rvci5qYXZh) | `77.77% <0.00%> (-5.56%)` | :arrow_down: |
| [...e/dolphinscheduler/remote/NettyRemotingClient.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11334/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==) | `50.00% <0.00%> (-2.78%)` | :arrow_down: |
| [...r/plugin/task/sqoop/parameter/SqoopParameters.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11334/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-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stc3Fvb3Avc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL3Rhc2svc3Fvb3AvcGFyYW1ldGVyL1Nxb29wUGFyYW1ldGVycy5qYXZh) | `55.12% <0.00%> (-1.29%)` | :arrow_down: |
| [...rver/master/runner/task/BlockingTaskProcessor.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11334/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-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9ydW5uZXIvdGFzay9CbG9ja2luZ1Rhc2tQcm9jZXNzb3IuamF2YQ==) | `75.60% <0.00%> (-0.30%)` | :arrow_down: |
| [...eduler/api/service/impl/DataSourceServiceImpl.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11334/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-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvaW1wbC9EYXRhU291cmNlU2VydmljZUltcGwuamF2YQ==) | `28.47% <0.00%> (ø)` | |
| [.../dolphinscheduler/plugin/task/datax/DataxTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11334/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-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stZGF0YXgvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL3Rhc2svZGF0YXgvRGF0YXhUYXNrLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...phinscheduler/plugin/task/chunjun/ChunJunTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11334/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-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stY2h1bmp1bi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZG9scGhpbnNjaGVkdWxlci9wbHVnaW4vdGFzay9jaHVuanVuL0NodW5KdW5UYXNrLmphdmE=) | `0.00% <0.00%> (ø)` | |
| ... and [3 more](https://codecov.io/gh/apache/dolphinscheduler/pull/11334/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: Codecov can now indicate which changes are the most critical in Pull Requests. [Learn more](https://about.codecov.io/product/feature/runtime-insights/?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] CFlyGoo commented on pull request #11334: [Feature-10562][Task Plugin] Add local env and global env priority support
Posted by GitBox <gi...@apache.org>.
CFlyGoo commented on PR #11334:
URL: https://github.com/apache/dolphinscheduler/pull/11334#issuecomment-1208756780
Hi @caishunfeng @SbloodyS @zhuangchong. This PR is ready, 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] CFlyGoo commented on a diff in pull request #11334: [Feature-10562][Task Plugin] Add local env and global env priority support
Posted by GitBox <gi...@apache.org>.
CFlyGoo commented on code in PR #11334:
URL: https://github.com/apache/dolphinscheduler/pull/11334#discussion_r946258750
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/ShellCommandExecutor.java:
##########
@@ -85,39 +85,37 @@ protected void createCommandFileIfNotExists(String execCommand, String commandFi
logger.info("tenantCode user:{}, task dir:{}", taskRequest.getTenantCode(),
taskRequest.getTaskAppId());
- // create if non existence
- if (!Files.exists(Paths.get(commandFile))) {
- logger.info("create command file:{}", commandFile);
-
- StringBuilder sb = new StringBuilder();
- if (SystemUtils.IS_OS_WINDOWS) {
- sb.append("@echo off\n");
- sb.append("cd /d %~dp0\n");
- if (!Strings.isNullOrEmpty(taskRequest.getEnvironmentConfig())) {
- sb.append(taskRequest.getEnvironmentConfig()).append("\n");
- } else {
- if (taskRequest.getEnvFile() != null) {
- sb.append("call ").append(taskRequest.getEnvFile()).append("\n");
- }
- }
- } else {
- sb.append("#!/bin/sh\n");
- sb.append("BASEDIR=$(cd `dirname $0`; pwd)\n");
- sb.append("cd $BASEDIR\n");
- if (!Strings.isNullOrEmpty(taskRequest.getEnvironmentConfig())) {
- sb.append(taskRequest.getEnvironmentConfig()).append("\n");
- } else {
- if (taskRequest.getEnvFile() != null) {
- sb.append("source ").append(taskRequest.getEnvFile()).append("\n");
- }
- }
- }
- sb.append(execCommand);
- logger.info("command : {}", sb);
-
- // write data to file
- FileUtils.writeStringToFile(new File(commandFile), sb.toString(), StandardCharsets.UTF_8);
+ if (Files.exists(Paths.get(commandFile))) {
+ return;
Review Comment:
> > This is old logic to avoid duplicating the creation of command file, such as rerun the task. So I think it should be kept. Will this have any side effects? @SbloodyS
>
> If the user modifies the contents of the command and rerun the task. This will cause an execution exception.
What type of tasks did you find this problem with? I tested it locally with python task. I tried to modify the content and environment config of the task after it runs successfully, and rerun it. The modified content and environment config can be loaded and run successfully.
--
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
Re: [PR] [Feature-10562][Task Plugin] Add local env and global env priority support [dolphinscheduler]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11334:
URL: https://github.com/apache/dolphinscheduler/pull/11334#issuecomment-1756495138
This pull request has been automatically marked as stale because it has not had recent activity for 120 days. It will be closed in 7 days if no further activity occurs.
--
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 #11334: [Feature-10562][Task Plugin] Add local env and global env priority support
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11334:
URL: https://github.com/apache/dolphinscheduler/pull/11334#issuecomment-1207586226
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11334)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11334&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11334&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11334&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11334&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11334&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
Re: [PR] [Feature-10562][Task Plugin] Add local env and global env priority support [dolphinscheduler]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #11334: [Feature-10562][Task Plugin] Add local env and global env priority support
URL: https://github.com/apache/dolphinscheduler/pull/11334
--
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
Re: [PR] [Feature-10562][Task Plugin] Add local env and global env priority support [dolphinscheduler]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11334:
URL: https://github.com/apache/dolphinscheduler/pull/11334#issuecomment-1767391533
This pull request has been closed because it has not had recent activity. You could reopen it if you try to continue your work, and anyone who are interested in it are encouraged to continue work on this pull request.
--
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