[ https://issues.apache.org/jira/browse/SENTRY-1572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16255840#comment-16255840 ]
Xinran Tinney edited comment on SENTRY-1572 at 11/17/17 9:10 PM: ----------------------------------------------------------------- [~sasha99] Do you want the code to be like this? This may need 2 other dependencies for core-common (sentry-provider-db and sentry-binding-hive) and after I added the dependencies, there is loop dependency since sentry-binding-hive also depends on sentry-core-common {code:java} Object command = null; switch (commandName){ case "service": command = new SentryService.CommandImpl(); break; case "config-tool": command = new SentryConfigTool.CommandImpl(); break; case "schema-tool": command = new SentrySchemaTool.CommandImpl(); break; default: printHelp(options, "Unknown command " + commandName + "\n"); } if (null != command) { ((Command) command).run(commandLine.getArgs()); } {code} [~akolb]was suggesting that SentryMain shouldn't live in core-common, what do you all think? [~lina...@cloudera.com],[~spena],[~kkalyan],[~arjunmishra13], [~hgadre] was (Author: xyu2017): [~sasha99] Do you want the code to be like this? This may need 2 other dependencies for core-common (sentry-provider-db and sentry-binding-hive) and after I added the dependencies, there is loop dependency since sentry-binding-hive also depends on sentry-core-common {code:java} Object command = null; switch (commandName){ case "service": command = new SentryService.CommandImpl(); break; case "config-tool": command = new SentryConfigTool.CommandImpl(); break; case "schema-tool": command = new SentrySchemaTool.CommandImpl(); break; default: printHelp(options, "Unknown command " + commandName + "\n"); } if (null != command) { ((Command) command).run(commandLine.getArgs()); } {code} > SentryMain() shouldn't dynamically load tool class > -------------------------------------------------- > > Key: SENTRY-1572 > URL: https://issues.apache.org/jira/browse/SENTRY-1572 > Project: Sentry > Issue Type: Improvement > Components: Sentry > Reporter: Alexander Kolbasov > Assignee: Xinran Tinney > Labels: bite-sized, newbie > > TheSentryMain class currently works by mapping the command name to a Java > class that is then dynamically loaded: > {code} > 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()); > } > {code} > 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. -- This message was sent by Atlassian JIRA (v6.4.14#64029)