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 part
of 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]
