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/06/24 02:05:43 UTC

[GitHub] [dolphinscheduler] someorz opened a new pull request, #10586: [Fix-9938][master] Fix 9938 query environment config set to task instance

someorz opened a new pull request, #10586:
URL: https://github.com/apache/dolphinscheduler/pull/10586

   check exists task environment code if not default query environment config set to task instance
   
   <!--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).-->
   
   ## Brief change log
   
   <!--*(for example:)*
     - *Add maven-checkstyle-plugin to root pom.xml*
   -->
   ## 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] caishunfeng commented on a diff in pull request #10586: [Fix-9938][master] Fix 9938 query environment config set to task instance

Posted by GitBox <gi...@apache.org>.
caishunfeng commented on code in PR #10586:
URL: https://github.com/apache/dolphinscheduler/pull/10586#discussion_r905691258


##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/WorkflowExecuteRunnable.java:
##########
@@ -1115,6 +1115,12 @@ private TaskInstance findTaskIfExists(Long taskCode, int taskVersion) {
     private TaskInstance createTaskInstance(ProcessInstance processInstance, TaskNode taskNode) {
         TaskInstance taskInstance = findTaskIfExists(taskNode.getCode(), taskNode.getVersion());
         if (taskInstance != null) {
+            if (!taskInstance.getEnvironmentCode().equals(-1L)) {
+                Environment environment = processService.findEnvironmentByCode(taskInstance.getEnvironmentCode());

Review Comment:
   > This bug will not appear in new tasks. This bug is that existing tasks will appear during fault tolerance. ` TaskInstance taskInstance = findTaskIfExists(taskNode.getCode(), taskNode.getVersion());` findTaskIfExists return instance EnvironmentConfig is null at version 2.0.5 if new version findTaskIfExists method return TaskInstance EnvironmentConfig is not null no need to query again
   
   In this case, we can add this logic into `initTaskQueue`, which will load the valid tasks at first if exist. What do you think?



-- 
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] someorz commented on pull request #10586: [Fix-9938][master] Fix 9938 query environment config set to task instance

Posted by GitBox <gi...@apache.org>.
someorz commented on PR #10586:
URL: https://github.com/apache/dolphinscheduler/pull/10586#issuecomment-1165113852

   @SbloodyS i had associate the issue.
   
   


-- 
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] someorz commented on a diff in pull request #10586: [Fix-9938][master] Fix 9938 query environment config set to task instance

Posted by GitBox <gi...@apache.org>.
someorz commented on code in PR #10586:
URL: https://github.com/apache/dolphinscheduler/pull/10586#discussion_r905693687


##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/WorkflowExecuteRunnable.java:
##########
@@ -1115,6 +1115,12 @@ private TaskInstance findTaskIfExists(Long taskCode, int taskVersion) {
     private TaskInstance createTaskInstance(ProcessInstance processInstance, TaskNode taskNode) {
         TaskInstance taskInstance = findTaskIfExists(taskNode.getCode(), taskNode.getVersion());
         if (taskInstance != null) {
+            if (!taskInstance.getEnvironmentCode().equals(-1L)) {
+                Environment environment = processService.findEnvironmentByCode(taskInstance.getEnvironmentCode());

Review Comment:
   greate is a more elegant solution，i think in baseSql add environment_config column will resolve the problem
   ![image](https://user-images.githubusercontent.com/5456361/175458093-176da91c-9e29-4fc7-8335-aa4fe003483f.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] someorz commented on a diff in pull request #10586: [Fix-9938][master] Fix 9938 query environment config set to task instance

Posted by GitBox <gi...@apache.org>.
someorz commented on code in PR #10586:
URL: https://github.com/apache/dolphinscheduler/pull/10586#discussion_r905682905


##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/WorkflowExecuteRunnable.java:
##########
@@ -1115,6 +1115,12 @@ private TaskInstance findTaskIfExists(Long taskCode, int taskVersion) {
     private TaskInstance createTaskInstance(ProcessInstance processInstance, TaskNode taskNode) {
         TaskInstance taskInstance = findTaskIfExists(taskNode.getCode(), taskNode.getVersion());
         if (taskInstance != null) {
+            if (!taskInstance.getEnvironmentCode().equals(-1L)) {
+                Environment environment = processService.findEnvironmentByCode(taskInstance.getEnvironmentCode());

Review Comment:
   This bug will not appear in new tasks. This bug is that existing tasks will appear during fault tolerance.
   ` TaskInstance taskInstance = findTaskIfExists(taskNode.getCode(), taskNode.getVersion());`
   findTaskIfExists return instance EnvironmentConfig is null at version 2.0.5 if new version findTaskIfExists method return TaskInstance EnvironmentConfig is not null no need to query again



-- 
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 #10586: [Fix-9938][master] Fix 9938 query environment config set to task instance

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #10586:
URL: https://github.com/apache/dolphinscheduler/pull/10586#issuecomment-1165115416

   # [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10586?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 [#10586](https://codecov.io/gh/apache/dolphinscheduler/pull/10586?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (92e8890) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/b2879c0e056b48c37d250b7b93cafdfba4621129?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2879c0) will **decrease** coverage by `0.07%`.
   > The diff coverage is `n/a`.
   
   ```diff
   @@             Coverage Diff              @@
   ##                dev   #10586      +/-   ##
   ============================================
   - Coverage     41.01%   40.93%   -0.08%     
   - Complexity     4874     4876       +2     
   ============================================
     Files           893      895       +2     
     Lines         36120    36216      +96     
     Branches       3992     3989       -3     
   ============================================
   + Hits          14813    14825      +12     
   - Misses        19843    19926      +83     
   - Partials       1464     1465       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/10586?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [.../server/master/runner/WorkflowExecuteRunnable.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10586/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-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9ydW5uZXIvV29ya2Zsb3dFeGVjdXRlUnVubmFibGUuamF2YQ==) | `7.66% <ø> (-0.07%)` | :arrow_down: |
   | [...hinscheduler/service/alert/AlertClientService.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10586/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-ZG9scGhpbnNjaGVkdWxlci1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3NlcnZpY2UvYWxlcnQvQWxlcnRDbGllbnRTZXJ2aWNlLmphdmE=) | `60.00% <0.00%> (-6.67%)` | :arrow_down: |
   | [...r/plugin/task/sqoop/parameter/SqoopParameters.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10586/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) | `53.33% <0.00%> (-1.34%)` | :arrow_down: |
   | [...erver/master/runner/WorkflowExecuteThreadPool.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10586/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-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9ydW5uZXIvV29ya2Zsb3dFeGVjdXRlVGhyZWFkUG9vbC5qYXZh) | `1.31% <0.00%> (-0.12%)` | :arrow_down: |
   | [...org/apache/dolphinscheduler/alert/AlertServer.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10586/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-ZG9scGhpbnNjaGVkdWxlci1hbGVydC9kb2xwaGluc2NoZWR1bGVyLWFsZXJ0LXNlcnZlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZG9scGhpbnNjaGVkdWxlci9hbGVydC9BbGVydFNlcnZlci5qYXZh) | `51.28% <0.00%> (-0.07%)` | :arrow_down: |
   | [.../server/master/runner/StateWheelExecuteThread.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10586/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-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9ydW5uZXIvU3RhdGVXaGVlbEV4ZWN1dGVUaHJlYWQuamF2YQ==) | `0.47% <0.00%> (-0.01%)` | :arrow_down: |
   | [...apache/dolphinscheduler/common/thread/Stopper.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10586/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-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3RocmVhZC9TdG9wcGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...che/dolphinscheduler/common/utils/LoggerUtils.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10586/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-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3V0aWxzL0xvZ2dlclV0aWxzLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...he/dolphinscheduler/common/thread/ThreadUtils.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10586/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-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3RocmVhZC9UaHJlYWRVdGlscy5qYXZh) | `0.00% <0.00%> (ø)` | |
   | ... and [26 more](https://codecov.io/gh/apache/dolphinscheduler/pull/10586/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) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10586?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/10586?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 [b2879c0...92e8890](https://codecov.io/gh/apache/dolphinscheduler/pull/10586?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 pull request #10586: [Fix-9938][master] Fix 9938 query environment config set to task instance

Posted by GitBox <gi...@apache.org>.
SbloodyS commented on PR #10586:
URL: https://github.com/apache/dolphinscheduler/pull/10586#issuecomment-1165107405

   Hi @someorz , please associate the issue.


-- 
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 #10586: [Fix-9938][master] Fix 9938 query environment config set to task instance

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10586:
URL: https://github.com/apache/dolphinscheduler/pull/10586#issuecomment-1165128901

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10586)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10586&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10586&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10586&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10586&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10586&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10586&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10586&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10586&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10586&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10586&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10586&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10586&resolved=false&types=CODE_SMELL)
   
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10586&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10586&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10586&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10586&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] ruanwenjun commented on pull request #10586: [Fix-9938][master] Fix 9938 query environment config set to task instance

Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on PR #10586:
URL: https://github.com/apache/dolphinscheduler/pull/10586#issuecomment-1165141741

   I find we already have this logic in `newTaskInstance`, so there is no need to do this in everywhere?


-- 
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] someorz closed pull request #10586: [Fix-9938][master] Fix 9938 query environment config set to task instance

Posted by GitBox <gi...@apache.org>.
someorz closed pull request #10586: [Fix-9938][master] Fix 9938 query environment config set to task instance
URL: https://github.com/apache/dolphinscheduler/pull/10586


-- 
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 #10586: [Fix-9938][master] Fix 9938 query environment config set to task instance

Posted by GitBox <gi...@apache.org>.
caishunfeng commented on code in PR #10586:
URL: https://github.com/apache/dolphinscheduler/pull/10586#discussion_r905678667


##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/WorkflowExecuteRunnable.java:
##########
@@ -1115,6 +1115,12 @@ private TaskInstance findTaskIfExists(Long taskCode, int taskVersion) {
     private TaskInstance createTaskInstance(ProcessInstance processInstance, TaskNode taskNode) {
         TaskInstance taskInstance = findTaskIfExists(taskNode.getCode(), taskNode.getVersion());
         if (taskInstance != null) {
+            if (!taskInstance.getEnvironmentCode().equals(-1L)) {
+                Environment environment = processService.findEnvironmentByCode(taskInstance.getEnvironmentCode());

Review Comment:
   I think we should not query again after workflow running, and I found that there was already in `WorkflowExecuteRunnable.newTaskInstance`.
   
   



##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/WorkflowExecuteRunnable.java:
##########
@@ -1115,6 +1115,12 @@ private TaskInstance findTaskIfExists(Long taskCode, int taskVersion) {
     private TaskInstance createTaskInstance(ProcessInstance processInstance, TaskNode taskNode) {
         TaskInstance taskInstance = findTaskIfExists(taskNode.getCode(), taskNode.getVersion());
         if (taskInstance != null) {
+            if (!taskInstance.getEnvironmentCode().equals(-1L)) {

Review Comment:
   ```suggestion
               if (!taskInstance.getEnvironmentCode().equals(Constants.DEFAULT_ENVIRONMENT_CODE)) {
   ```



-- 
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] someorz commented on a diff in pull request #10586: [Fix-9938][master] Fix 9938 query environment config set to task instance

Posted by GitBox <gi...@apache.org>.
someorz commented on code in PR #10586:
URL: https://github.com/apache/dolphinscheduler/pull/10586#discussion_r905693687


##########
dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/WorkflowExecuteRunnable.java:
##########
@@ -1115,6 +1115,12 @@ private TaskInstance findTaskIfExists(Long taskCode, int taskVersion) {
     private TaskInstance createTaskInstance(ProcessInstance processInstance, TaskNode taskNode) {
         TaskInstance taskInstance = findTaskIfExists(taskNode.getCode(), taskNode.getVersion());
         if (taskInstance != null) {
+            if (!taskInstance.getEnvironmentCode().equals(-1L)) {
+                Environment environment = processService.findEnvironmentByCode(taskInstance.getEnvironmentCode());

Review Comment:
   great is a more elegant solution，i think in baseSql add environment_config column will resolve the problem
   ![image](https://user-images.githubusercontent.com/5456361/175458093-176da91c-9e29-4fc7-8335-aa4fe003483f.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] someorz commented on pull request #10586: [Fix-9938][master] Fix 9938 query environment config set to task instance

Posted by GitBox <gi...@apache.org>.
someorz commented on PR #10586:
URL: https://github.com/apache/dolphinscheduler/pull/10586#issuecomment-1165153413

   > I find we already have this logic in `newTaskInstance`, so there is no need to do this in everywhere, am I right?
   
   This bug will not appear in new tasks. This bug is that existing tasks will appear during fault tolerance.
   TaskInstance taskInstance = findTaskIfExists(taskNode.getCode(), taskNode.getVersion());
   findTaskIfExists return instance EnvironmentConfig is null at version 2.0.5 if new version findTaskIfExists method return TaskInstance EnvironmentConfig is not null no need to query again


-- 
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