[ 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