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

[#625] improvement: Package sun.security.krb5 is not visible in Java 11 and 17. #726

Merged
merged 2 commits into from
Mar 15, 2023

Conversation

slfan1989
Copy link
Collaborator

What changes were proposed in this pull request?

Try remove sun.security.krb5.Config.refresh();

Why are the changes needed?

sun.security.krb5 is not visible in Java 11 and 17. We need to compile on JDK11 and JDK17

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Unit test verification.

@jerqi jerqi changed the title [#625] Package sun.security.krb5 is not visible in Java 11 and 17. [#625] improvement: Package sun.security.krb5 is not visible in Java 11 and 17. Mar 15, 2023
@jerqi jerqi requested a review from zuston March 15, 2023 12:54
@codecov-commenter
Copy link

codecov-commenter commented Mar 15, 2023

Codecov Report

Merging #726 (f2dcd0f) into master (e38d799) will increase coverage by 2.30%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #726      +/-   ##
============================================
+ Coverage     60.60%   62.91%   +2.30%     
- Complexity     1849     1859      +10     
============================================
  Files           229      217      -12     
  Lines         12749    10897    -1852     
  Branches       1064     1073       +9     
============================================
- Hits           7727     6856     -871     
+ Misses         4611     3686     -925     
+ Partials        411      355      -56     
Impacted Files Coverage Δ
...uniffle/common/security/HadoopSecurityContext.java 81.08% <ø> (-0.50%) ⬇️

... and 18 files with indirect coverage changes

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

Copy link
Contributor

@jerqi jerqi left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @slfan1989

@jerqi jerqi merged commit 2be4fd7 into apache:master Mar 15, 2023
@slfan1989
Copy link
Collaborator Author

@jerqi @zuston @kaijchen Thank you very much for helping to review the code!

advancedxy pushed a commit to advancedxy/incubator-uniffle that referenced this pull request Mar 21, 2023
… Java 11 and 17. (apache#726)

### What changes were proposed in this pull request?

Try remove `sun.security.krb5.Config.refresh();`

### Why are the changes needed?

`sun.security.krb5` is not visible in Java 11 and 17. We need to compile on JDK11 and JDK17

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Unit test verification.

Co-authored-by: slfan1989 <louj1988@@>
xianjingfeng pushed a commit to xianjingfeng/incubator-uniffle that referenced this pull request Apr 5, 2023
… Java 11 and 17. (apache#726)

### What changes were proposed in this pull request?

Try remove `sun.security.krb5.Config.refresh();`

### Why are the changes needed?

`sun.security.krb5` is not visible in Java 11 and 17. We need to compile on JDK11 and JDK17

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Unit test verification.

Co-authored-by: slfan1989 <louj1988@@>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants