> 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
> 
> Abraham Elmahrek wrote:
>     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.

I'm concerned about the overlap as well. What about one of the following:

1) Let's have two REST interfaces for retriving objects one for "id" another 
for "name".
2) Let's define name rules so that name can never ever start with number. E.g 
creating link with name "1" will fail on invalid name or something like that.

I think that both will solve our overlap problem.


- Jarek


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27330/#review58985
-----------------------------------------------------------


On Oct. 30, 2014, 9:08 p.m., Veena Basavaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27330/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2014, 9:08 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.
> 
> 
> If someone is wondering why START is changed to submit: there are tons of 
> places in the code and in the java docs, we actually mean submit when we say 
> START
> 
>   DRIVER_0008("Invalid combination of submission and execution engines"),
> 
>   DRIVER_0009("Job has been disabled. Cannot submit this job."),
> 
>   DRIVER_0010("Link for this job has been disabled. Cannot submit this job."),
> 
>   DRIVER_0011("Connector does not support specified direction. Cannot submit 
> this job."),
> 
>   ;
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/sqoop/client/SqoopClient.java 33a0c3c 
>   client/src/main/java/org/apache/sqoop/client/SubmissionCallback.java 
> de7211a 
>   
> 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 
>   
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcPartitioner.java
>  2411169 
>   
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestPartitioner.java
>  3ae64f0 
>   core/src/main/java/org/apache/sqoop/driver/JobManager.java f6447c6 
>   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
>  b996a0b 
>   
> 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/ConnectorRequestHandler.java 
> 5694ea5 
>   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/AbortCommand.java PRE-CREATION 
>   shell/src/main/java/org/apache/sqoop/shell/ShowJobStatusFunction.java 
> PRE-CREATION 
>   shell/src/main/java/org/apache/sqoop/shell/SqoopShell.java 2e87965 
>   shell/src/main/java/org/apache/sqoop/shell/StartCommand.java 7c56980 
>   shell/src/main/java/org/apache/sqoop/shell/StartJobFunction.java 4363f05 
>   shell/src/main/java/org/apache/sqoop/shell/StatusCommand.java 3447a87 
>   shell/src/main/java/org/apache/sqoop/shell/StatusJobFunction.java fb83af3 
>   shell/src/main/java/org/apache/sqoop/shell/StopCommand.java 50b2e81 
>   shell/src/main/java/org/apache/sqoop/shell/StopJobFunction.java 790c522 
>   shell/src/main/java/org/apache/sqoop/shell/SubmitCommand.java PRE-CREATION 
>   shell/src/main/java/org/apache/sqoop/shell/SubmitJobFunction.java 
> PRE-CREATION 
>   shell/src/main/java/org/apache/sqoop/shell/UpdateJobFunction.java dd075d7 
>   shell/src/main/java/org/apache/sqoop/shell/UpdateLinkFunction.java 176833a 
>   shell/src/main/java/org/apache/sqoop/shell/core/Constants.java 44d5920 
>   shell/src/main/resources/shell-resource.properties 0e63c50 
>   test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java 
> af31769 
>   
> test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/FromRDBMSToHDFSTest.java
>  36f7443 
>   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
> 
>

Reply via email to