-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35216/#review87122
-----------------------------------------------------------
It is a good catch.
Here is my thought. Since the param repository is GLOBAL, could we avoid
passing it to the function, and get it in line the function?
public static long getJobIdFromIdentifier(String identifier) {
Repository repository = RepositoryManager.getInstance().getRepository();
...
}
Further, from my perspective, the repository is abused. How about move all
repository into HandlerUtils class or sqoop-core component? But it seems a huge
refactor. We could split the tasks.
1. move all repository from *RequestHandler into HandlerUtils
2. move all repository from HandlerUtils into ConnectorMangerUtils etc. in
sqoop-core component.
- richard zhou
On June 8, 2015, 3:04 p.m., Dian Fu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35216/
> -----------------------------------------------------------
>
> (Updated June 8, 2015, 3:04 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-2395
> https://issues.apache.org/jira/browse/SQOOP-2395
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> 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.
>
>
> 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/35216/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dian Fu
>
>
