> On Dec. 4, 2017, 1 p.m., Colm O hEigeartaigh wrote: > > Why is a new "sentry-main" module needed as part of this fix?
because of dependency. If the command is under one of existing directories, it will create circular dependency. That may be why the original code takes that approach "mapping the command name to a Java class that is then dynamically loaded" - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64259/#review192691 ----------------------------------------------------------- On Dec. 1, 2017, 9:15 p.m., Xinran Tinney wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64259/ > ----------------------------------------------------------- > > (Updated Dec. 1, 2017, 9:15 p.m.) > > > Review request for sentry, Alexander Kolbasov, Arjun Mishra, kalyan kumar > kalvagadda, Na Li, and Sergio Pena. > > > 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 > ----- > > bin/config_tool 4da85673 > bin/run_sentry.sh d58d5e5c > bin/sentry 54e545aa > pom.xml dd408d85 > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/SentryMain.java > 3a981b2a > sentry-main/pom.xml PRE-CREATION > > > Diff: https://reviews.apache.org/r/64259/diff/1/ > > > Testing > ------- > > mvn clean install, on cloudcat and all SUCCESS > > > Thanks, > > Xinran Tinney > >
