----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27330/#review58985 -----------------------------------------------------------
Good first cut. There are a lot of renames that don't seem to be part of this Jira. Can we do them in a separate Jira? Also, your IDE may be able to remove extra white spaces for you. common/src/main/java/org/apache/sqoop/json/JobBean.java <https://reviews.apache.org/r/27330/#comment100241> in* core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java <https://reviews.apache.org/r/27330/#comment100244> These renames do not seem relevant core/src/main/java/org/apache/sqoop/repository/Repository.java <https://reviews.apache.org/r/27330/#comment100243> This change does not seem relevant? repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java <https://reviews.apache.org/r/27330/#comment100245> sp repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java <https://reviews.apache.org/r/27330/#comment100246> sp server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java <https://reviews.apache.org/r/27330/#comment100247> sp server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java <https://reviews.apache.org/r/27330/#comment100248> sp server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java <https://reviews.apache.org/r/27330/#comment100249> sp server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java <https://reviews.apache.org/r/27330/#comment100250> sp server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java <https://reviews.apache.org/r/27330/#comment100252> Maybe add a condition to skip hitting the database? i.e. if (job.getPersistanceId() == -1) { ... } server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java <https://reviews.apache.org/r/27330/#comment100251> sp server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java <https://reviews.apache.org/r/27330/#comment100253> space server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java <https://reviews.apache.org/r/27330/#comment100255> The identifier should always be name? In this case, the identifier will always be a string. server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java <https://reviews.apache.org/r/27330/#comment100256> Maybe other way around? try { return Long.valueOf(identifier); } catch(NumberFormatException ex) { if (repository.findJob(identifier) != null) { return repository.findJob(identifier).getPersistenceId(); } else { throw new SqoopException } } Otherwise, if the job doesn't exist, just throw an exception? server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java <https://reviews.apache.org/r/27330/#comment100257> sp server/src/main/java/org/apache/sqoop/server/v1/JobServlet.java <https://reviews.apache.org/r/27330/#comment100258> sp server/src/main/java/org/apache/sqoop/server/v1/JobServlet.java <https://reviews.apache.org/r/27330/#comment100259> sp shell/src/main/java/org/apache/sqoop/shell/UpdateJobFunction.java <https://reviews.apache.org/r/27330/#comment100260> rm? shell/src/main/java/org/apache/sqoop/shell/UpdateLinkFunction.java <https://reviews.apache.org/r/27330/#comment100261> rm? I'd update SQOOP-1634 with a little bit of detail just in case. - Abraham Elmahrek On Oct. 29, 2014, 2:15 p.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27330/ > ----------------------------------------------------------- > > (Updated Oct. 29, 2014, 2:15 p.m.) > > > Review request for Sqoop. > > > Repository: sqoop-sqoop2 > > > Description > ------- > > see JIRA and its parent JIRA for details > > > Diffs > ----- > > > client/src/main/java/org/apache/sqoop/client/request/JobResourceRequest.java > 83c08b3 > > client/src/main/java/org/apache/sqoop/client/request/SqoopResourceRequests.java > 4a56bb7 > > client/src/main/java/org/apache/sqoop/client/request/SubmissionResourceRequest.java > 5055783 > common/src/main/java/org/apache/sqoop/json/JobBean.java 082d591 > common/src/main/java/org/apache/sqoop/json/JobsBean.java PRE-CREATION > common/src/main/java/org/apache/sqoop/json/JsonBean.java ba86511 > common/src/main/java/org/apache/sqoop/json/LinksBean.java 5858a18 > common/src/main/java/org/apache/sqoop/json/SubmissionBean.java 4b80338 > common/src/main/java/org/apache/sqoop/json/SubmissionsBean.java > PRE-CREATION > common/src/main/java/org/apache/sqoop/model/MLink.java 7a9f538 > common/src/main/java/org/apache/sqoop/model/MSubmission.java 7290df5 > common/src/test/java/org/apache/sqoop/json/TestJobBean.java 1fc8dbd > common/src/test/java/org/apache/sqoop/json/TestLinkBean.java 811cbf0 > common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java e4d50bf > core/src/main/java/org/apache/sqoop/driver/JobManager.java f6447c6 > core/src/main/java/org/apache/sqoop/driver/SubmissionEngine.java 3a32e9f > core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 2aeb07e > core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java > ad380d3 > core/src/main/java/org/apache/sqoop/repository/Repository.java 79742b9 > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java > 514b5ac > > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaInsertUpdateDeleteSelectQuery.java > c894d06 > > repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestSubmissionHandling.java > 4c2d062 > server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java > 5547988 > server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java > 35a9635 > server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java > 8555b0c > server/src/main/java/org/apache/sqoop/server/RequestHandler.java 508edd2 > server/src/main/java/org/apache/sqoop/server/v1/JobServlet.java d295237 > server/src/main/java/org/apache/sqoop/server/v1/JobsServlet.java > PRE-CREATION > server/src/main/java/org/apache/sqoop/server/v1/LinkServlet.java 127903a > server/src/main/java/org/apache/sqoop/server/v1/SubmissionServlet.java > 5c1d883 > server/src/main/java/org/apache/sqoop/server/v1/SubmissionsServlet.java > PRE-CREATION > server/src/main/webapp/WEB-INF/web.xml 6ad90d2 > shell/src/main/java/org/apache/sqoop/shell/UpdateJobFunction.java dd075d7 > shell/src/main/java/org/apache/sqoop/shell/UpdateLinkFunction.java 176833a > > submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java > 631ceca > tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryDumpTool.java > c219e68 > tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryLoadTool.java > 64b08fc > > Diff: https://reviews.apache.org/r/27330/diff/ > > > Testing > ------- > > yes > > > Thanks, > > Veena Basavaraj > >
