> On Sept. 18, 2015, 8:53 p.m., Jarek Cecho wrote: > > core/src/main/java/org/apache/sqoop/driver/JobManager.java, line 311 > > <https://reviews.apache.org/r/38480/diff/1/?file=1076474#file1076474line311> > > > > It seems that we're doing a lot of changes to eventually call a method > > getJob(long) that will go and call > > RepositoryManager.getInstance().getRepository().findJob(long); > > > > Would it make more sense to simply convert this method to > > getJob(String) and use findJob(String) from Repository?
I removed most of the places which use method getJob(long) but I found that we still have to keep this method as for thread JobManager#UpdateThread, it has the following code path: UpdateThread -> updateSubmission(MSubmission) -> invokeDestroyerOnJobSuccess(MSubmission) -> getJob(long) and during this code path, we don't know job name and so have to retrieve job with jobId. Any thoughts? - Dian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38480/#review99598 ----------------------------------------------------------- On Sept. 22, 2015, 2:27 a.m., Dian Fu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38480/ > ----------------------------------------------------------- > > (Updated Sept. 22, 2015, 2:27 a.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-2581 > https://issues.apache.org/jira/browse/SQOOP-2581 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > This JIRA is a follow up of SQOOP-2573. > > > Diffs > ----- > > common/src/main/java/org/apache/sqoop/error/code/DriverError.java 0ec9310 > common/src/main/java/org/apache/sqoop/utils/UrlSafeUtils.java 75defd7 > core/src/main/java/org/apache/sqoop/driver/JobManager.java 923df0d > core/src/test/java/org/apache/sqoop/driver/TestJobManager.java 414d3e9 > server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java > f65cb81 > > Diff: https://reviews.apache.org/r/38480/diff/ > > > Testing > ------- > > > Thanks, > > Dian Fu > >
