-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
HBASE-26205 Use specified cluster conf for UserProvider in TableMRUtil#initCredentialsForCluster #3592
Conversation
…l#initCredentialsForCluster
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@bitterfox This change is required when the cluster running the VerifyReplication job is not secure and the target cluster is secure. Even after the change, I don't understand how will we fetch the token for secure cluster. Could you write a test case for this scenario ? Thank you ! |
Thanks for the comment. Added test cases that this PR want to fix. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
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.
Other than 1 minor comment, +1 ltgm. Thank you for adding test case. :)
Collection<Token<? extends TokenIdentifier>> tokens = credentials.getAllTokens(); | ||
assertTrue(tokens.isEmpty()); | ||
|
||
util1.shutdownMiniCluster(); |
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.
move shutdownminicluster to finally block
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. Added better resource management.
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
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.
In the initCredentialsForHBase method, we have this code
if("kerberos".equalsIgnoreCase(peerConf.get("hbase.security.authentication"))){
TableMapReduceUtil.initCredentialsForCluster(job, peerConf);
}
So I think the PR here to use Configuration instead of job.getConfiguration is reasonable.
…l#initCredentialsForCluster (#3592) Signed-off-by: Duo Zhang <zhangduo@apache.org> Reviewed-by: Rushabh Shah <shahrs87@gmail.com>
…l#initCredentialsForCluster (#3592) Signed-off-by: Duo Zhang <zhangduo@apache.org> Reviewed-by: Rushabh Shah <shahrs87@gmail.com>
…l#initCredentialsForCluster (#3592) Signed-off-by: Duo Zhang <zhangduo@apache.org> Reviewed-by: Rushabh Shah <shahrs87@gmail.com>
https://issues.apache.org/jira/browse/HBASE-26205