> On Jan. 6, 2015, 12:48 a.m., Chris Riccomini wrote:
> > samza-sql/src/main/java/org/apache/samza/sql/api/operators/RelationRelationOperator.java,
> >  line 19
> > <https://reviews.apache.org/r/29592/diff/1/?file=806961#file806961line19>
> >
> >     I'm not crazy about setter methods, in general. I prefer keeping 
> > classes immutable, if possible. How often are these setNextOp methods 
> > getting called? Who calls them?
> 
> Yi Pan (Data Infrastructure) wrote:
>     Yeah... This has been the biggest debatable point in my trial. I used 
> this setter method with one use case in mind: allowing users to dynamically 
> connecting the operators on-demand. I have tried to put the next operator in 
> the class constructors and/or init methods before. It creates two issues: a) 
> the constructor now requires an abstract class to be exposed as part of API 
> and we do not have a clean set of interface-only classes as our APIs; b) if 
> putting the next operators in init() method, each different type of operator 
> class now need a different signature for the init() method.
>     
>     Now, thinking of it more, I would agree with you that the use case of 
> dynamically connecting operators on-demand does not seem to be a popular one. 
> Let me try to remove this setter method here.
> 
> Chris Riccomini wrote:
>     It sounds like you're still considering handling the message routing from 
> within the operator, which I agree would add complexity since you still need 
> to pass operators in through the constructor, or something. What if we remove 
> the routing from the operator API all together?

Yes. That's another possible solution, if we remove the need to support dynamic 
routing between the operators. Let me try on that.


- Yi


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


On Jan. 5, 2015, 10:19 p.m., Yi Pan (Data Infrastructure) wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29592/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2015, 10:19 p.m.)
> 
> 
> Review request for samza, Chris Riccomini, Navina Ramesh, and Naveen 
> Somasundaram.
> 
> 
> Bugs: SAMZA-482
>     https://issues.apache.org/jira/browse/SAMZA-482
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> StreamSQL operator API draft 
> - This is the first draft of the StreamSQL operator APIs 
> - org.apache.samza.sql.api.* contains definitions of all interface classes 
> - org.apache.samza.sql.operators.* are skeleton implementation of some 
> example build-in operators 
> - src/test/java contains the example application (a stream-join application) 
> using the above APIs
> 
> 
> Diffs
> -----
> 
>   build.gradle 38383bd9e3f0847d6088a4ea4c1ee6f3dcd1e430 
>   samza-sql/src/main/java/org/apache/samza/sql/api/data/Relation.java 
> PRE-CREATION 
>   samza-sql/src/main/java/org/apache/samza/sql/api/data/RelationSpec.java 
> PRE-CREATION 
>   samza-sql/src/main/java/org/apache/samza/sql/api/data/RelationStore.java 
> PRE-CREATION 
>   samza-sql/src/main/java/org/apache/samza/sql/api/data/StreamSpec.java 
> PRE-CREATION 
>   samza-sql/src/main/java/org/apache/samza/sql/api/data/Tuple.java 
> PRE-CREATION 
>   samza-sql/src/main/java/org/apache/samza/sql/api/operators/Operator.java 
> PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/api/operators/RelationOperator.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/api/operators/RelationRelationOperator.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/api/operators/RelationTupleOperator.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/api/operators/SqlOperatorFactory.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/api/operators/TupleOperator.java 
> PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/api/operators/TupleRelationOperator.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/api/operators/TupleTupleOperator.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/api/operators/spec/OperatorSpec.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/api/operators/spec/RelationOperatorSpec.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/api/operators/spec/RelationRelationSpec.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/api/operators/spec/RelationTupleSpec.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/api/operators/spec/TupleOperatorSpec.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/api/operators/spec/TupleRelationSpec.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/api/operators/spec/TupleTupleSpec.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/operators/factory/SimpleOperatorFactoryImpl.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/operators/output/SystemStreamOp.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/operators/output/SystemStreamSpec.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/operators/partition/PartitionOp.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/operators/partition/PartitionSpec.java
>  PRE-CREATION 
>   samza-sql/src/main/java/org/apache/samza/sql/operators/relation/Join.java 
> PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/operators/relation/JoinSpec.java 
> PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/operators/stream/InsertStream.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/operators/stream/InsertStreamSpec.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/operators/window/BoundedTimeWindow.java
>  PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/operators/window/WindowSpec.java 
> PRE-CREATION 
>   
> samza-sql/src/main/java/org/apache/samza/sql/operators/window/WindowState.java
>  PRE-CREATION 
>   samza-sql/src/main/java/org/apache/samza/task/SqlTaskContext.java 
> PRE-CREATION 
>   samza-sql/src/test/java/org/apache/samza/sql/data/IncomingMessageTuple.java 
> PRE-CREATION 
>   samza-sql/src/test/java/org/apache/samza/sql/store/SQLRelationStore.java 
> PRE-CREATION 
>   samza-sql/src/test/java/org/apache/samza/sql/task/StreamSqlTask.java 
> PRE-CREATION 
>   settings.gradle 3a01fd66359b8c79954ae8f34eeaf4b2e3fdc0b4 
> 
> Diff: https://reviews.apache.org/r/29592/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Yi Pan (Data Infrastructure)
> 
>

Reply via email to