Skip to content

#1310 add executor user to the process instance & task instance pages#1973

Merged
davidzollo merged 11 commits intoapache:devfrom
Eights-Li:dev-instance
Feb 20, 2020
Merged

#1310 add executor user to the process instance & task instance pages#1973
davidzollo merged 11 commits intoapache:devfrom
Eights-Li:dev-instance

Conversation

@Eights-Li
Copy link
Copy Markdown
Contributor

@Eights-Li Eights-Li commented Feb 18, 2020 •
edited
Loading

What is the purpose of the pull request

#1310 add executor user to the process instance & task instance pages

Brief change log

  • Add executor_id field in t_ds_task_instance table
  • api/controller/ProcessInstanceController
  • api/controller/TaskInstanceController

Verify this pull request

This change added tests and can be verified as follows:

  • api/service/ProcessInstanceServiceTest
  • api/service/TaskInstanceServiceTest
  • add executor_id field in t_ds_task_instance table have verfied in mysql5.6 & pg9.6

@codecov-io
Copy link
Copy Markdown

codecov-io commented Feb 18, 2020 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (dev@665349a). Click here to learn what that means.
The diff coverage is 37.83%.

Impacted file tree graph

@@          Coverage Diff           @@
##             dev    #1973   +/-   ##
======================================
  Coverage       ?   30.82%           
  Complexity     ?     1381           
======================================
  Files          ?      294           
  Lines          ?    15022           
  Branches       ?     1810           
======================================
  Hits           ?     4631           
  Misses         ?     9931           
  Partials       ?      460
Impacted Files Coverage Δ Complexity Δ
...uler/api/controller/ProcessInstanceController.java 2.43% <ø> (ø) 2 <0> (?)
...ache/dolphinscheduler/dao/entity/TaskInstance.java 0.87% <ø> (ø) 1 <0> (?)
...heduler/api/controller/TaskInstanceController.java 22.22% <ø> (ø) 2 <0> (?)
...e/dolphinscheduler/dao/entity/ProcessInstance.java 0% <ø> (ø) 0 <0> (?)
...he/dolphinscheduler/server/utils/ProcessUtils.java 16.54% <0%> (ø) 8 <0> (?)
...che/dolphinscheduler/api/service/UsersService.java 74.27% <100%> (ø) 64 <3> (?)
...phinscheduler/api/service/TaskInstanceService.java 83.33% <100%> (ø) 6 <0> (?)
.../apache/dolphinscheduler/common/utils/OSUtils.java 48.34% <6.81%> (ø) 22 <1> (?)
...he/dolphinscheduler/api/service/LoggerService.java 91.3% <77.77%> (ø) 7 <1> (?)
...nscheduler/api/service/ProcessInstanceService.java 15.03% <85.71%> (ø) 8 <0> (?)

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 665349a...b417dfc. Read the comment docs.

@Eights-Li Eights-Li requested review from lenboo and lgcareer February 18, 2020 11:39
@Eights-Li
Copy link
Copy Markdown
Contributor Author

Eights-Li commented Feb 18, 2020

@break60 please review front-end, thx~

@break60
Copy link
Copy Markdown
Contributor

break60 commented Feb 18, 2020

front-end +1

lgcareer
lgcareer reviewed Feb 19, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@lgcareer lgcareer left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Master Server genrate the task instance,the executor id should init when create the task.
@lenboo please review it.

lenboo
lenboo reviewed Feb 19, 2020
View reviewed changes
...scheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/TaskInstanceService.java Outdated

//executor name query
int executorId = 0;
if (StringUtils.isNotEmpty(executorName)) {
Copy link
Copy Markdown
Contributor

@lenboo lenboo Feb 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can duplicate code be refactored together？
Suggest that put these them in UserService.

Copy link
Copy Markdown
Contributor Author

@Eights-Li Eights-Li Feb 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx, i will fix it

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Feb 19, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 2 Code Smells

63.6% 63.6% Coverage
0.0% 0.0% Duplication

@Eights-Li Eights-Li requested a review from lenboo February 19, 2020 05:09
lenboo
lenboo approved these changes Feb 19, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@lenboo lenboo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

davidzollo
davidzollo approved these changes Feb 20, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@davidzollo davidzollo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@davidzollo davidzollo merged commit 619fb30 into apache:dev Feb 20, 2020
@Eights-Li Eights-Li deleted the dev-instance branch February 20, 2020 13:13
lenboo pushed a commit to lenboo/dolphinscheduler that referenced this pull request Feb 21, 2020
@Eights-Li
apache#1310 add executor user to the process instance & task instance…
34af1af 
… pages (apache#1973)

* add executor name in processDefinitionInstance & taskInstance

* modify processInstanceTest

* modify processInstanceTest

* apache#1310 add executor user to the process instance & task instance pages

* add processInstanceUT & taskInstanceUT

* add processInstanceUT & taskInstanceUT

* modify processInstanceTest & taskInstanceTest

* Remove duplicate code

* add userServiceUT
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lgcareer lgcareer lgcareer left review comments

@davidzollo davidzollo davidzollo approved these changes

@lenboo lenboo lenboo approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@Eights-Li @codecov-io @break60 @lgcareer @davidzollo @lenboo