> On Oct. 29, 2014, 8:01 a.m., Abraham Elmahrek wrote: > > core/src/main/java/org/apache/sqoop/repository/Repository.java, lines > > 427-443 > > <https://reviews.apache.org/r/27330/diff/1/?file=740285#file740285line427> > > > > This change does not seem relevant?
its just consistency and cleanup. Are we going to be this anal about a rename of ID to Id to? If it is a functionality change I totallya agree. > On Oct. 29, 2014, 8:01 a.m., Abraham Elmahrek wrote: > > core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java, lines > > 605-609 > > <https://reviews.apache.org/r/27330/diff/1/?file=740283#file740283line605> > > > > These renames do not seem relevant it is releated to submissions, Why is it not relevant? and everwhere it is findSubmissions and it makes sense to be consistent. > On Oct. 29, 2014, 8:01 a.m., Abraham Elmahrek wrote: > > server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java, line > > 303 > > <https://reviews.apache.org/r/27330/diff/1/?file=740289#file740289line303> > > > > The identifier should always be name? In this case, the identifier will > > always be a string. we support both in all places for consistency. I will update the doc if I have missed it > On Oct. 29, 2014, 8:01 a.m., Abraham Elmahrek wrote: > > server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java, lines > > 336-346 > > <https://reviews.apache.org/r/27330/diff/1/?file=740289#file740289line336> > > > > 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? How is this any better? I prfer looking up name since that is what we want to get to, the id remains for use cases we have so far But the docs should encourage using names going fowards both in client and the REST > On Oct. 29, 2014, 8:01 a.m., Abraham Elmahrek wrote: > > shell/src/main/java/org/apache/sqoop/shell/UpdateLinkFunction.java, line 63 > > <https://reviews.apache.org/r/27330/diff/1/?file=740300#file740300line63> > > > > rm? I'd update SQOOP-1634 with a little bit of detail just in case. It is related to adding config as an api, the parent ticket has description. I will add more. the whole point is able to just use a config Id already created and dont have to make a call here during LINK/ JOB creation. less http calls. - Veena ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27330/#review58985 ----------------------------------------------------------- On Oct. 29, 2014, 7:15 a.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27330/ > ----------------------------------------------------------- > > (Updated Oct. 29, 2014, 7:15 a.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 > >
