Skip to content
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.

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

[Fix-3262][common] When you request the URL through applicationID to get the application status, you cannot get it if Kerberos authentication is enabled #3264

Merged
merged 39 commits into from
Aug 10, 2020

Conversation

felix-thinkingdata
Copy link
Contributor

@felix-thinkingdata felix-thinkingdata commented Jul 21, 2020

[Bug][dolphinscheduler-common] When you request the URL through applicationID to get the application status, you cannot get it if Kerberos authentication is enabled #3262

Tips
Thanks very much for contributing to Apache DolphinScheduler.
Please review https://dolphinscheduler.apache.org/en-us/community/index.html before opening a pull request.
What is the purpose of the pull request

fix #3262

@codecov-commenter
Copy link

Codecov Report

Merging #3264 into dev will decrease coverage by 0.16%.
The diff coverage is 19.83%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev    #3264      +/-   ##
============================================
- Coverage     34.54%   34.37%   -0.17%     
  Complexity     2464     2464              
============================================
  Files           443      444       +1     
  Lines         20641    20739      +98     
  Branches       2532     2541       +9     
============================================
  Hits           7130     7130              
- Misses        12849    12946      +97     
- Partials        662      663       +1     
Impacted Files Coverage Δ Complexity Δ
...phinscheduler/common/utils/KerberosHttpClient.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...pache/dolphinscheduler/common/utils/HttpUtils.java 61.90% <61.53%> (-10.32%) 3.00 <2.00> (ø)
...g/apache/dolphinscheduler/dao/utils/DagHelper.java 27.11% <0.00%> (-1.63%) 18.00% <0.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 6f2667b...e0a368c. Read the comment docs.

@yangyichao-mango
Copy link
Contributor

Hi,

Please change the title according to the pr specifications[1]. Thx a lot~

[1] https://dolphinscheduler.apache.org/zh-cn/docs/development/pull-request.html

@felix-thinkingdata
Copy link
Contributor Author

felix-thinkingdata commented Jul 22, 2020 via email

@felix-thinkingdata felix-thinkingdata changed the title fix bug #3262 When you request the URL through applicationID to get the application status, you cannot get it if Kerberos authentication is enabled [Fix-3262][common] When you request the URL through applicationID to get the application status, you cannot get it if Kerberos authentication is enabled Jul 22, 2020
@yangyichao-mango
Copy link
Contributor

yangyichao-mango commented Jul 22, 2020

Please add test case code the for new code, sonar request 33.3% coverage for new code.
image

@felix-thinkingdata
Copy link
Contributor Author

felix-thinkingdata commented Jul 22, 2020 via email

@felix-thinkingdata
Copy link
Contributor Author

please revert the code format changes, and only keep your changes. it will make our submission records confusing.

please revert the code format changes, and only keep your changes. it will make our submission records confusing.

The code format changes have been completed ,Please check the code for me again,thank you

pom.xml Outdated Show resolved Hide resolved
@yangyichao-mango
Copy link
Contributor

yangyichao-mango commented Jul 28, 2020

Hi,
Good job,
I create an improvement issue #3332 about HttpClient and KerberosHttpClient can further optimize singleton mode.

@felix-thinkingdata
Copy link
Contributor Author

felix-thinkingdata commented Jul 28, 2020 via email

@yangyichao-mango
Copy link
Contributor

yangyichao-mango commented Jul 28, 2020

Should I continue to optimize my patch or create a new patch? 原始邮件 发件人: Yichao Yangnotifications@github.com 收件人: apache/incubator-dolphinschedulerincubator-dolphinscheduler@noreply.github.com 抄送: felix.wangfelix@thinkingdata.cn; State changestate_change@noreply.github.com 发送时间: 2020年7月28日(周二) 19:49 主题: Re: [apache/incubator-dolphinscheduler] [Fix-3262][common] When yourequest the URL through applicationID to get the application status, youcannot get it if Kerberos authentication is enabled (#3264) Hi, Good job, I create an issue #3332 about HttpClient and KerberosHttpClient can further optimize singleton mode. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or unsubscribe.

Hi~,
I think this pr is just for fixing the bug of the issue #3262 , and at present this pr LGTM.
But the issue #3332 is for httpClient singleton optimize, it is not the part of this pr , so I create #3332 .
And it will be better if you are interested in optimizing the singleton in this pr, if you want to optimize it, please leave a message, after that I will close #3332 .

@felix-thinkingdata
Copy link
Contributor Author

felix-thinkingdata commented Jul 28, 2020 via email

@felix-thinkingdata
Copy link
Contributor Author

felix-thinkingdata commented Jul 28, 2020 via email

@felix-thinkingdata
Copy link
Contributor Author

felix-thinkingdata commented Jul 28, 2020 via email

@sonarcloud
Copy link

sonarcloud bot commented Jul 30, 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

62.1% 62.1% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@yangyichao-mango yangyichao-mango left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Contributor

@qiaozhanwei qiaozhanwei left a comment

Choose a reason for hiding this comment

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

+1

@qiaozhanwei qiaozhanwei merged commit 5584f0c into apache:dev Aug 10, 2020
@davidzollo davidzollo added this to the 1.4.0 milestone Aug 11, 2020
@felix-thinkingdata felix-thinkingdata deleted the mergeHttpClient branch August 26, 2020 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants