[jira] [Commented] (SLIDER-773) Add co-processor support for app packages

["Steve Loughran (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SLIDER-773?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14482966#comment-14482966 ] 

Steve Loughran commented on SLIDER-773:
---------------------------------------

I should warn that I that I don't know the .py-side of things (or the agent provider) as well as Gour or Sumit; their reviews are needed too. From what I can see, looks good

.h2 Production code

.h3 {{Heartbeat.py}}

-what if there is no componentPackage? will it be null or ""?  And does the AM expect this


.h3 {{AgentProviderService.java}}

like 764, looks like a log entry was accidentally deleted ... can you restore it.

line 1679; the javadocs have a {{@param pkg}}, but the method doesn't. Is this an incomplete method or just a javadoc line to be deleted?

.h3 {{State.java}}

class needs a {{toString}} method printing state, if its to be used in exception messages.

.h3 {{AbstractComponent}}

fields should be {{protected}} or {{private}}

.h3 {{AbstractMetainfoParser}}

I see that when there are parsing problems, the exceptions are simply caught and null returned. This is exactly what {{MetainfoParser}}. I don't think it should be done. The exceptions should be logged @ debug level, to help debug things like XML parse problems. (Ultimately that exception should be relayed -this could be filed as a separate JIRA)


.h3 logging

* logs (especially debug ones), should use the Slf4J built in expansion, of things like
* I think there's too much @ info, such as in ComponentInstanceState. They can be downgraded to debug; if needed for diagnosing things, log4j.properties is easy to edit. That 
{code}
log.debug("addonMap.get(key): {}" , addonMap.get(key))
{code}

it is a bit more efficient as it doesn't do the string expansion unless needed, and reports nulls too.

h2. Tests

I think we need more tests. 

# Building a cluster: add tests to build a cluster with a package; include ones to test invalid details (e.g. no such package, package with bad metadata)
# copy of TestAgentEcho with a stub package
# Anything we can do to test CLI


Functional tests can be added as a separate/sub JIRA; I can imagine two
# add a simple package to one of the AgentIT tests; have it do something minimal like touch a file in HDFS. The test would look for that file existing.
# add a missing stub package, expect failures

h2. Documentation

needs a JIRA on this

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