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