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

Hudson commented on FLUME-1852:
-------------------------------

Integrated in flume-trunk #357 (See 
[https://builds.apache.org/job/flume-trunk/357/])
    FLUME-1852. Issues with EmbeddedAgentConfiguration. (Revision 
3df65e12c8d480cd46f190a0bb4addfee4272062)

     Result = SUCCESS
mpercy : 
http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=3df65e12c8d480cd46f190a0bb4addfee4272062
Files : 
* 
flume-ng-embedded-agent/src/main/java/org/apache/flume/agent/embedded/EmbeddedAgentConfiguration.java
* 
flume-ng-embedded-agent/src/test/java/org/apache/flume/agent/embedded/TestEmbeddedAgentEmbeddedSource.java
* 
flume-ng-embedded-agent/src/test/java/org/apache/flume/agent/embedded/TestEmbeddedAgentConfiguration.java
* 
flume-ng-embedded-agent/src/main/java/org/apache/flume/agent/embedded/package-info.java

                
> 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
>
>         Attachments: FLUME-1852-0.patch
>
>
> 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

Reply via email to