-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3516/#review4847
-----------------------------------------------------------
Everything looks good to me, except your ChannelException change broke one of
the jdbc tests:
TestJdbcChannelProvider
org.apache.flume.channel.jdbc.TestJdbcChannelProvider
testDerbyChannelCapacity(org.apache.flume.channel.jdbc.TestJdbcChannelProvider)
java.lang.NoSuchMethodError:
org.apache.flume.ChannelException.<init>(Ljava/lang/String;Ljava/lang/Exception;)V
at
org.apache.flume.channel.jdbc.JdbcChannelException.<init>(JdbcChannelException.java:29)
at
org.apache.flume.channel.jdbc.impl.JdbcChannelProviderImpl.persistEvent(JdbcChannelProviderImpl.java:245)
at
org.apache.flume.channel.jdbc.TestJdbcChannelProvider.testDerbyChannelCapacity(TestJdbcChannelProvider.java:113)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:616)
at
org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:45)
at
org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
at
org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:42)
at
org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
at
org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
at
org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:30)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:263)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:68)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:47)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:231)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:60)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:229)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:50)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:222)
at org.junit.runners.ParentRunner.run(ParentRunner.java:300)
at
org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:50)
at
org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
at
org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
at
org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
at
org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
at
org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197)
Apart from that I think it looks good, hopefully we'll hear from one of the
committers.
- Juhani
On 2012-02-06 23:09:15, Peter Newcomb wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3516/
> -----------------------------------------------------------
>
> (Updated 2012-02-06 23:09:15)
>
>
> Review request for Flume.
>
>
> Summary
> -------
>
> Implementation of FLUME-935 as new classes BasicChannelSemantics,
> BasicTransactionSemantics, and ChannelUtils. It might be better to fold
> BasicChannelSemantics into AbstractChannel and rename
> BasicTransactionSemantics to AbstractTransaction, but doing that would
> require refactoring of existing classes that extend AbstractChannel.
>
>
> This addresses bug FLUME-935.
> https://issues.apache.org/jira/browse/FLUME-935
>
>
> Diffs
> -----
>
>
> /branches/flume-728/flume-ng-core/src/test/java/org/apache/flume/channel/AbstractBasicChannelSemanticsTest.java
> PRE-CREATION
>
> /branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/channel/ChannelUtils.java
> PRE-CREATION
>
> /branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/ChannelException.java
> 1241246
>
> /branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/channel/BasicChannelSemantics.java
> PRE-CREATION
>
> /branches/flume-728/flume-ng-core/src/main/java/org/apache/flume/channel/BasicTransactionSemantics.java
> PRE-CREATION
>
> /branches/flume-728/flume-ng-core/src/test/java/org/apache/flume/channel/TestBasicChannelSemantics.java
> PRE-CREATION
>
> /branches/flume-728/flume-ng-core/src/test/java/org/apache/flume/channel/TestChannelUtils.java
> PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3516/diff
>
>
> Testing
> -------
>
> I am using these in production code, and they have survived significant
> integration testing there, including failure modes. Note also that these
> classes are largely error handling and precondition testing code designed to
> test the correctness of the code around them.
>
> A fairly comprehensive set of unit tests is included.
>
>
> Thanks,
>
> Peter
>
>
