fapifta commented on code in PR #4921:
URL: https://github.com/apache/ozone/pull/4921#discussion_r1235308495
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HddsServerUtil.java:
##########
@@ -472,31 +470,6 @@ public static SCMSecurityProtocolClientSideTranslatorPB
getScmSecurityClient(
ugi == null ? UserGroupInformation.getCurrentUser() : ugi));
}
- public static SCMSecurityProtocolClientSideTranslatorPB
Review Comment:
I have looked into how we can bring back the SCMSecurityProtocolClient
creation into the CertificateClient, and then I realized the main reason why it
has to be outside, besides other advantages I think are there and I noted
before.
It is using Hadoop RPC, with that it requires the full Configuration, and
not just the SecurityConfig, so with that we need to have the full
Configuration in the CertificateClient code again, and that means the bulk if
this PR is not really meaningful anymore.
Actually the code of creation is not duplicated, we are calling the same
utility method everywhere to create the client, and the provide it to the
CertificateClient instance.
I have played around with the idea of having the SCMClientConfig in the
constructor, and based on it create the client, but that one is also somewhat
tedious, as either we take the Configuration and convert it internally, or we
do something different...
One idea I can imagine working, where we take a ConfigurationSource in the
DefaultCertificateClient constructor instead of SecurityConfig, and then in the
DefaultCertificateClient we create the SecurityConfig and the proper
SCMClientConfig and then use these further down the line. Unsure, but I can
play around with this idea if it sounds more feasible, should I?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
