> On Oct. 29, 2014, 3:01 p.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? > > Veena Basavaraj wrote: > 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
Now that I think about, we have a bit of an overlap unfortunately. If the name looks like an ID (e.g. "1"), then we can't look up by ID. It will have to be by name. With that I'd probably keep it the way you have it and prefer name over ID. > On Oct. 29, 2014, 3:01 p.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? > > Veena Basavaraj wrote: > 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. Usually we try to limit the scope of code changes to the Jira for 'git blame'. There has been exceptions with sweeping refactoring, but sqoop2 is reaching a state where we can start doing this again. > On Oct. 29, 2014, 3:01 p.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 > > Veena Basavaraj wrote: > it is releated to submissions, Why is it not relevant? and everwhere it > is findSubmissions and it makes sense to be consistent. Fair! - Abraham ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27330/#review58985 ----------------------------------------------------------- On Oct. 29, 2014, 4:52 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, 4:52 p.m.) > > > Review request for Sqoop. > > > Repository: sqoop-sqoop2 > > > Description > ------- > > see JIRA and its parent JIRA for details > > All the WS stuff, I will do it in the end once the functionality is reviewd. > > > 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 > >
