> On March 10, 2013, 7:02 a.m., Cheolsoo Park wrote: > > client/src/main/java/org/apache/sqoop/client/SqoopClient.java, line 255 > > <https://reviews.apache.org/r/9717/diff/1/?file=264569#file264569line255> > > > > Wouldn't "getSubmissionStatus" be a better name for this method? > > Jarek Cecho wrote: > I feel that it would be incompatible with the other method names that are > in form "action" "object", e.g. "startSubmission", "updateJob", > "createConnection". That being said, I can definitely change it if you prefer > to, I personally prefer this way.
I agree that it would be good to keep the form of "(action)(object)". But "action" has to be a verb. "statusSubmission" doesn't make sense to me because "status" is not an action. If you could replace "status" with a verb, that would be great. I have thought about it but didn't have a good idea. So I suggested "getSubmissionStatus". > On March 10, 2013, 7:02 a.m., Cheolsoo Park wrote: > > client/src/main/java/org/apache/sqoop/client/request/SqoopRequests.java, > > lines 129-140 > > <https://reviews.apache.org/r/9717/diff/1/?file=264575#file264575line129> > > > > Can you factor out this into a separate method so that duplicate code > > can be removed in createConnectionApplyValidations and > > updateConnectionApplyValidations? > > > > Jarek Cecho wrote: > I would prefer to do it once SQOOP-919 will get committed, so that I can > use the newly introduced MFormList list class during refactorization. Would > be follow up JIRA acceptable for you at this point? Sure. If you're planning to do it later, that's fine. > On March 10, 2013, 7:02 a.m., Cheolsoo Park wrote: > > client/src/main/java/org/apache/sqoop/client/request/SqoopRequests.java, > > lines 179-190 > > <https://reviews.apache.org/r/9717/diff/1/?file=264575#file264575line179> > > > > The same here as above. Can you factor out this to remove duplicate > > code? > > Jarek Cecho wrote: > I would prefer to do it once SQOOP-919 will get committed, so that I can > use the newly introduced MFormList list class during refactorization. Would > be follow up JIRA acceptable for you at this point? Sure. If you're planning to do it later, that's fine. - Cheolsoo ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9717/#review17654 ----------------------------------------------------------- On March 10, 2013, 5:20 p.m., Jarek Cecho wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9717/ > ----------------------------------------------------------- > > (Updated March 10, 2013, 5:20 p.m.) > > > Review request for Sqoop. > > > Description > ------- > > I've created first simple SqoopClient API and altered shell to use it. I've > noticed that there is a lot of duplicate code in the shell, so I've tried to > clean it up. The changes are simple, however are spanning across all shell > files, thus making this patch bigger than necessary. > > This is first implementation of the SqoopClient API. I'm intending to improve > it, for example by caching objects that was already received (especially > connector info and resource bundles). As this patch is big enough already, I > would prefer to finish this functionality in follow up JIRA. > > > This addresses bug SQOOP-918. > https://issues.apache.org/jira/browse/SQOOP-918 > > > Diffs > ----- > > client/src/main/java/org/apache/sqoop/client/SqoopClient.java PRE-CREATION > client/src/main/java/org/apache/sqoop/client/core/Constants.java > c364aa8856bf02f501b0ca2f3630c8e8c6fd10b1 > client/src/main/java/org/apache/sqoop/client/core/Environment.java > 5d1af26a0e8f96c11c7ef7d8fcfd0013db5ccce6 > client/src/main/java/org/apache/sqoop/client/core/RequestCache.java > 808b9f15d52ba9d3620e094a3640b6a93e2b0848 > client/src/main/java/org/apache/sqoop/client/request/ConnectionRequest.java > f1e4d685eb0466e916ede21e23f6766f6b263c27 > client/src/main/java/org/apache/sqoop/client/request/ConnectorRequest.java > 9ea9d5db91cb28337ef4b6d997af9d0cd0ef7781 > client/src/main/java/org/apache/sqoop/client/request/JobRequest.java > c2449f5106e2918b5e5068ff41733def68180804 > client/src/main/java/org/apache/sqoop/client/request/SqoopRequests.java > PRE-CREATION > client/src/main/java/org/apache/sqoop/client/request/SubmissionRequest.java > 60dcbb29cb75106c58d4de29a6160c1976bc4d67 > client/src/main/java/org/apache/sqoop/client/shell/CloneCommand.java > abec66b4bdcd10bfe6a7b62e421a131083359b5e > > client/src/main/java/org/apache/sqoop/client/shell/CloneConnectionFunction.java > 9d6c39627fc7c244adfc6a23a0d6470ab834ee9e > client/src/main/java/org/apache/sqoop/client/shell/CloneJobFunction.java > d06f05113cee801e0d956aa2f07f12f6bf748c46 > client/src/main/java/org/apache/sqoop/client/shell/CreateCommand.java > f4872d2822859739d0bf86c32d349190905d52ad > > client/src/main/java/org/apache/sqoop/client/shell/CreateConnectionFunction.java > 2c750d337a34a6e2c08f524dfa61a3c287f385d1 > client/src/main/java/org/apache/sqoop/client/shell/CreateJobFunction.java > 1f01cc75b5be572385c8ad3c6c20e33903e88fd6 > client/src/main/java/org/apache/sqoop/client/shell/DeleteCommand.java > 38b2fdade23a046e6f4e5c5fcfb823d88181db02 > > client/src/main/java/org/apache/sqoop/client/shell/DeleteConnectionFunction.java > 154acc7382335773f65569c045d7a4dff02eb728 > client/src/main/java/org/apache/sqoop/client/shell/DeleteJobFunction.java > 9d6c53e7fcf4e43753c20668697bdfb25168b927 > client/src/main/java/org/apache/sqoop/client/shell/HelpCommand.java > e14f738d5fd77ed7f7ec913cae2456aa4edd2123 > client/src/main/java/org/apache/sqoop/client/shell/SetCommand.java > 6c17e2515c6c034feb6569b6f8b40e53504b6412 > client/src/main/java/org/apache/sqoop/client/shell/SetOptionFunction.java > 5d5e9e7570e432c2e8c00c656153a2f1c09bb14c > client/src/main/java/org/apache/sqoop/client/shell/SetServerFunction.java > 0e1e027c04829cb9e00fc307d182dccc9c119fcf > client/src/main/java/org/apache/sqoop/client/shell/ShellEnvironment.java > PRE-CREATION > client/src/main/java/org/apache/sqoop/client/shell/ShowCommand.java > a043aa339b8617e17216627c425362f305e9ac1b > > client/src/main/java/org/apache/sqoop/client/shell/ShowConnectionFunction.java > af325d5431fa9ab3cc1cc1d58cdd99f393c84c50 > > client/src/main/java/org/apache/sqoop/client/shell/ShowConnectorFunction.java > c2464d2c16bf5573c21e23b470dc074826a15468 > > client/src/main/java/org/apache/sqoop/client/shell/ShowFrameworkFunction.java > 31de8dc6d0d5126188f796dafc026934c83237e6 > client/src/main/java/org/apache/sqoop/client/shell/ShowJobFunction.java > 0d67133c30b14755b7d47a30964b18a96e6a908a > client/src/main/java/org/apache/sqoop/client/shell/ShowOptionFunction.java > d8af0b22660f4367aa80dd22f4fdb1fc97c84fe0 > client/src/main/java/org/apache/sqoop/client/shell/ShowServerFunction.java > e1f6fa66cdfb99934b331eae32c5abc97550af67 > client/src/main/java/org/apache/sqoop/client/shell/ShowVersionFunction.java > 0dfc6c842acb1081c9c3d8c028a2a8e9be71e018 > client/src/main/java/org/apache/sqoop/client/shell/SqoopCommand.java > df9350f9680101269f57fbd52cd1e4f783f9dc1d > client/src/main/java/org/apache/sqoop/client/shell/SqoopFunction.java > 5632baca98ee9b4e883fec3a509a04186b0142c8 > client/src/main/java/org/apache/sqoop/client/shell/SqoopShell.java > b4d352c105ebe9e3f6806731e9246d50918b1409 > client/src/main/java/org/apache/sqoop/client/shell/SubmissionCommand.java > af4231d583da428d9c851163bb8d5822887c6b7f > > client/src/main/java/org/apache/sqoop/client/shell/SubmissionStartFunction.java > ba05dbbf8e21e1a4349e7a7f9f978ef5599452f1 > > client/src/main/java/org/apache/sqoop/client/shell/SubmissionStatusFunction.java > dd63cd1f594efb633e62aec2daa0c38b0b51288e > > client/src/main/java/org/apache/sqoop/client/shell/SubmissionStopFunction.java > e71f8bf8d18672c54cf8e512bc46d6e25732172b > client/src/main/java/org/apache/sqoop/client/shell/UpdateCommand.java > 0057efbbea8b2a3ac63bcaf58406b577918e0837 > > client/src/main/java/org/apache/sqoop/client/shell/UpdateConnectionFunction.java > dd0df24e45454abf570560a7ef7a59aa4a650a8c > client/src/main/java/org/apache/sqoop/client/shell/UpdateJobFunction.java > 4fac97726fad4a91380007f8512ab09785042b51 > client/src/main/java/org/apache/sqoop/client/utils/FormDisplayer.java > 4d0442fbabd4f416e766cebf31d89c0d6a8437ef > client/src/main/java/org/apache/sqoop/client/utils/FormFiller.java > e3fe02f91e5bf185323d928151f79f360581a20d > client/src/main/java/org/apache/sqoop/client/utils/SubmissionDisplayer.java > 8afb4926dd6cf01f1a353f0cbb451d121e4b6c90 > client/src/main/java/org/apache/sqoop/client/utils/TableDisplayer.java > 63d2ef4dc6059e8da95728b4a839a6624f710537 > client/src/main/java/org/apache/sqoop/client/utils/ThrowableDisplayer.java > 45c78fb26afedacec47074f7c7a714a77df45ec3 > client/src/main/resources/client-resource.properties > 58cfe55e6c147c0399cc199721932935a7281ae4 > test/src/test/java/org/apache/sqoop/integration/TomcatTestCase.java > 8b4617954cbc952bfb4739e91339ad475fc5a737 > > test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/TableImportTest.java > 20588d31da48ac93d979718c22fb4e179d6b2d19 > > Diff: https://reviews.apache.org/r/9717/diff/ > > > Testing > ------- > > Unit tests seems to be passing and I've tried the changes on real cluster. > > > Thanks, > > Jarek Cecho > >
