devmadhuu commented on code in PR #9627:
URL: https://github.com/apache/ozone/pull/9627#discussion_r2703988439
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java:
##########
@@ -104,9 +108,23 @@ public Void call() throws Exception {
ReconServer.class, originalArgs, LOG, configuration);
ConfigurationProvider.setConfiguration(configuration);
+ String reconStarterUser =
UserGroupInformation.getCurrentUser().getShortUserName();
+ Collection<String> adminUsers =
+ OzoneAdmins.getOzoneAdminsFromConfig(configuration, reconStarterUser);
Review Comment:
Okay, can we refactor and extract to a helper method instead of inline code
directly in call() method:
```
private OzoneAdmins createReconAdmins(OzoneConfiguration conf) {
String starterUser =
UserGroupInformation.getCurrentUser().getShortUserName();
Collection<String> adminUsers =
OzoneAdmins.getOzoneAdminsFromConfig(conf, starterUser);
adminUsers.addAll(
conf.getStringCollection(ReconConfigKeys.OZONE_RECON_ADMINISTRATORS));
Collection<String> adminGroups =
OzoneAdmins.getOzoneAdminsGroupsFromConfig(conf);
adminGroups.addAll(
conf.getStringCollection(ReconConfigKeys.OZONE_RECON_ADMINISTRATORS_GROUPS));
return new OzoneAdmins(adminUsers, adminGroups);
}
```
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconControllerModule.java:
##########
@@ -92,8 +92,15 @@ public class ReconControllerModule extends AbstractModule {
private static final Logger LOG =
LoggerFactory.getLogger(ReconControllerModule.class);
+ private final ReconServer reconServer;
+
+ public ReconControllerModule(ReconServer reconServer) {
+ this.reconServer = reconServer;
+ }
+
@Override
protected void configure() {
+ bind(ReconServer.class).toInstance(reconServer);
Review Comment:
Okay, your implementation is correct for the current architecture. The
toInstance() pattern is appropriate here because ReconServer is created and
initialized before Guice, and we need to bind the specific initialized
instance. While OM/SCM store admin info similarly, they don't use Guice, so in
a way, the comparison isn't directly applicable.
--
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]
