falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Pallavi Rao <pallavi....@inmobi.com>
Subject Re: Review Request 51424: Effective Time in Entity Update
Date Thu, 03 Nov 2016 06:05:36 GMT

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




client/src/main/java/org/apache/falcon/client/FalconClient.java (line 545)
<https://reviews.apache.org/r/51424/#comment224300>

    EffectiveTime query parameter is not added.



client/src/main/java/org/apache/falcon/client/FalconClient.java (line 926)
<https://reviews.apache.org/r/51424/#comment224301>

    Should be StringUtils.isBlank(type). Else, it will always throw an exception.



common/src/main/java/org/apache/falcon/entity/EntityUtil.java (line 1207)
<https://reviews.apache.org/r/51424/#comment224307>

    Nit : Typo in method name. HasCode should be HashCode



common/src/main/java/org/apache/falcon/entity/EntityUtil.java (line 1209)
<https://reviews.apache.org/r/51424/#comment224308>

    Out of curiosity. Why hashCode and not directly use checksum?



oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java (line 1473)
<https://reviews.apache.org/r/51424/#comment224319>

    How is effectiveTime in the future handled?



prism/src/main/java/org/apache/falcon/resource/proxy/SchedulableEntityManagerProxy.java (line
316)
<https://reviews.apache.org/r/51424/#comment224331>

    equalsIgnoreCase?



webapp/src/main/java/org/apache/falcon/resource/SchedulableEntityManager.java (line 346)
<https://reviews.apache.org/r/51424/#comment224332>

    Why effectiveTime as a dimension?



common/src/main/java/org/apache/falcon/entity/EntityDictionaryUtil.java (line 201)
<https://reviews.apache.org/r/51424/#comment224302>

    Why does a method variable need to be concurrent?



common/src/main/java/org/apache/falcon/entity/EntityDictionaryUtil.java (line 211)
<https://reviews.apache.org/r/51424/#comment224303>

    Too much nesting. Hard to read. Can move the runnable to an inner class?



common/src/main/java/org/apache/falcon/entity/EntityDictionaryUtil.java (line 227)
<https://reviews.apache.org/r/51424/#comment224305>

    Nit: May be use debug level. Will help in figuring which libs were copied



common/src/main/java/org/apache/falcon/entity/EntityDictionaryUtil.java (line 237)
<https://reviews.apache.org/r/51424/#comment224304>

    As HDFS latencies are not predictable, either make the five minute configurable OR await
without timeout.



common/src/main/java/org/apache/falcon/entity/EntityDictionaryUtil.java (line 294)
<https://reviews.apache.org/r/51424/#comment224306>

    Shouldn't it be an exception if the base staging path itself is missing?



docs/src/site/twiki/restapi/EntityUpdate.twiki (line 15)
<https://reviews.apache.org/r/51424/#comment224312>

    Might want to add some additional documentation on the behavior. What happens to overlapping
instances etc.



prism/src/main/java/org/apache/falcon/resource/AbstractSchedulableEntityManager.java (line
370)
<https://reviews.apache.org/r/51424/#comment224329>

    equalsIgnoreCase?


- Pallavi Rao


On Oct. 19, 2016, 6:43 p.m., sandeep samudrala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51424/
> -----------------------------------------------------------
> 
> (Updated Oct. 19, 2016, 6:43 p.m.)
> 
> 
> Review request for Falcon and Pallavi Rao.
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Effective Time in Entity Update
> 
> 
> Diffs
> -----
> 
>   cli/src/main/java/org/apache/falcon/cli/FalconCLI.java 0dd11f6 
>   cli/src/main/java/org/apache/falcon/cli/FalconEntityCLI.java a8aea52 
>   client/src/main/java/org/apache/falcon/client/AbstractFalconClient.java 5d6eff5 
>   client/src/main/java/org/apache/falcon/client/FalconCLIConstants.java 04f1599 
>   client/src/main/java/org/apache/falcon/client/FalconClient.java 8f77fad 
>   common/src/main/java/org/apache/falcon/entity/ClusterHelper.java f89def3 
>   common/src/main/java/org/apache/falcon/entity/EntityDictionaryUtil.java PRE-CREATION

>   common/src/main/java/org/apache/falcon/entity/EntityLibEntry.java PRE-CREATION 
>   common/src/main/java/org/apache/falcon/entity/EntityUtil.java 8fe316c 
>   common/src/main/java/org/apache/falcon/entity/ProcessHelper.java e563d18 
>   common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java 38fa3ae

>   common/src/main/java/org/apache/falcon/update/UpdateHelper.java 266319f 
>   common/src/main/java/org/apache/falcon/workflow/engine/AbstractWorkflowEngine.java
16a1753 
>   common/src/test/java/org/apache/falcon/entity/EntityDictionaryUtilTest.java PRE-CREATION

>   common/src/test/java/org/apache/falcon/update/UpdateHelperTest.java 826686f 
>   docs/src/site/twiki/falconcli/Touch.twiki afbd848 
>   docs/src/site/twiki/falconcli/UpdateEntity.twiki 146a60f 
>   docs/src/site/twiki/restapi/EntityUpdate.twiki cbf33db 
>   oozie/src/main/java/org/apache/falcon/oozie/OozieBundleBuilder.java 5f93cc2 
>   oozie/src/main/java/org/apache/falcon/oozie/feed/FeedBundleBuilder.java c758411 
>   oozie/src/main/java/org/apache/falcon/oozie/process/HiveProcessWorkflowBuilder.java
9f9579c 
>   oozie/src/main/java/org/apache/falcon/oozie/process/OozieProcessWorkflowBuilder.java
f93a599 
>   oozie/src/main/java/org/apache/falcon/oozie/process/PigProcessWorkflowBuilder.java
a1a7c12 
>   oozie/src/main/java/org/apache/falcon/oozie/process/ProcessBundleBuilder.java 6661dd5

>   oozie/src/main/java/org/apache/falcon/oozie/process/ProcessExecutionCoordinatorBuilder.java
91f4757 
>   oozie/src/main/java/org/apache/falcon/oozie/process/ProcessExecutionWorkflowBuilder.java
20eeffd 
>   oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java 394600c

>   oozie/src/test/java/org/apache/falcon/oozie/process/OozieProcessWorkflowBuilderTest.java
05b513e 
>   oozie/src/test/resources/config/process/dumb-hive-process.xml c504074 
>   oozie/src/test/resources/config/process/hive-process-FSInputFeed.xml d871377 
>   oozie/src/test/resources/config/process/hive-process-FSOutputFeed.xml 23d96c3 
>   oozie/src/test/resources/config/process/hive-process.xml 4dac8e9 
>   oozie/src/test/resources/config/process/pig-process-0.1.xml 8d20cee 
>   prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java aefd699 
>   prism/src/main/java/org/apache/falcon/resource/AbstractSchedulableEntityManager.java
3bdeb99 
>   prism/src/main/java/org/apache/falcon/resource/extensions/ExtensionManager.java 92b5531

>   prism/src/main/java/org/apache/falcon/resource/proxy/SchedulableEntityManagerProxy.java
07334d6 
>   scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java
9ba62a1 
>   shell/src/main/java/org/apache/falcon/shell/commands/FalconEntityCommands.java 35a6f2a

>   src/build/checkstyle.xml 292a0a3 
>   unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java 53073f0 
>   unit/src/main/java/org/apache/falcon/unit/LocalSchedulableEntityManager.java 7398c8a

>   unit/src/test/java/org/apache/falcon/unit/FalconUnitTestBase.java bfc8b08 
>   unit/src/test/java/org/apache/falcon/unit/TestFalconUnit.java 0bc7755 
>   unit/src/test/resources/process.xml 6854311 
>   webapp/src/main/java/org/apache/falcon/resource/ConfigSyncService.java 7b32bd5 
>   webapp/src/main/java/org/apache/falcon/resource/SchedulableEntityManager.java 5525207

>   webapp/src/test/java/org/apache/falcon/resource/EntityManagerJerseyIT.java 876ada5

> 
> Diff: https://reviews.apache.org/r/51424/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> sandeep samudrala
> 
>


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