----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8627/#review14578 -----------------------------------------------------------
As you already have provided test cases for URIhandlers, can you add test case for URIHandlerService? Specifically the stripPath() incorrectly removes the scheme from the URI. Can you change it and provide a test case for the same?..Other comments related to patch are below.. http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java <https://reviews.apache.org/r/8627/#comment31009> I think the URI's for dataIn() el function are separated by comma. http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java <https://reviews.apache.org/r/8627/#comment31010> same comment as above http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/util/HCatURI.java <https://reviews.apache.org/r/8627/#comment31011> seems a false change http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionUpdatePushMissingDependency.java <https://reviews.apache.org/r/8627/#comment31012> references to yahoo should be removed from here and elsewhere in patch http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/dependency/TestFSURIHandler.java <https://reviews.apache.org/r/8627/#comment31013> @Test annotation..same comment for other relevant places in the patch - Virag Kothari On Dec. 16, 2012, 6:42 p.m., Rohini Palaniswamy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8627/ > ----------------------------------------------------------- > > (Updated Dec. 16, 2012, 6:42 p.m.) > > > Review request for oozie. > > > Description > ------- > > Changed partition from uri query param to path. > > > Note: > 1) Missed including testcases for URIHandler in OOZIE-1075 patch. Included > them as part of this. > > > This addresses bug OOZIE-1111. > https://issues.apache.org/jira/browse/OOZIE-1111 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/command/coord/CoordActionUpdatePushMissingDependency.java > 1422648 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java > 1422648 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/main/java/org/apache/oozie/util/HCatURI.java > 1422648 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionUpdatePushMissingDependency.java > 1422648 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/command/coord/TestCoordPushDependencyCheckXCommand.java > 1422648 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/coord/TestCoordCommandUtils.java > 1422648 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java > 1422648 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/dependency/TestFSURIHandler.java > PRE-CREATION > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/dependency/TestHCatURIHandler.java > PRE-CREATION > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/jms/TestHCatMessageHandler.java > 1422648 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestJMSAccessorService.java > 1422648 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestMetaDataAccessorService.java > 1422648 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/service/TestPartitionDependencyManagerService.java > 1422648 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XDataTestCase.java > 1422648 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/test/XHCatTestCase.java > 1422648 > > http://svn.apache.org/repos/asf/oozie/branches/hcat-intre/core/src/test/java/org/apache/oozie/util/TestHCatURI.java > 1422648 > > Diff: https://reviews.apache.org/r/8627/diff/ > > > Testing > ------- > > > Thanks, > > Rohini Palaniswamy > >
