> On Nov. 3, 2016, 6:05 a.m., Pallavi Rao wrote: > > client/src/main/java/org/apache/falcon/client/FalconClient.java, line 545 > > <https://reviews.apache.org/r/51424/diff/1-3/?file=1485665#file1485665line545> > > > > EffectiveTime query parameter is not added.
Added the query param. > On Nov. 3, 2016, 6:05 a.m., Pallavi Rao wrote: > > client/src/main/java/org/apache/falcon/client/FalconClient.java, line 926 > > <https://reviews.apache.org/r/51424/diff/1-3/?file=1485665#file1485665line926> > > > > Should be StringUtils.isBlank(type). Else, it will always throw an > > exception. Corrected it. > On Nov. 3, 2016, 6:05 a.m., Pallavi Rao wrote: > > common/src/main/java/org/apache/falcon/entity/EntityUtil.java, line 1211 > > <https://reviews.apache.org/r/51424/diff/1-3/?file=1485667#file1485667line1211> > > > > Nit : Typo in method name. HasCode should be HashCode Changed it. > On Nov. 3, 2016, 6:05 a.m., Pallavi Rao wrote: > > prism/src/main/java/org/apache/falcon/resource/proxy/SchedulableEntityManagerProxy.java, > > line 316 > > <https://reviews.apache.org/r/51424/diff/1-3/?file=1485692#file1485692line316> > > > > equalsIgnoreCase? Fixed it. > On Nov. 3, 2016, 6:05 a.m., Pallavi Rao wrote: > > common/src/main/java/org/apache/falcon/entity/EntityDictionaryUtil.java, > > line 201 > > <https://reviews.apache.org/r/51424/diff/3/?file=1541713#file1541713line201> > > > > Why does a method variable need to be concurrent? Not required. It was part of very early changes. Changed it. > On Nov. 3, 2016, 6:05 a.m., Pallavi Rao wrote: > > common/src/main/java/org/apache/falcon/entity/EntityDictionaryUtil.java, > > line 227 > > <https://reviews.apache.org/r/51424/diff/3/?file=1541713#file1541713line227> > > > > Nit: May be use debug level. Will help in figuring which libs were > > copied Changed it. > On Nov. 3, 2016, 6:05 a.m., Pallavi Rao wrote: > > common/src/main/java/org/apache/falcon/entity/EntityDictionaryUtil.java, > > line 237 > > <https://reviews.apache.org/r/51424/diff/3/?file=1541713#file1541713line237> > > > > As HDFS latencies are not predictable, either make the five minute > > configurable OR await without timeout. Made it configurable. > On Nov. 3, 2016, 6:05 a.m., Pallavi Rao wrote: > > prism/src/main/java/org/apache/falcon/resource/AbstractSchedulableEntityManager.java, > > line 370 > > <https://reviews.apache.org/r/51424/diff/3/?file=1541741#file1541741line370> > > > > equalsIgnoreCase? Corrected it. > On Nov. 3, 2016, 6:05 a.m., Pallavi Rao wrote: > > common/src/main/java/org/apache/falcon/entity/EntityDictionaryUtil.java, > > line 294 > > <https://reviews.apache.org/r/51424/diff/3/?file=1541713#file1541713line294> > > > > Shouldn't it be an exception if the base staging path itself is missing? Removing the check altogether as the paths created in prepareEntityBuildPath(). > On Nov. 3, 2016, 6:05 a.m., Pallavi Rao wrote: > > docs/src/site/twiki/restapi/EntityUpdate.twiki, line 15 > > <https://reviews.apache.org/r/51424/diff/3/?file=1541724#file1541724line15> > > > > Might want to add some additional documentation on the behavior. What > > happens to overlapping instances etc. Done. Updated it. > On Nov. 3, 2016, 6:05 a.m., Pallavi Rao wrote: > > common/src/main/java/org/apache/falcon/entity/EntityDictionaryUtil.java, > > line 211 > > <https://reviews.apache.org/r/51424/diff/3/?file=1541713#file1541713line211> > > > > Too much nesting. Hard to read. Can move the runnable to an inner class? Moved it to inner class. > On Nov. 3, 2016, 6:05 a.m., Pallavi Rao wrote: > > common/src/main/java/org/apache/falcon/entity/EntityUtil.java, line 1213 > > <https://reviews.apache.org/r/51424/diff/1-3/?file=1485667#file1485667line1213> > > > > Out of curiosity. Why hashCode and not directly use checksum? Its upto the filesystem implementation to use the checksum.tostring, which doesn't qualify for comparision in this case . > On Nov. 3, 2016, 6:05 a.m., Pallavi Rao wrote: > > webapp/src/main/java/org/apache/falcon/resource/SchedulableEntityManager.java, > > line 346 > > <https://reviews.apache.org/r/51424/diff/1-3/?file=1485700#file1485700line346> > > > > Why effectiveTime as a dimension? Not required. Removing it. > On Nov. 3, 2016, 6:05 a.m., Pallavi Rao wrote: > > oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java, > > line 1487 > > <https://reviews.apache.org/r/51424/diff/1-3/?file=1485688#file1485688line1487> > > > > How is effectiveTime in the future handled? Made changes to handle the same. - sandeep ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51424/#review154674 ----------------------------------------------------------- On Nov. 11, 2016, 4:38 a.m., sandeep samudrala wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51424/ > ----------------------------------------------------------- > > (Updated Nov. 11, 2016, 4:38 a.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/oozie/process/SparkProcessWorkflowBuilder.java > 51db75d > > 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 > src/conf/startup.properties 6d82516 > 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/cli/FalconCLIIT.java 5cdbf93 > webapp/src/test/java/org/apache/falcon/resource/EntityManagerJerseyIT.java > 876ada5 > > Diff: https://reviews.apache.org/r/51424/diff/ > > > Testing > ------- > > > Thanks, > > sandeep samudrala > >
