[
https://issues.apache.org/jira/browse/FLUME-1852?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13555515#comment-13555515
]
Brock Noland commented on FLUME-1852:
-------------------------------------
11. We copy the map at the top of the configure method:
// we are going to modify the properties as we parse the config
properties = new HashMap<String, String>(properties);
> Issues with EmbeddedAgentConfiguration
> --------------------------------------
>
> Key: FLUME-1852
> URL: https://issues.apache.org/jira/browse/FLUME-1852
> Project: Flume
> Issue Type: Bug
> Affects Versions: v1.4.0
> Reporter: Will McQueen
> Assignee: Brock Noland
> Fix For: v1.4.0
>
>
> Hi,
> Could you please provide feedback on the following points?
> In EmbeddedAgentConfiguration.java:
> 1. Comment says "Source type, choices are 'embedded' or 'avro'
> //fix: should only be 'embedded'
> 2. 'embedded' doesn't work as a value of 'source.type'. Either the
> 'source.type' line should be ommitted in the config (so that it defaults to
> the proper class), or the source.type needs to say:
> "source.type=org.apache.flume.agent.embedded.EmbeddedSource"
> Ideally, it would be nice if a user actually could specify "embedded" as an
> alias for the entire fully-qualified class name. I see that we don't include
> EMBEDDED in the SourceType class, maybe intentionally so as not to confuse an
> embedded-agent-specific type with generic flume agent types. In any case, the
> Flume User Guide says that the default value for source.type is "embedded".
> 3. Search for "pulAll" in comments. Should be "putAll".
> 4. Search for "recommended source" in comments. Should be "only source".
> 5. Search for "File based channel which stores events in heap" in comments.
> Should change 'in heap' to 'on local disk'.
> 6. Search for "Choices are 'default' (failover) or 'load_balance'". AFAIK,
> "default", "failover", and "load_balance" are 3 separate sink processor
> types, and so 'default' is not the same as 'failover'. The
> ALLOWED_SINK_PROCESSORS array seems to support this thinking since it also 3
> entries.
> 8. In configure(), why do we define "Joiner joiner = Joiner.on(SEPARATOR)"
> since the JOINER constant has already be defined. Would it make sense just to
> use the constant instead?
> 9. "point agent at source" comment is repeated twice. The second time it
> should be "point agent at sinks"
> 10. "the the" => "the"
> 11. Search for "properties.remove(key)". Wouldn't this give an error if the
> user passes-in an ImmutableMap instance to EmbeddedAgent.configure(..), which
> eventually delegates that same map to
> EmbeddedAgentConfiguration.configure(..)? I imagine that it's entirely
> reasonable for a user to do this, especially given a functional programming
> mindset.
> 12. Search for "key.startsWith(sink)". Should be
> "key.startsWith(sink+SEPARATOR)". Otherwise, if you have for example 11 sinks
> numbered k1 through k11, then your 'key' var might point to value "k1", and
> then startsWith("k1") could find k11's prop values and store those into the
> k1 sink so that k1 sink would actually have some or all of k11's values it
> seems.
> 13. Similarly, please do search-and-replace for:
> key.startsWith(SOURCE) => key.startsWith(SOURCE_PREFIX) .. (1 place)
> key.startsWith(CHANNEL) => key.startsWith(CHANNEL_PREFIX) .. (1 place)
> key.startsWith(SINK_PROCESSOR) => key.startsWith(SINK_PROCESSOR_PREFIX) .. (1
> place)
> 14. Please do search-and-replace for:
> key.replace(SOURCE, sourceName) => key.replaceFirst(SOURCE, sourceName)
> key.replace(CHANNEL, channelName) => key.replaceFirst(CHANNEL, channelName)
> ...since the intent is to replace only the first instance rather than all of
> them, right?
> 15. The strings in the embedded agent's original config file that start with
> user-defined chars are the sink aliases like k1, k2, etc. All other lines
> must begin with one of "source", "channel", "processor", or "sinks". From
> what I can tell looking at the EmbeddedAgentConfiguration code, a user must
> not specify a sink alias name that matches one "source", "channel", or
> "processor" strings.. otherwise EmbeddedAgentConfiguration would seem to have
> issues.
> 16. There's an "XXX" comment that shows that we throw a FlumeException when
> the user-specified embedded agent config contains an unexpected prop. The
> comment say "XXX should we simply ignore this?" meaning that we're currently
> doing strict checking of the user-provided props. If we plan to continue
> doing this, could we please remove the comment?
> Thank you :-)
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
