> On Nov. 4, 2014, 8:46 a.m., Srikanth Sundarrajan wrote:
> > addons/designer/actions/src/main/java/org/apache/falcon/designer/action/configuration/EmailActionConfiguration.java,
> >  line 32
> > <https://reviews.apache.org/r/25384/diff/1/?file=680331#file680331line32>
> >
> >     No XmlElement in this ?

@XmlAccessorType(XmlAccessType.PROPERTY) is added at class level .. should take 
care of the fields that should be serialized


> On Nov. 4, 2014, 8:46 a.m., Srikanth Sundarrajan wrote:
> > addons/designer/actions/src/main/java/org/apache/falcon/designer/action/configuration/TransformationActionConfiguration.java,
> >  line 68
> > <https://reviews.apache.org/r/25384/diff/1/?file=680332#file680332line68>
> >
> >     typo ?

Will remove it


> On Nov. 4, 2014, 8:46 a.m., Srikanth Sundarrajan wrote:
> > addons/designer/actions/src/main/java/org/apache/falcon/designer/action/configuration/TransformationActionConfiguration.java,
> >  line 64
> > <https://reviews.apache.org/r/25384/diff/2/?file=683554#file683554line64>
> >
> >     It should be possible to extend the transformation list beyond this 
> > list at run-time. Please consider changing this appropriately.

have removed the static list. Will address this issue in a seperate ticket on 
ser/deserialization.


> On Nov. 4, 2014, 8:46 a.m., Srikanth Sundarrajan wrote:
> > addons/designer/pom.xml, line 105
> > <https://reviews.apache.org/r/25384/diff/2/?file=683604#file683604line105>
> >
> >     Depend on the minimum version of hadoop under 2.x. You are needing this 
> > only for fileSystem operation and it shouldn't require higher versions

what i can see on maven , 2.0 are all alpha version and 2.1 are all beta. Would 
it be ok to depend on them?


> On Nov. 4, 2014, 8:46 a.m., Srikanth Sundarrajan wrote:
> > addons/designer/pom.xml, line 647
> > <https://reviews.apache.org/r/25384/diff/1/?file=680382#file680382line647>
> >
> >     Fix formatting. Why would we turn off failOnViolation ?
> 
> samar kumar wrote:
>     Some of the existing code was failing with these violations. Wanted to 
> create a seperate ticket to resolve those and not add it as part of design 
> patch

will remove failOnViolation


> On Nov. 4, 2014, 8:46 a.m., Srikanth Sundarrajan wrote:
> > addons/designer/actions/src/main/java/org/apache/falcon/designer/action/configuration/TransformationActionConfiguration.java,
> >  line 45
> > <https://reviews.apache.org/r/25384/diff/2/?file=683554#file683554line45>
> >
> >     Is there a cache of transformation? Why is this needed ? Or it simply 
> > implying the list of transformation configurations being held?

removed with the new DAG design


> On Nov. 4, 2014, 8:46 a.m., Srikanth Sundarrajan wrote:
> > addons/designer/pom.xml, line 647
> > <https://reviews.apache.org/r/25384/diff/2/?file=683604#file683604line647>
> >
> >     We can't turn off checksytle & findbugs validations, especially given 
> > that these are newly written code.

Will remove this tag


On Nov. 4, 2014, 8:46 a.m., samar kumar wrote:
> > Overall observations on the design:
> > 
> > 1. ActionConfiguration seems to hold the transition information and that 
> > seem to burden actions with transition information as well. Would prefer 
> > that flow holds the list of actions and the transition information.
> > 2. Version of flows isn't following a stack model, that allows you to pop 
> > out newer changes and reverting to older version, instead it seem to be 
> > branching off while simultaneous
> > ly delinking from the parent. This has to be further debated and agreed on. 
> > Personally I can see some advantages with the current model, but there are 
> > also challenges. We need to
> >  be fully aware of the limitations and advantages before we proceed with 
> > one approach or the other
> > 3. Designer Service should have mechanism to create new actions or 
> > transforms. This is essentially to truly making this an extensible platform.
> > 4. Since all the classes are foundational, please write detailed class 
> > level and method level javadocs.
> > 5. Is the module name "core" required in the package name ?
> 
> samar kumar wrote:
>     5. Is the module name "core" required in the package name ? 
>     >>>> Assuming it is convention to have the module name as part of the 
> package name , have added core. Can remove it if suggested

3 Designer Service should have mechanism to create new actions or transforms. 
This is essentially to truly making this an extensible platform.
       Would have a UI /Rest call to add a new action/transformation. I was 
expecting it will like jar dropin.. dropped in a specific folder and loaded and 
hence no API exposed here.


- samar


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


On Sept. 9, 2014, 7:22 a.m., samar kumar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25384/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2014, 7:22 a.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-672
>     https://issues.apache.org/jira/browse/FALCON-672
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Contails Falcon Desinger Data Model and JAVA Apis design for the server/client
> 
> 
> Diffs
> -----
> 
>   addons/designer/actions/pom.xml 7d5afbb 
>   
> addons/designer/actions/src/main/java/org/apache/falcon/designer/action/configuration/EmailActionConfiguration.java
>  PRE-CREATION 
>   
> addons/designer/actions/src/main/java/org/apache/falcon/designer/action/configuration/TransformationActionConfiguration.java
>  PRE-CREATION 
>   
> addons/designer/actions/src/main/java/org/apache/falcon/designer/action/primitive/EmailAction.java
>  PRE-CREATION 
>   
> addons/designer/actions/src/main/java/org/apache/falcon/designer/action/primitive/builder/TransformationActionConfigurationBuilder.java
>  PRE-CREATION 
>   addons/designer/core/pom.xml ddd8814 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/configuration/Configuration.java
>  dba908a 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/configuration/ActionConfiguration.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/configuration/Configuration.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/configuration/Feed.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/configuration/FlowConfig.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/configuration/SerdeException.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/configuration/TransformConfiguration.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/primitive/Action.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/primitive/Code.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/primitive/CompilationException.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/primitive/Flow.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/primitive/Message.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/primitive/Primitive.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/primitive/Transform.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/primitive/builder/BuilderException.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/primitive/builder/FlowBuilder.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/schema/RelationalData.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/schema/RelationalSchema.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/service/FalconDesigner.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/service/impl/FalconDesignerImpl.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/source/DataSource.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/storage/Storage.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/storage/StorageException.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/storage/Storeable.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/storage/Version.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/storage/VersionedStorage.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/storage/impl/HDFSStorage.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/core/sysconfig/SystemConfiguration.java
>  PRE-CREATION 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/primitive/Action.java
>  c40e462 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/primitive/Code.java
>  35eeeb1 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/primitive/CompilationException.java
>  603225b 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/primitive/Message.java
>  e5a68a8 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/primitive/Primitive.java
>  aa2b988 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/primitive/Transform.java
>  72cf988 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/schema/RelationalData.java
>  d930e40 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/schema/RelationalSchema.java
>  f4f44d1 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/source/DataSource.java
>  227277c 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/storage/Storage.java
>  5b63b31 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/storage/StorageException.java
>  c8c2f58 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/storage/Storeable.java
>  384d17a 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/storage/Version.java
>  35c2e86 
>   
> addons/designer/core/src/main/java/org/apache/falcon/designer/storage/VersionedStorage.java
>  7f5edc5 
>   
> addons/designer/core/src/test/java/org/apache/falcon/designer/core/storage/impl/HDFSStorageTest.java
>  PRE-CREATION 
>   addons/designer/examples/pom.xml PRE-CREATION 
>   
> addons/designer/examples/src/main/java/org/apache/falcon/designer/examples/flow/SimpleFlowExample.java
>  PRE-CREATION 
>   addons/designer/flows/pom.xml ce706a3 
>   addons/designer/pom.xml 3e1a98a 
>   addons/designer/transforms/pom.xml 6f55129 
>   
> addons/designer/transforms/src/main/java/org/apache/falcon/designer/transformation/configuration/CoGroupTransformation.java
>  PRE-CREATION 
>   
> addons/designer/transforms/src/main/java/org/apache/falcon/designer/transformation/configuration/FilterTransformation.java
>  PRE-CREATION 
>   
> addons/designer/transforms/src/main/java/org/apache/falcon/designer/transformation/configuration/GroupByTransformation.java
>  PRE-CREATION 
>   
> addons/designer/transforms/src/main/java/org/apache/falcon/designer/transformation/configuration/JoinTransformation.java
>  PRE-CREATION 
>   
> addons/designer/transforms/src/main/java/org/apache/falcon/designer/transformation/configuration/ProjectionTransformation.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25384/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> samar kumar
> 
>

Reply via email to