----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64259/ -----------------------------------------------------------
(Updated Jan. 10, 2018, 9:38 p.m.) Review request for sentry, Alexander Kolbasov, Arjun Mishra, kalyan kumar kalvagadda, Na Li, and Sergio Pena. Changes ------- Fixed the path of SentryMain.java 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/sentry 54e545aa sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryMain.java 3a981b2a sentry-tools/pom.xml 45cfdb56 sentry-tools/src/main/java/org/apache/sentry/SentryMain.java PRE-CREATION Diff: https://reviews.apache.org/r/64259/diff/5/ Changes: https://reviews.apache.org/r/64259/diff/4-5/ Testing ------- mvn clean install, on cloudcat and all SUCCESS Thanks, Xinran Tinney