----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18369/#review35192 -----------------------------------------------------------
This is my first pass. There's a lot to digest here and I'll try to take another look at it this weekend. trunk/streams-core/src/main/java/org/apache/streams/core/StreamsPersistReader.java <https://reviews.apache.org/r/18369/#comment65639> Can you remove these if they aren't used anymore? trunk/streams-core/src/main/java/org/apache/streams/core/StreamsProvider.java <https://reviews.apache.org/r/18369/#comment65640> Please remove these. trunk/streams-core/src/main/java/org/apache/streams/core/builders/LocalStreamBuilder.java <https://reviews.apache.org/r/18369/#comment65644> ExecutorService.submit() will return a Future. Is it possible to wait on all of the Futures instead of iterating every 10 seconds and checking? trunk/streams-core/src/main/java/org/apache/streams/core/builders/LocalStreamBuilder.java <https://reviews.apache.org/r/18369/#comment65645> If this is meant to be implemented later, it's probably good to add a "TODO" inside the method body trunk/streams-core/src/main/java/org/apache/streams/core/builders/StreamBuilder.java <https://reviews.apache.org/r/18369/#comment65646> Small nit on empty lines trunk/streams-core/src/main/java/org/apache/streams/core/builders/StreamComponent.java <https://reviews.apache.org/r/18369/#comment65647> Note on style: this is probably better suited as a private constructor that takes the id. You can then call "this(id)" in all of the other constructors. - Stanton Sievers On Feb. 21, 2014, 8:14 p.m., Ryan Ebanks wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18369/ > ----------------------------------------------------------- > > (Updated Feb. 21, 2014, 8:14 p.m.) > > > Review request for streams. > > > Repository: streams > > > Description > ------- > > I made changes to simplify the interfaces for StreamsPersistWriter, > StreamsProcessor, and StreamsProvider. The reasons for the changes was to > create a simple framework for running an activity stream in a single JVM, and > to create a class that allowed a user to easily declare, and run their stream > in a few lines of of code. The interfaces have only slightly changed, mainly > its the removal of certain methods. The new changes, I believe this will > allow for easy mapping of activity streams to other projects frame work, such > as Storm, Hadoop, etc. The builder interface used for declare and launching > the stream will be easily extended to Storm. When the patch gets approved I > will work on implementing that. > > *The changes happen in streams-core and streams-util packages > ** The new interface changes create compile errors in the streams-storm and > streams-contrib package. Steve Blackmon, the author of most of those > classes, suggested I push my changes to be reviewed before making changes to > those packages. > > > Diffs > ----- > > trunk/streams-core/pom.xml 1570675 > trunk/streams-core/src/main/java/org/apache/streams/core/StreamsDatum.java > 1570675 > > trunk/streams-core/src/main/java/org/apache/streams/core/StreamsOperation.java > PRE-CREATION > > trunk/streams-core/src/main/java/org/apache/streams/core/StreamsPersistReader.java > 1570675 > > trunk/streams-core/src/main/java/org/apache/streams/core/StreamsPersistWriter.java > 1570675 > > trunk/streams-core/src/main/java/org/apache/streams/core/StreamsProcessor.java > 1570675 > > trunk/streams-core/src/main/java/org/apache/streams/core/StreamsProvider.java > 1570675 > > trunk/streams-core/src/main/java/org/apache/streams/core/builders/InvalidStreamException.java > PRE-CREATION > > trunk/streams-core/src/main/java/org/apache/streams/core/builders/LocalStreamBuilder.java > PRE-CREATION > > trunk/streams-core/src/main/java/org/apache/streams/core/builders/StreamBuilder.java > PRE-CREATION > > trunk/streams-core/src/main/java/org/apache/streams/core/builders/StreamComponent.java > PRE-CREATION > > trunk/streams-core/src/main/java/org/apache/streams/core/tasks/BaseStreamsTask.java > PRE-CREATION > > trunk/streams-core/src/main/java/org/apache/streams/core/tasks/StreamsMergeTask.java > PRE-CREATION > > trunk/streams-core/src/main/java/org/apache/streams/core/tasks/StreamsPersistWriterTask.java > PRE-CREATION > > trunk/streams-core/src/main/java/org/apache/streams/core/tasks/StreamsProcessorTask.java > PRE-CREATION > > trunk/streams-core/src/main/java/org/apache/streams/core/tasks/StreamsProviderTask.java > PRE-CREATION > > trunk/streams-core/src/main/java/org/apache/streams/core/tasks/StreamsTask.java > PRE-CREATION > > trunk/streams-core/src/test/java/org/apache/streams/core/builders/LocalStreamBuilderTest.java > PRE-CREATION > > trunk/streams-core/src/test/java/org/apache/streams/core/builders/ToyLocalBuilderExample.java > PRE-CREATION > > trunk/streams-core/src/test/java/org/apache/streams/core/tasks/BasicTasksTest.java > PRE-CREATION > > trunk/streams-core/src/test/java/org/apache/streams/core/test/processors/DoNothingProcessor.java > PRE-CREATION > > trunk/streams-core/src/test/java/org/apache/streams/core/test/processors/PassthroughDatumCounterProcessor.java > PRE-CREATION > > trunk/streams-core/src/test/java/org/apache/streams/core/test/providers/NumericMessageProvider.java > PRE-CREATION > > trunk/streams-core/src/test/java/org/apache/streams/core/test/writer/DatumCounterWriter.java > PRE-CREATION > > trunk/streams-core/src/test/java/org/apache/streams/core/test/writer/DoNothingWriter.java > PRE-CREATION > > trunk/streams-core/src/test/java/org/apache/streams/core/test/writer/SystemOutWriter.java > PRE-CREATION > > trunk/streams-util/src/main/java/org/apache/streams/util/SerializationUtil.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/18369/diff/ > > > Testing > ------- > > There are units for the new functionality and classes. I admit they are a > little 'hacky'. > > > Thanks, > > Ryan Ebanks > >
