> On Dec. 20, 2012, 4:15 p.m., Rohini Palaniswamy wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java,
> >  lines 115-116
> > <https://reviews.apache.org/r/8679/diff/2/?file=241548#file241548line115>
> >
> >     If there is an exception here, you should log and continue. Not throw 
> > error materializing.

kk..also adding validate check for uri before job materializes


> On Dec. 20, 2012, 4:15 p.m., Rohini Palaniswamy wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/HCatMessageHandler.java,
> >  line 60
> > <https://reviews.apache.org/r/8679/diff/2/?file=241551#file241551line60>
> >
> >     Don't replace a hardcoded scheme. Replace the scheme till ://

k


> On Dec. 20, 2012, 4:15 p.m., Rohini Palaniswamy wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java,
> >  line 99
> > <https://reviews.apache.org/r/8679/diff/2/?file=241553#file241553line99>
> >
> >     Does this ever return true? You are doing a uri.toString in 
> > HCatURIHandler and the map only has the authority. You should pass the URI 
> > and check for uri.getAuthority() in connSessionMap

uri.toString() evaluates to uri.getScheme()+"://"+uri.getAuthority, so 
works..but will make it explicit 


> On Dec. 20, 2012, 4:15 p.m., Rohini Palaniswamy wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java,
> >  lines 152-157
> > <https://reviews.apache.org/r/8679/diff/2/?file=241553#file241553line152>
> >
> >     Can initialize the default rule as a class variable while initializing 
> > the rules instead of doing it everytime.

kk..good suggestion..doing


> On Dec. 20, 2012, 4:15 p.m., Rohini Palaniswamy wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java,
> >  lines 327-330
> > <https://reviews.apache.org/r/8679/diff/2/?file=241553#file241553line327>
> >
> >     close the exception in a finally block and close quietly. i.e ignore 
> > exceptions while closing connections.

cannot close in finally..but ignoring silently in catch()


> On Dec. 20, 2012, 4:15 p.m., Rohini Palaniswamy wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java,
> >  line 459
> > <https://reviews.apache.org/r/8679/diff/2/?file=241554#file241554line459>
> >
> >     throw new MetadataServiceException(ErrorCode.E1025, hcatURI, e);

k


> On Dec. 20, 2012, 4:15 p.m., Rohini Palaniswamy wrote:
> > branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java,
> >  lines 289-291
> > <https://reviews.apache.org/r/8679/diff/2/?file=241554#file241554line289>
> >
> >     If there are more partitions available in between you will be returning 
> > false and logging a spurious warning in 
> > CoordActionUpdatePushMissingDependency that you were not able to remove 
> > even though it was removed. Better not return false in this case and return 
> > only if actionId could not be found.

discussed offline..works


- Virag


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


On Dec. 20, 2012, 2:53 a.m., Virag Kothari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8679/
> -----------------------------------------------------------
> 
> (Updated Dec. 20, 2012, 2:53 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Description
> -------
> 
> https://issues.apache.org/jira/browse/OOZIE-1138
> 
> 
> This addresses bug OOZIE-1138.
>     https://issues.apache.org/jira/browse/OOZIE-1138
> 
> 
> Diffs
> -----
> 
>   branches/hcat-intre/core/src/main/java/org/apache/oozie/ErrorCode.java 
> 1424271 
>   
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java
>  1424271 
>   
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java
>  1424271 
>   
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java
>  1424271 
>   
> branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordPushDependencyCheckXCommand.java
>  1424271 
>   
> branches/hcat-intre/core/src/main/java/org/apache/oozie/dependency/HCatURIHandler.java
>  1424271 
>   
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/HCatMessageHandler.java
>  1424271 
>   
> branches/hcat-intre/core/src/main/java/org/apache/oozie/jms/MessageReceiver.java
>  1424271 
>   
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/JMSAccessorService.java
>  1424271 
>   
> branches/hcat-intre/core/src/main/java/org/apache/oozie/service/PartitionDependencyManagerService.java
>  1424271 
>   
> branches/hcat-intre/core/src/main/java/org/apache/oozie/util/MappingRule.java 
> PRE-CREATION 
>   
> branches/hcat-intre/core/src/main/java/org/apache/oozie/util/PartitionWrapper.java
>  1424271 
>   branches/hcat-intre/core/src/main/resources/oozie-default.xml 1424271 
>   
> branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionUpdatePushMissingDependency.java
>  1424271 
>   
> branches/hcat-intre/core/src/test/java/org/apache/oozie/coord/TestCoordCommandUtils.java
>  1424271 
>   
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java
>  1424271 
>   
> branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestMessageReceiver.java
>  1424271 
>   
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java
>  1424271 
>   
> branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java
>  1424271 
> 
> Diff: https://reviews.apache.org/r/8679/diff/
> 
> 
> Testing
> -------
> 
> end to end test done and connections to jms server verified
> 
> 
> Thanks,
> 
> Virag Kothari
> 
>

Reply via email to