> On Dec. 9, 2012, 12:24 a.m., Mike Percy wrote: > > Brock, looks very nice overall. > > > > Some general suggestions: > > 1. Can we try to add a Javadoc at the top of every interface and class? It > > helps to specifies its purpose for other devs / reviewers. > > 2. Does the EmbeddedAgentConfiguration support making the channel > > "optional" via a channel selector? I can't tell. If not, can we add support > > for that? > > 3. Needs a doc section in the Flume developer guide > > > > Nit: a few whitespace red things on RB > > > > Other specific feedback inline. > >
Hi Mike, Thank you for the feedback! Regarding 1, 3, and the whitespace I agree whole heartedly. As I said in in the RB description these are items I plan on addressing in the next iteration. This iteration was to solicit feedback on the approach. Regarding #2, in the design document we specified only a single channel would be allowed. If we allowed that channel to be optional I'd think the user might as well use the RPCClient. What do you think? I'll address your feedback in the next iteration of the patch! Thanks again! Brock - Brock ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8127/#review14085 ----------------------------------------------------------- On Nov. 19, 2012, 7:42 p.m., Brock Noland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8127/ > ----------------------------------------------------------- > > (Updated Nov. 19, 2012, 7:42 p.m.) > > > Review request for Flume. > > > Description > ------- > > Patch is on top of the FLUME-1502 branch which has 1630 committed to it. > > This patch creates a module flume-ng-embedded-agent which implements > FLUME-1502. The patch is not read for commit. Specifically, it needs the > following: > > 1) whitespace removal > 2) docs in package-info.java and the developer guide > > However, I'd like to start soliciting feedback as these are not items which > will affect it's overall design. > > > This addresses bug FLUME-1502. > https://issues.apache.org/jira/browse/FLUME-1502 > > > Diffs > ----- > > > flume-ng-configuration/src/main/java/org/apache/flume/conf/FlumeConfiguration.java > 9b209e8 > flume-ng-core/src/main/java/org/apache/flume/sink/SinkProcessorFactory.java > f39b919 > flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java 47ccf9f > flume-ng-dist/pom.xml d929c88 > flume-ng-embedded-agent/pom.xml PRE-CREATION > > flume-ng-embedded-agent/src/main/java/org/apache/flume/agent/embedded/AvroLocalSource.java > PRE-CREATION > > flume-ng-embedded-agent/src/main/java/org/apache/flume/agent/embedded/EmbeddedAgent.java > PRE-CREATION > > flume-ng-embedded-agent/src/main/java/org/apache/flume/agent/embedded/EmbeddedAgentConfiguration.java > PRE-CREATION > > flume-ng-embedded-agent/src/main/java/org/apache/flume/agent/embedded/EmbeddedSource.java > PRE-CREATION > > flume-ng-embedded-agent/src/main/java/org/apache/flume/agent/embedded/LocalSource.java > PRE-CREATION > > flume-ng-embedded-agent/src/main/java/org/apache/flume/agent/embedded/MaterializedConfigurationProvider.java > PRE-CREATION > > flume-ng-embedded-agent/src/main/java/org/apache/flume/agent/embedded/MemoryConfigurationProvider.java > PRE-CREATION > > flume-ng-embedded-agent/src/main/java/org/apache/flume/agent/embedded/package-info.java > PRE-CREATION > > flume-ng-embedded-agent/src/test/java/org/apache/flume/agent/embedded/TestEmbeddedAgentAvroSource.java > PRE-CREATION > > flume-ng-embedded-agent/src/test/java/org/apache/flume/agent/embedded/TestEmbeddedAgentConfiguration.java > PRE-CREATION > > flume-ng-embedded-agent/src/test/java/org/apache/flume/agent/embedded/TestEmbeddedAgentEmbeddedSource.java > PRE-CREATION > > flume-ng-embedded-agent/src/test/java/org/apache/flume/agent/embedded/TestEmbeddedAgentState.java > PRE-CREATION > flume-ng-embedded-agent/src/test/resources/log4j.properties PRE-CREATION > pom.xml 76b8351 > > Diff: https://reviews.apache.org/r/8127/diff/ > > > Testing > ------- > > Additional unit tests have been added and they pass. > > > Thanks, > > Brock Noland > >
