sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Abraham Elmahrek" <...@cloudera.com>
Subject Re: Review Request 34930: SQOOP-2386: Sqoop2: refactor code in HandlerUtils
Date Mon, 08 Jun 2015 12:46:45 GMT


> On June 3, 2015, 7:43 a.m., Abraham Elmahrek wrote:
> > server/src/main/java/org/apache/sqoop/handler/HandlerUtils.java, line 71
> > <https://reviews.apache.org/r/34930/diff/1/?file=976396#file976396line71>
> >
> >     What's the difference between using `Repository` versus `ConnectorManager`?
> 
> Dian Fu wrote:
>     ConnectorManager will call Repository to complete its work, so the function is identical.
In HandlerUtils.java, there are three methods and the other two methods both take "String
identifier, Repository repository" as the signature. Making method getConnectorIdFromIdentifier
also taking "String identifier, Repository repository" as the signature will makes the code
more consistent.

If it's just clean up, let's make this change in a separate Jira?


- Abraham


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


On June 2, 2015, 8:35 a.m., Dian Fu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34930/
> -----------------------------------------------------------
> 
> (Updated June 2, 2015, 8:35 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> In HandlerUtils, there is code like this:
>  if (repository.findJob(identifier) != null) {
>       jobId = repository.findJob(identifier).getPersistenceId();
>       ...
>  }
> This will execute repository.findJob twice, this is unnecessary.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java d1537bf 
>   server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java 5128a27

>   server/src/main/java/org/apache/sqoop/handler/HandlerUtils.java 93ff60b 
>   server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java c96d66d 
> 
> Diff: https://reviews.apache.org/r/34930/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dian Fu
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message