> On Nov. 5, 2012, 5:49 a.m., Mohammad Islam wrote:
> > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/service/MetadataServiceException.java,
> >  line 26
> > <https://reviews.apache.org/r/7801/diff/2/?file=185243#file185243line26>
> >
> >     replace HadoopAccessorService

silly mistake. thnx


> On Nov. 5, 2012, 5:49 a.m., Mohammad Islam wrote:
> > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java,
> >  line 47
> > <https://reviews.apache.org/r/7801/diff/2/?file=185244#file185244line47>
> >
> >     adding default in the prop name could be more meaning ful. like 
> > hcat.default.server.name

ok


> On Nov. 5, 2012, 5:49 a.m., Mohammad Islam wrote:
> > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java,
> >  line 129
> > <https://reviews.apache.org/r/7801/diff/2/?file=185244#file185244line129>
> >
> >     in place of calling two methods. Call one method like 
> > actionList.addActions(actionId), that will  add and update the last mod time

good point.done


> On Nov. 5, 2012, 5:49 a.m., Mohammad Islam wrote:
> > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java,
> >  line 134
> > <https://reviews.apache.org/r/7801/diff/2/?file=185244#file185244line134>
> >
> >     This is not a good idea. Wandering if passing templates <P, A> is good. 
> > Why can't do the actual type. Because very unlikely to support any other 
> > type.

Using generics in the class to make the code readable in addition to flexible.


> On Nov. 5, 2012, 5:49 a.m., Mohammad Islam wrote:
> > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java,
> >  line 222
> > <https://reviews.apache.org/r/7801/diff/2/?file=185244#file185244line222>
> >
> >     Similar one returns "boolean". this one is void. any reason?

had missed it. thanks


> On Nov. 5, 2012, 5:49 a.m., Mohammad Islam wrote:
> > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java,
> >  line 272
> > <https://reviews.apache.org/r/7801/diff/2/?file=185244#file185244line272>
> >
> >     why keeping the string as value for partition, in place of the object 
> > itself?

As we discussed, the downstream command (e.g. ActionCheck) can use the string 
directly for updating COORD_ACTION pushMissingDeps entry


> On Nov. 5, 2012, 5:49 a.m., Mohammad Islam wrote:
> > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/resources/oozie-default.xml,
> >  line 116
> > <https://reviews.apache.org/r/7801/diff/2/?file=185247#file185247line116>
> >
> >     ultimately this should not be in ooze-default.xml. because it is not a 
> > mandatory service

hmm okay taking it off. modified the testcase


> On Nov. 5, 2012, 5:49 a.m., Mohammad Islam wrote:
> > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java,
> >  line 140
> > <https://reviews.apache.org/r/7801/diff/2/?file=185244#file185244line140>
> >
> >     better to replace with something like this:
> >     tablePartitionsMap.put(partition, getActionList(partition, actionId))

these lines of code in createPartitionMapForTable are being repeated twice in 
the code, hence made it into a function


- Mona


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


On Nov. 4, 2012, 5:18 p.m., Mona Chitnis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7801/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2012, 5:18 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Description
> -------
> 
> Implementing the data structures for handling Partition-based dependencies 
> and interfacing methods which are used by all other services
> 
> 
> This addresses bug OOZIE-1039.
>     https://issues.apache.org/jira/browse/OOZIE-1039
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/pom.xml 
> 1404444 
>   
> http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/ErrorCode.java
>  1404444 
>   
> http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/service/MetadataServiceException.java
>  PRE-CREATION 
>   
> http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java
>  PRE-CREATION 
>   
> http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/util/HCatURI.java
>  1404444 
>   
> http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/util/WaitingActions.java
>  PRE-CREATION 
>   
> http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/resources/oozie-default.xml
>  1404444 
>   
> http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java
>  PRE-CREATION 
>   http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/pom.xml 1404444 
> 
> Diff: https://reviews.apache.org/r/7801/diff/
> 
> 
> Testing
> -------
> 
> unit tests included
> 
> 
> Thanks,
> 
> Mona Chitnis
> 
>

Reply via email to