-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64259/#review192691
-----------------------------------------------------------



Why is a new "sentry-main" module needed as part of this fix?

- Colm O hEigeartaigh


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
> 
>

Reply via email to