mcvsubbu commented on a change in pull request #5260: Add access control for
Pinot server segment download api.
URL: https://github.com/apache/incubator-pinot/pull/5260#discussion_r410486118
##########
File path:
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixServerStarter.java
##########
@@ -159,9 +160,11 @@ public HelixServerStarter(String helixClusterName, String
zkAddress, Configurati
LOGGER.info("Using class: {} as the AccessControlFactory",
accessControlFactoryClass);
final AccessControlFactory accessControlFactory;
try {
- accessControlFactory = (AccessControlFactory)
Class.forName(accessControlFactoryClass).newInstance();
+ accessControlFactory =
PluginManager.get().createInstance(accessControlFactoryClass);
} catch (Exception e) {
- throw new RuntimeException("Caught exception while creating new
AccessControlFactory instance", e);
+ throw new RuntimeException(
+ "Caught exception while creating new AccessControlFactory instance
in class " + this.getClass()
Review comment:
The class name you want to add to the exception is not `this.getClass()`.
Instead, it should be the value of `accessControlFactory`. The idea is that the
exception message should reflect the remedial action as close as possible (e.g.
in this case, it could be a config typo). It may even be useful to put it in
quotes, so as to spot extra spaces (e.g. `"... in class ' +
accessControlFactoryClass + "'", e)`)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
