Skip to content

[Feature-5087][SqlTask] Add a switch to send mail and print head logs in SqlTask#5088

Merged
CalvinKirs merged 2 commits intoapache:devfrom
chengshiwen:feature-switch-to-send-email-in-sql-task
Mar 19, 2021
Merged

[Feature-5087][SqlTask] Add a switch to send mail and print head logs in SqlTask#5088
CalvinKirs merged 2 commits intoapache:devfrom
chengshiwen:feature-switch-to-send-email-in-sql-task

Conversation

@chengshiwen
Copy link
Copy Markdown
Member

@chengshiwen chengshiwen commented Mar 18, 2021 •
edited
Loading

Purpose of the pull request

*[Feature-5087][SqlTask] Add a switch to send mail and print head logs in SqlTask

This closes #174 closes #547 closes #5087

Brief change log

  • Add a switch to send mail in SqlTask
  • Print head logs in SqlTask
  • Compatible with historical versions

Verify this pull request

This pull request is already covered by existing tests, such as (please describe tests).

(and)

This change added tests and can be verified as follows:

  • Manually verified the change by testing locally.

Snapshots

Configure SQLTask:

Picture 1:
image

Picture 2:
image

Picture 3:
image

Send email but failed

image

Turn off sending email

image

@dailidong @CalvinKirs @chenxingchun

@chengshiwen chengshiwen force-pushed the feature-switch-to-send-email-in-sql-task branch from 1273cca to c37af40 Compare March 18, 2021 05:59
@chengshiwen chengshiwen force-pushed the feature-switch-to-send-email-in-sql-task branch from c37af40 to 08e4dba Compare March 18, 2021 06:06
@codecov-io
Copy link
Copy Markdown

codecov-io commented Mar 18, 2021

Codecov Report

Merging #5088 (c37af40) into dev (29d42fd) will decrease coverage by 0.05%.
The diff coverage is 0.00%.

❗ Current head c37af40 differs from pull request most recent head 08e4dba. Consider uploading reports for the commit 08e4dba to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##                dev    #5088      +/-   ##
============================================
- Coverage     47.23%   47.17%   -0.06%     
+ Complexity     3685     3680       -5     
============================================
  Files           569      569              
  Lines         23895    23905      +10     
  Branches       2749     2753       +4     
============================================
- Hits          11287    11278       -9     
- Misses        11580    11593      +13     
- Partials       1028     1034       +6
Impacted Files Coverage Δ Complexity Δ
.../org/apache/dolphinscheduler/common/Constants.java 81.81% <ø> (ø) 1.00 <0.00> (ø)
...olphinscheduler/common/task/sql/SqlParameters.java 2.32% <0.00%> (-0.38%) 1.00 <0.00> (ø)
...lphinscheduler/server/worker/task/sql/SqlTask.java 7.46% <0.00%> (-0.16%) 2.00 <0.00> (ø)
...er/master/dispatch/host/assign/RandomSelector.java 77.77% <0.00%> (-5.56%) 3.00% <0.00%> (-1.00%)
...dolphinscheduler/service/zk/ZookeeperOperator.java 36.27% <0.00%> (-3.93%) 14.00% <0.00%> (-2.00%)
...r/server/worker/processor/TaskCallbackService.java 38.09% <0.00%> (-3.18%) 8.00% <0.00%> (ø%)
...inscheduler/common/thread/ThreadPoolExecutors.java 19.23% <0.00%> (-1.93%) 3.00% <0.00%> (-2.00%)

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 29d42fd...08e4dba. Read the comment docs.

@chengshiwen
Copy link
Copy Markdown
Member Author

chengshiwen commented Mar 18, 2021

@CalvinKirs I have tried to improve the unit test coverage to 22.2% from 0%. But the db is more difficult to mock, so the SqlTaskTest also has never been enabled to test.

CalvinKirs
CalvinKirs approved these changes Mar 18, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@CalvinKirs CalvinKirs left a comment

Choose a reason for hiding this comment

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

LGTM.backend

@CalvinKirs CalvinKirs added feature new feature backend UI ui and front end related Waiting for review Waiting for review labels Mar 18, 2021
@CalvinKirs
Copy link
Copy Markdown
Member

CalvinKirs commented Mar 18, 2021

btw, send email default value should is true. please reviewers notice.

@chengshiwen chengshiwen force-pushed the feature-switch-to-send-email-in-sql-task branch from ea3f0c9 to fb333d1 Compare March 18, 2021 15:41
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Mar 18, 2021

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

22.2% 22.2% Coverage
0.0% 0.0% Duplication

chengshiwen added a commit to chengshiwen/dolphinscheduler that referenced this pull request Mar 18, 2021
@chengshiwen
[1.3.6-prepare][Feature-5087][SqlTask] Add a switch to send mail and …
ee34311 
…print head logs in SqlTask apache#5088
@chengshiwen chengshiwen mentioned this pull request Mar 18, 2021
Merged
break60
break60 approved these changes Mar 19, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@break60 break60 left a comment

Choose a reason for hiding this comment

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

+1

@CalvinKirs CalvinKirs merged commit 8ac72e8 into apache:dev Mar 19, 2021
chengshiwen added a commit to chengshiwen/dolphinscheduler that referenced this pull request Mar 20, 2021
@chengshiwen
[1.3.6-prepare][Feature-5087][SqlTask] Add a switch to send mail and …
3ff9ee0 
…print head logs in SqlTask apache#5088
xingchun-chen pushed a commit that referenced this pull request Mar 22, 2021
@chengshiwen
[1.3.6-prepare][Feature-5087][SqlTask] Add a switch to send mail and …
7df22bd 
…print head logs in SqlTask #5088 (#5104)

* [1.3.6-prepare][Improvement][UI] Rename from-mirror to form-mirror and from-model to form-model

* [1.3.6-prepare][Feature-5087][SqlTask] Add a switch to send mail and print head logs in SqlTask #5088
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@CalvinKirs CalvinKirs CalvinKirs approved these changes

+1 more reviewer

@break60 break60 break60 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

backend feature new feature UI ui and front end related Waiting for review Waiting for review

Projects

None yet

Milestone

No milestone

4 participants

@chengshiwen @codecov-io @CalvinKirs @break60