----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25384/#review52466 -----------------------------------------------------------
addons/designer/actions/src/main/java/org/apache/falcon/designer/action/configuration/EmailActionConfiguration.java <https://reviews.apache.org/r/25384/#comment91252> Why is this EmailActionConfiguration2 ? Is there a different version of this ? addons/designer/actions/src/main/java/org/apache/falcon/designer/action/configuration/EmailActionConfiguration.java <https://reviews.apache.org/r/25384/#comment91256> No XmlElement in this ? addons/designer/actions/src/main/java/org/apache/falcon/designer/action/configuration/TransformationActionConfiguration.java <https://reviews.apache.org/r/25384/#comment91255> typo ? addons/designer/pom.xml <https://reviews.apache.org/r/25384/#comment91250> Looks like there is a implicit assumption that this will work only for hadoop-2? Is that right? If we are making that assumption, it is better to discuss that independently, as that is a major decision. addons/designer/pom.xml <https://reviews.apache.org/r/25384/#comment91251> Fix formatting. Why would we turn off failOnViolation ? addons/designer/actions/src/main/java/org/apache/falcon/designer/action/configuration/EmailActionConfiguration.java <https://reviews.apache.org/r/25384/#comment101047> Typo ? Since all the classes are foundational, please write detailed class level and method level javadocs addons/designer/actions/src/main/java/org/apache/falcon/designer/action/configuration/TransformationActionConfiguration.java <https://reviews.apache.org/r/25384/#comment101048> Is there a cache of transformation? Why is this needed ? Or it simply implying the list of transformation configurations being held? addons/designer/actions/src/main/java/org/apache/falcon/designer/action/configuration/TransformationActionConfiguration.java <https://reviews.apache.org/r/25384/#comment101049> It should be possible to extend the transformation list beyond this list at run-time. Please consider changing this appropriately. addons/designer/core/src/main/java/org/apache/falcon/designer/core/primitive/builder/FlowBuilder.java <https://reviews.apache.org/r/25384/#comment101050> Consider using lombok. Check APL 2.0 compatibility before introducing addons/designer/core/src/main/java/org/apache/falcon/designer/core/service/FalconDesigner.java <https://reviews.apache.org/r/25384/#comment101051> Deleting a specific version seems very distrubing to me. addons/designer/pom.xml <https://reviews.apache.org/r/25384/#comment101044> 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 addons/designer/pom.xml <https://reviews.apache.org/r/25384/#comment101045> Replace tabs with white spaces. Also fix indendation addons/designer/pom.xml <https://reviews.apache.org/r/25384/#comment101046> We can't turn off checksytle & findbugs validations, especially given that these are newly written code. 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 ? - Srikanth Sundarrajan 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 > >
