> 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
> 
>

Reply via email to