You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@streams.apache.org by Ryan Ebanks <ry...@gmail.com> on 2014/02/21 21:05:03 UTC

Review Request 18369: Issue STREAMS-26 in JIRA

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18369/
-----------------------------------------------------------

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.


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


Re: Review Request 18369: Issue STREAMS-26 in JIRA

Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
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
> 
>


Re: Review Request 18369: Issue STREAMS-26 in JIRA

Posted by Ryan Ebanks <ry...@gmail.com>.
-----------------------------------------------------------
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 (updated)
-------

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