----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64259/ -----------------------------------------------------------
(Updated Dec. 11, 2017, 8:03 p.m.) Review request for sentry, Alexander Kolbasov, Arjun Mishra, kalyan kumar kalvagadda, Na Li, and Sergio Pena. Changes ------- removed space and un-necessary check Repository: sentry Description ------- TheSentryMain class currently works by mapping the command name to a Java class that is then dynamically loaded: String commandName = commandLine.getOptionValue(COMMAND); String commandClazz = COMMANDS.get(commandName); Object command; try { command = Class.forName(commandClazz.trim()).newInstance(); } catch (Exception e) { String msg = "Could not create instance of " + commandClazz + " for command " + commandName; throw new IllegalStateException(msg, e); } if (!(command instanceof Command)) { String msg = "Command " + command.getClass().getName() + " is not an instance of " + Command.class.getName(); throw new IllegalStateException(msg); } ((Command)command).run(commandLine.getArgs()); } This ia too complicated and causes subtle problems at runtime. Instead it should just create a new instance of appropriate class and call it directly. Diffs (updated) ----- bin/config_tool 4da85673 bin/run_sentry.sh d58d5e5c bin/sentry 54e545aa pom.xml dd408d85 sentry-command-line-tools/pom.xml PRE-CREATION sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryMain.java 3a981b2a Diff: https://reviews.apache.org/r/64259/diff/2/ Changes: https://reviews.apache.org/r/64259/diff/1-2/ Testing ------- mvn clean install, on cloudcat and all SUCCESS Thanks, Xinran Tinney