-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
YARN-11158. Support (Create/Renew/Cancel) DelegationToken API's for Federation. #5104
Conversation
…elegationToken API's for Federation.
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@goiri Can you help review this pr? Thank you very much! |
🎊 +1 overall
This message was automatically generated. |
@goiri Can you help review this pr? Thank you very much! |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@@ -1998,4 +2088,5 @@ protected int getNumMaxThreads(Configuration conf) { | |||
public void setNumSubmitRetries(int numSubmitRetries) { | |||
this.numSubmitRetries = numSubmitRetries; | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for reviewing the code, I will fix it.
} | ||
|
||
try { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid this empty line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will fix it.
interceptor.setTokenSecretManager(tokenSecretManager); | ||
} catch (Exception e) { | ||
LOG.error(e.getMessage()); | ||
Assert.fail(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just let the exception surface?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your suggestion, I will modify the code.
YarnConfiguration.RM_DELEGATION_TOKEN_RENEW_INTERVAL_KEY, | ||
YarnConfiguration.RM_DELEGATION_TOKEN_RENEW_INTERVAL_DEFAULT); | ||
long renewDate = delegationTokenState.get(rMDelegationTokenIdentifier); | ||
Assert.assertEquals((issueDate + tokenRenewInterval), renewDate); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for internal parenthesis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will fix it.
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@goiri Thank you very much for helping to review the code! Can you help me merge this pr into the trunk branch? I will continue to follow up YARN-11225. [Federation] Add postDelegationToken, postDelegationTokenExpiration, cancelDelegationToken REST APIs for Router. |
@goiri Thank you very much for helping to review the code! |
JIRA: YARN-11158. Support getDelegationToken, renewDelegationToken, cancelDelegationToken API's for Federation.