Skip to content

fix(dolphinscheduler-alert): fix create http request error #14793

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
brave-lee merged 3 commits into from
Sep 9, 2023
Merged

fix(dolphinscheduler-alert): fix create http request error #14793

brave-lee merged 3 commits into from
Sep 9, 2023

Conversation

@hunter-cloud09
Copy link
Contributor

@hunter-cloud09 hunter-cloud09 commented Aug 23, 2023

This pull request is code cleanup without any test coverage.
request type should use equalsIgnoreCase to equal,msg should add URLEncoder.encode #14777 #14781

@hunter-cloud09
fix(dolphinscheduler-alert): fix create http request error
d3d9c50 
request type should use equalsIgnoreCase to equal,msg should add URLEncoder.encode
@Radeity Radeity added the bug Something isn't working label Aug 23, 2023
@Radeity Radeity added this to the 3.2.0 milestone Aug 23, 2023
@fuchanghai
Copy link
Member

fuchanghai commented Aug 28, 2023

@Radeity plz update and rerun the failed part

fuchanghai
fuchanghai approved these changes Aug 28, 2023
View reviewed changes
Copy link
Member

@fuchanghai fuchanghai left a comment

Choose a reason for hiding this comment

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

LGTM ,if pass CI

@codecov-commenter
Copy link

codecov-commenter commented Aug 28, 2023 •
edited
Loading

Codecov Report

Merging #14793 (15287f9) into dev (3fa69d2) will decrease coverage by 0.02%.
The diff coverage is 33.33%.

❗ Current head 15287f9 differs from pull request most recent head 04679df. Consider uploading reports for the commit 04679df to get more accurate results

@@             Coverage Diff              @@
##                dev   #14793      +/-   ##
============================================
- Coverage     38.88%   38.87%   -0.02%     
- Complexity     4585     4586       +1     
============================================
  Files          1232     1232              
  Lines         43243    43246       +3     
  Branches       4772     4770       -2     
============================================
- Hits          16817    16810       -7     
- Misses        24566    24578      +12     
+ Partials       1860     1858       -2
Files Changed Coverage Δ
...dolphinscheduler/plugin/alert/http/HttpSender.java 47.88% <33.33%> (-0.65%) ⬇️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Radeity Radeity added ready-to-merge 3.2.0 for 3.2.0 version labels Aug 28, 2023
hunter-cloud09
hunter-cloud09 commented Aug 28, 2023
View reviewed changes
...duler-alert-http/src/main/java/org/apache/dolphinscheduler/plugin/alert/http/HttpSender.java
}
url = String.format("%s%s%s=%s", url, type, contentField, msg);
try {
url = String.format("%s%s%s=%s", url, type, contentField, URLEncoder.encode(msg, DEFAULT_CHARSET));
Copy link
Contributor Author

@hunter-cloud09 hunter-cloud09 Aug 28, 2023

Choose a reason for hiding this comment

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

Hi, @Radeity @fuchanghai , If add this, Should I remove the 124 lines of code to construct the URL, and then directly construct the URI through url?

@Radeity Radeity added the first time contributor First-time contributor label Aug 28, 2023
@zhongjiajie zhongjiajie modified the milestones: 3.2.0, 3.3.0 Aug 30, 2023
brave-lee
brave-lee approved these changes Sep 9, 2023
View reviewed changes
Copy link
Contributor

@brave-lee brave-lee left a comment

Choose a reason for hiding this comment

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

+1

@brave-lee brave-lee removed the request for review from caishunfeng September 9, 2023 07:59
@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 9, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

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

60.0% 60.0% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@brave-lee brave-lee merged commit 3c71fb0 into apache:dev Sep 9, 2023
This was referenced Sep 9, 2023
Closed
Closed
@brave-lee brave-lee removed the 3.2.0 for 3.2.0 version label Sep 9, 2023
@brave-lee brave-lee modified the milestones: 3.3.0, 3.2.1, 3.1.9 Sep 9, 2023
@brave-lee brave-lee modified the milestones: 3.2.1, 3.1.9 Sep 9, 2023
zhuangchong pushed a commit that referenced this pull request Sep 19, 2023
@hunter-cloud09 @zhuangchong
cherry-pick fix create http request error #14793
0609119
zhongjiajie pushed a commit that referenced this pull request Oct 11, 2023
@hunter-cloud09 @Radeity
@brave-lee @zhongjiajie
fix(dolphinscheduler-alert): fix create http request error (#14793)
e2c8815 
request type should use equalsIgnoreCase to equal,msg should add URLEncoder.encode

Co-authored-by: hunter-cloud09 <[email protected]>
Co-authored-by: Aaron Wang <[email protected]>
Co-authored-by: JinYong Li <[email protected]>
(cherry picked from commit 3c71fb0)
@zhuangchong zhuangchong added the release cherry-pick Mark this issue/PR had cherry-pick for release version label Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fuchanghai fuchanghai fuchanghai approved these changes

@brave-lee brave-lee brave-lee approved these changes

@Radeity Radeity Radeity approved these changes

Assignees

@hunter-cloud09 hunter-cloud09

Labels

backend bug Something isn't working first time contributor First-time contributor ready-to-merge release cherry-pick Mark this issue/PR had cherry-pick for release version

Projects

None yet

Milestone

3.1.9

Development

Successfully merging this pull request may close these issues.

7 participants

@hunter-cloud09 @fuchanghai @codecov-commenter @brave-lee @Radeity @zhongjiajie @zhuangchong