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



samza-sql-core/src/main/java/org/apache/samza/sql/api/data/EntityName.java
<https://reviews.apache.org/r/34500/#comment137181>

    How is this "anonymous" system used?



samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/OperatorSource.java
<https://reviews.apache.org/r/34500/#comment137270>

    OperatorSource and OperatorSink have the same method signatures. Is that 
even allowed in Java? It's kind of confusing, even though the implementation 
can be semantically different.



samza-sql-core/src/main/java/org/apache/samza/sql/operators/OperatorTopology.java
<https://reviews.apache.org/r/34500/#comment137271>

    Same question as before. Which method gets invoked when I do a 
<OperatorTopology_object>.getName() ?



samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/SimpleOperatorImpl.java
<https://reviews.apache.org/r/34500/#comment137187>

    Looks like you left behind some merge conflict statements in comments :)



samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/TopologyBuilder.java
<https://reviews.apache.org/r/34500/#comment137325>

    Can you add info on what an unbound input or unbound output is? I think it 
will be useful to add/move the comment from Line 205 and Line 218 here.



samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/TopologyBuilder.java
<https://reviews.apache.org/r/34500/#comment137339>

    Can we change this source() rather than stream() ?



samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/TopologyBuilder.java
<https://reviews.apache.org/r/34500/#comment137338>

    Can you explain when sink should be used?



samza-sql-core/src/test/java/org/apache/samza/task/sql/UserCallbacksSqlTask.java
<https://reviews.apache.org/r/34500/#comment137333>

    I thought TopologyBuilder was to abstract away the spec and provide a 
simplified API for a user implementing a simple SQL query. 
    Imo, this still seems pretty involved for a user concerned with just 
defining a simple join query. 
    
    I assumed we could have a builder pattern as below:
    
    ```
    TopologyBuilder builder = TopologyBuilder
                                .create()
                                .join(window("stream1", 10), window("stream2", 
10), List{joinKey1, joinKey2, ...})
                                .partition(partitionKey)
    .build()
    ```
    
    The idea here is that the build statement order determines the topology. 
The builder just validates and chains them together. 
    I can see that this can be a problem with running operators in parallel and 
possibly, make it hard for the user to understand the correct sequence of 
operators. 
    I am wondering if you think this kind of a model is possible. It would 
greatly simplify the API for most users. 
    Just wanted to put this comment out so that we can discuss further.



samza-sql-core/src/test/java/org/apache/samza/task/sql/UserCallbacksSqlTask.java
<https://reviews.apache.org/r/34500/#comment137337>

    Why do we need 2 instances of the TopologyBuilder here?
    I think this occurs because stream() and sink() method return 
OperatorSource type rather than a TopologyBuilder instance. 
    
    How differently does the TopologyBuilder handle the OperatorSource and the 
OperatorSink ?


- Navina Ramesh


On May 20, 2015, 11:13 p.m., Yi Pan (Data Infrastructure) wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34500/
> -----------------------------------------------------------
> 
> (Updated May 20, 2015, 11:13 p.m.)
> 
> 
> Review request for samza, Yan Fang, Chris Riccomini, Guozhang Wang, Milinda 
> Pathirage, Navina Ramesh, and Naveen Somasundaram.
> 
> 
> Bugs: SAMZA-552
>     https://issues.apache.org/jira/browse/SAMZA-552
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> SAMZA-552: added operator builder API
> - The current operator builder only supports single output DAG topology yet
> 
> 
> Diffs
> -----
> 
>   samza-sql-core/src/main/java/org/apache/samza/sql/api/data/EntityName.java 
> PRE-CREATION 
>   samza-sql-core/src/main/java/org/apache/samza/sql/api/data/Table.java 
> PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/Operator.java 
> PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/OperatorCallback.java
>  PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/OperatorRouter.java
>  PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/OperatorSink.java
>  PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/OperatorSource.java
>  PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/SimpleOperator.java
>  PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/sql/data/IncomingMessageTuple.java
>  PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/sql/operators/OperatorTopology.java
>  PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/NoopOperatorCallback.java
>  PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/SimpleOperatorImpl.java
>  PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/SimpleOperatorSpec.java
>  PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/SimpleRouter.java
>  PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/TopologyBuilder.java
>  PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/sql/operators/join/StreamStreamJoin.java
>  PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/sql/operators/join/StreamStreamJoinSpec.java
>  PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/sql/operators/partition/PartitionOp.java
>  PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/sql/operators/partition/PartitionSpec.java
>  PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/sql/operators/window/BoundedTimeWindow.java
>  PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/sql/operators/window/WindowSpec.java
>  PRE-CREATION 
>   
> samza-sql-core/src/main/java/org/apache/samza/task/sql/SimpleMessageCollector.java
>  PRE-CREATION 
>   
> samza-sql-core/src/test/java/org/apache/samza/task/sql/RandomWindowOperatorTask.java
>  PRE-CREATION 
>   samza-sql-core/src/test/java/org/apache/samza/task/sql/StreamSqlTask.java 
> PRE-CREATION 
>   
> samza-sql-core/src/test/java/org/apache/samza/task/sql/UserCallbacksSqlTask.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34500/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew clean build passed
> 
> 
> Thanks,
> 
> Yi Pan (Data Infrastructure)
> 
>

Reply via email to