falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Srikanth Sundarrajan" <srik...@hotmail.com>
Subject Re: Review Request 25384: Falcon Desinger Data Model and JAVA Apis design
Date Tue, 04 Nov 2014 08:46:53 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message