[
https://issues.apache.org/jira/browse/SLIDER-773?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14491261#comment-14491261
]
Ted Yu commented on SLIDER-773:
-------------------------------
Some minor comments:
{code}
+ log.debug("metainfo map for master and addon is: {}",
+ packageMetainfo.toString());
{code}
The contents of packageMetainfo have been logged in the prior while loop. Is
the above needed ?
{code}
+ log.debug("for comp " + roleName + " pkg " +
componentStatus.getNextPkgToInstall() + " issuing " + command.toString());
{code}
Wrap long line. In HBase, line length is limited to 100.
{code}
+ ComponentCommand installCmd = null;
+ for (ComponentCommand compCmd : comp.getCommands()) {
+ if (compCmd.getName().equals("INSTALL")) {
+ installCmd = compCmd;
+ }
+ }
+ addInstallCommand(roleName, containerId, response, null,
+ installCmd, timeout, nextPkgToInstall);
{code}
What if 'INSTALL' command isn't found in the for loop above ?
{code}
+ String appDef, boolean
addonPackage) throws IOException, BadConfigException {
{code}
Long line.
{code}
+ * @param pkg TODO
{code}
Please finish javadoc
{code}
+ metainfoParser = new AddoPackageMetainfoParser();
{code}
Typo in classname above: missing 'n' between 'o' and 'P'
{code}
+ log.debug("commandissued: component: {} is in {}", componentName,
currentState);
{code}
Insert space between 'd' and 'i'.
{code}
+ if (pkg != null && !pkg.isEmpty()
+ && !pkg.equals(Component.MASTER_PACKAGE_NAME)) {
{code}
The above can be simplified if you use Component.MASTER_PACKAGE_NAME.equals(pkg)
{code}
+ public void applyCommandResult(CommandResult result, Command command, String
pkg) {
...
+ public void applyCommandResult(CommandResult result, Command command) {
{code}
Looks like there is common code between the above two methods which can be
extracted.
{code}
+ for (String pkg : pkgStatuses.keySet()) {
+ State pkgState = pkgStatuses.get(pkg);
{code}
Please use pkgStatuses.entrySet() since both key and value are accessed.
I think the two loops can be combined: if State.INSTALLING is encountered,
return Command.NOP. Otherwise remember the pkg until the end of loop and return
Command.INSTALL_ADDON for the pkg.
{code}
+ case INSTALL_ADDON:
+ if (this == State.INIT || this == State.INSTALL_FAILED) {
+ return State.INSTALLING;
{code}
For State.INSTALL_FAILED, do we need to keep a counter to limit the number of
failures ?
{code}
+/**
+ *
+ */
+public abstract class AbstractMetainfoParser {
{code}
Please fill in javadoc for the class.
> Add co-processor support for app packages
> -----------------------------------------
>
> Key: SLIDER-773
> URL: https://issues.apache.org/jira/browse/SLIDER-773
> Project: Slider
> Issue Type: Bug
> Components: app-package, client
> Affects Versions: Slider 0.60
> Reporter: Sumit Mohanty
> Assignee: thomas liu
> Priority: Critical
> Fix For: Slider 0.80
>
> Attachments: Co-processorSupport.pdf, coprocessor-apri-4th.patch,
> coprocessor.patch, coprocessor.patch
>
>
> It is typical for applications to allow plugins/co-processors that are
> essentially a set of additional jar files in the classpath and optionally a
> set of config files or config changes.
> Current, slider app packages can handle additional config changes/entries
> very well. Additional configs files can be added as well but it is not easy
> if the config files include parameters that need to be resolved by the agent.
> This requires app package changes. Dropping additional jar files into the
> class path is not easy and requires app package changes.
> It is not efficient to modify the app package to support such plugins. App
> packaging and create command should be modified such that the user can
> dynamically specify additional jars, config files, configs etc.
> Specific scenarios are modifying HBase to add support for Phoenix or Ranger.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
