> On Oct. 20, 2014, 1:11 p.m., Jarek Cecho wrote:
> > spi/src/main/java/org/apache/sqoop/connector/spi/ConnectorConfigurableUpgrader.java,
> > lines 42-66
> > <https://reviews.apache.org/r/26880/diff/7/?file=726213#file726213line42>
> >
> > We should throw an exception in the default case because it means that
> > Sqoop is trying to upgrade something that the connector developper did not
> > anticipated. I don't think that no-oop is correct behaviour here - we
> > should die early.
>
> Veena Basavaraj wrote:
> this is a empty method for reason. There will be cases in connector that
> may not have the from part. see the HbaseToConnector.
>
> or Link config is missing in HDSF
>
> In that case, why would we throw an exception? Not making sense to me.
>
> Jarek Cecho wrote:
> If the connector don't support given entity than it's upgrade method
> should not be even called in my mind. And we should ensure that it's the case
> rather then silently ignoring such case.
>
> Similarly if the connector developper "forgets" to implement upgrade for
> entity that he is using, we should be very direct about that and fail as soon
> as possible, rather then wait to see if for example validations will fail
> (because they don't).
>
> Veena Basavaraj wrote:
> this is not what the current design does.
>
> There is no way for a connector to tell that it supports only
> FromJobConfig. Even if they told, we dont store such info.
>
> The upgrade code in Derby, blindly calls all the methods. Am I missing
> something here? since you seem to allude at things I dont seen in the code.
this is the code in the the abstract class Repository, it will call these
methods for every connector.
public final void upgradeConnector(MConnector oldConnector, MConnector
newConnector) {
LOG.info("Upgrading connector: " + oldConnector.getUniqueName());
long connectorID = oldConnector.getPersistenceId();
newConnector.setPersistenceId(connectorID);
RepositoryTransaction tx = null;
try {
SqoopConnector connector =
ConnectorManager.getInstance().getSqoopConnector(
newConnector.getUniqueName());
Validator connectorConfigValidator = connector.getConfigValidator();
boolean upgradeSuccessful = true;
// 1. Get an upgrader for the connector
ConnectorConfigurableUpgrader upgrader =
connector.getConfigurableUpgrader();
// 2. Get all links associated with the connector.
List<MLink> existingLinksByConnector = findLinksForConnector(connectorID);
// 3. Get all jobs associated with the connector.
List<MJob> existingJobsByConnector = findJobsForConnector(connectorID);
// -- BEGIN TXN --
tx = getTransaction();
tx.begin();
// 4. Delete the inputs for all of the jobs and links (in that order) for
// this connector
deletelinksAndJobs(existingLinksByConnector, existingJobsByConnector, tx);
// 5. Delete all inputs and configs associated with the connector, and
// insert the new configs and inputs for this connector
upgradeConnectorConfigs(newConnector, tx);
// 6. Run upgrade logic for the configs related to the link objects
for (MLink link : existingLinksByConnector) {
// Make a new copy of the configs
List<MConfig> linkConfig =
newConnector.getLinkConfig().clone(false).getConfigs();
MLinkConfig newLinkConfig = new MLinkConfig(linkConfig);
MLinkConfig oldLinkConfig = link.getConnectorLinkConfig();
upgrader.upgradeLinkConfig(oldLinkConfig, newLinkConfig);
MLink newlink = new MLink(link, newLinkConfig);
Object newConfigurationObject = ClassUtils.instantiate(connector
.getLinkConfigurationClass());
ConfigUtils.fromConfigs(newlink.getConnectorLinkConfig().getConfigs(),
newConfigurationObject);
// 7. Run link config validation
ConfigValidator configValidator = connectorConfigValidator
.validateConfigForLink(newConfigurationObject);
if (configValidator.getStatus().canProceed()) {
updateLink(newlink, tx);
} else {
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26880/#review57419
-----------------------------------------------------------
On Oct. 20, 2014, 1:09 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26880/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2014, 1:09 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> See JIRA : https://issues.apache.org/jira/browse/SQOOP-1551
>
> for the details of the ticket and the changes in the RB
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/model/Configurable.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
> common/src/main/java/org/apache/sqoop/model/MJob.java b3dec27
>
> 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/DriverConfigurableUpgrader.java
> PRE-CREATION
> 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/JdbcRepository.java f06fd0c
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
> 5a8e026
> 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/test/java/org/apache/sqoop/repository/derby/TestDriverHandling.java
> 95fbe07
> server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java
> 7109ae5
> server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java
> 462579c
> server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java
> 80e65b8
>
> spi/src/main/java/org/apache/sqoop/configurable/ConfigurableUpgradeUtil.java
> PRE-CREATION
>
> spi/src/main/java/org/apache/sqoop/connector/spi/ConnectorConfigurableUpgrader.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/RepositoryDumpTool.java
> f89c546
> 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
>
>