> On Dec. 21, 2012, 12:56 p.m., Rohini Palaniswamy wrote: > > branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java, > > line 287 > > <https://reviews.apache.org/r/8679/diff/2-3/?file=241547#file241547line287> > > > > To be removed
k > On Dec. 21, 2012, 12:56 p.m., Rohini Palaniswamy wrote: > > branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java, > > line 290 > > <https://reviews.apache.org/r/8679/diff/2-3/?file=241547#file241547line290> > > > > Thanks for cleaning up. It is annoying to see objects passed to methods > > to be updated. Makes it difficult to understand what is happening in code. > > > > Can you use StringBuilder instead of StringBuffer as there is no thread > > safety involved. > > > > It is good to return strings instead of StringBuffer as it is better > > to keep return types immutable. But not a big deal and fine if you are > > trying to avoid one more object construction. Yes, I understand its better to use stringbuilder as the variable is used locally and no thread safety issues will arise. But, there are couple of other places which requires this change. So, I thought of doing that in a separate JIRA as this patch has already become messed up with changes not related to original JIRA summary. > On Dec. 21, 2012, 12:56 p.m., Rohini Palaniswamy wrote: > > branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java, > > line 545 > > <https://reviews.apache.org/r/8679/diff/2-3/?file=241547#file241547line545> > > > > Can you move this just before dependencyList.put(pullOrPush, depList);. > > Just for sake of readability This method actually resolves the EL functions for datetimestamp and the uri validation is a part of this method itself. So if we move this method just above dependencyList.put (), we may have already created a JMS connection for determining push or pull dependency. > On Dec. 21, 2012, 12:56 p.m., Rohini Palaniswamy wrote: > > branches/hcat-intre/core/src/main/java/org/apache/oozie/dependency/HCatURIHandler.java, > > line 123 > > <https://reviews.apache.org/r/8679/diff/2-3/?file=241550#file241550line123> > > > > ErrorCode.E1025, uri, e k > On Dec. 21, 2012, 12:56 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-3/?file=241551#file241551line60> > > > > Earlier I meant removing mentioning thrift all together from code. i.e > > getting index of :// and doing a substring after that to remove the scheme. kk. > On Dec. 21, 2012, 12:56 p.m., Rohini Palaniswamy wrote: > > branches/hcat-intre/core/src/main/java/org/apache/oozie/dependency/FSURIHandler.java, > > line 123 > > <https://reviews.apache.org/r/8679/diff/3/?file=242056#file242056line123> > > > > extra space to be removed. k - Virag ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8679/#review14815 ----------------------------------------------------------- On Dec. 21, 2012, 1:48 a.m., Virag Kothari wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8679/ > ----------------------------------------------------------- > > (Updated Dec. 21, 2012, 1:48 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/FSURIHandler.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/dependency/URIHandler.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/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/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 > >
