fapifta commented on code in PR #4921:
URL: https://github.com/apache/ozone/pull/4921#discussion_r1234997862
##########
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:
This method was only referenced from HASecurityUtils#getRootCASignedSCMCert,
hence the move and then I also made this private in HASecurityUtils.
I guess you are referring to getScmSecurityClientWithMaxRetry that is a
change everywhere where we create a CertificateClient, as the creation of the
ScmSecurityClient this way is moved out of the CertificateClient. By injecting
the ScmSecurityClient via the constructor gives us the benefit of not having to
be worried about initialization order in tests we do not need a setter just for
testing, and we can easily inject mocking for the communicaiton via the
constructor (actually we have to, which makes the test writer think about this,
and do not just assume that the server communication would not be call from the
test flow), also all the services can separately specify retries, timeouts and
such, which can be a benefit in the future (but I admit it is not really a
reason now).
--
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]
