sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Veena Basavaraj" <vbasava...@cloudera.com>
Subject Re: Review Request 26880: SQOOP-1551:Repository Upgrader api - Extensibility
Date Mon, 20 Oct 2014 00:13:27 GMT


> On Oct. 18, 2014, 9:27 a.m., Jarek Cecho wrote:
> > spi/src/main/java/org/apache/sqoop/connector/spi/ConfigurableUpgrader.java, line
50
> > <https://reviews.apache.org/r/26880/diff/2/?file=724556#file724556line50>
> >
> >     Was exactly my concern :) Can we fille a JIRA for that and create put the JIRA
number there directly?
> 
> Veena Basavaraj wrote:
>     The JIRA is arelady there, it is part of this JIRA, please see the comments in the
JIRA, this is the point where I needed to discsus if we need ConnectorUpgrader and DriverUpgrader
subclasses? rather than interfaces.
> 
> Jarek Cecho wrote:
>     I'm confused, if we want to revisit the API as a part of this JIRA, why the patch
is adding note that it will be done in the future?
>     
>     Anyway I'm wondering what are your thoughts about separate Connector and Driver upgrader?
>     
>     The current code forces the connector developper to detect whether we gave him "From"
or "To" job forms which doesn't seem right. I think that we should add a parameter Direction
that will specify what direction are we upgrading.
> 
> Veena Basavaraj wrote:
>     but direction does not make sense for every config type that we will add
>     
>     it is not in the current patch, since I still dont see any comments in JIRA validating
the proposal I have
>     
>     Please read my comment in JIRA, it says I have part1 and part2 for the same reason,
I can wait for this discussion to proceed and then upload a full patch ( both part 1 and part
2)
>     
>     I kindly request that you think about this carefully.
>     
>     Each configurable behaves differently, for instance Connector and Driver expose configs
but are very different entities, so does it makes sense to have one interface for all objects
that expose configs?
>     
>     second thing that I have noticed, the configurables themselves do not tell what type
of config they persist, we decide in SQOOP that it is of LINK/ JOB, this makes it really hard
to have an api that is self aware for these configurables.
>     
>     Let me know if I am making any sense to you:)

Jarcec, I have updated a new patch, that aligns with having a single extensible api for upgrades.
Let me know what your thoughts are. 

This resolves all the issues that was supposed to addressed by SQOOP -1551


- Veena


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


On Oct. 17, 2014, 11:08 a.m., Veena Basavaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26880/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2014, 11:08 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> #1 - + 1 from Jarek Jarcec Cecho, we shall call it ConfigurableUpgrader
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/sqoop/model/MConfigurable.java PRE-CREATION 
>   common/src/main/java/org/apache/sqoop/model/MConfigurableType.java PRE-CREATION 
>   common/src/main/java/org/apache/sqoop/model/MConnector.java 2f42191 
>   common/src/main/java/org/apache/sqoop/model/MDriver.java 685439e 
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java
87ac2af 
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorUpgrader.java
a069b3e 
>   connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConfigUpgrader.java
b17aa21 
>   connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java
606b9fa 
>   connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnectorUpgrader.java
PRE-CREATION 
>   core/src/main/java/org/apache/sqoop/connector/ConnectorHandler.java 54bdd13 
>   core/src/main/java/org/apache/sqoop/connector/ConnectorManager.java 5226926 
>   core/src/main/java/org/apache/sqoop/driver/Driver.java f1b45bb 
>   core/src/main/java/org/apache/sqoop/driver/DriverConfigUpgrader.java 847b73d 
>   core/src/main/java/org/apache/sqoop/driver/DriverConfigValidator.java 9c3b660 
>   core/src/main/java/org/apache/sqoop/driver/DriverUpgrader.java PRE-CREATION 
>   core/src/main/java/org/apache/sqoop/driver/JobManager.java df2a5ab 
>   core/src/main/java/org/apache/sqoop/driver/configuration/DriverConfiguration.java d4e2254

>   core/src/main/java/org/apache/sqoop/driver/configuration/JobConfiguration.java PRE-CREATION

>   core/src/main/java/org/apache/sqoop/repository/Repository.java 74a9e12 
>   core/src/test/java/org/apache/sqoop/driver/TestDriverConfigUpgrader.java dc4e8c8 
>   core/src/test/java/org/apache/sqoop/driver/TestJobManager.java 3b475c6 
>   core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java 34bd8a5 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
c888910 
>   repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
44ec2e3 
>   server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java 462579c 
>   server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java 80e65b8 
>   shell/src/main/java/org/apache/sqoop/shell/core/Constants.java a1bc5d5 
>   spi/src/main/java/org/apache/sqoop/connector/spi/ConfigurableUpgrader.java PRE-CREATION

>   spi/src/main/java/org/apache/sqoop/connector/spi/RepositoryUpgrader.java 879e428 
>   spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 5315e1f 
>   tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryLoadTool.java 76ebd3b 
> 
> Diff: https://reviews.apache.org/r/26880/diff/
> 
> 
> Testing
> -------
> 
> yes
> 
> 
> Thanks,
> 
> Veena Basavaraj
> 
>


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