You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Hari Shreedharan <hs...@cloudera.com> on 2013/04/04 06:29:47 UTC
Review Request: SQOOP-659. Design metadata upgrade procedure
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/
-----------------------------------------------------------
Review request for Sqoop.
Description
-------
Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
This addresses bug SQOOP-659.
https://issues.apache.org/jira/browse/SQOOP-659
Diffs
-----
common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 486635d
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
Diff: https://reviews.apache.org/r/10281/diff/
Testing
-------
Thanks,
Hari Shreedharan
Re: Review Request: SQOOP-659. Design metadata upgrade procedure
Posted by Hari Shreedharan <hs...@cloudera.com>.
> On April 12, 2013, 3:07 p.m., Jarek Cecho wrote:
> > Hi Hari,
> > thank you very much for working on this big JIRA. Your effort is greatly appreciated. It seems that current patch is implementing the work of upgrading the metadata structures in the Derby repository. As we are going to have much more repository implementations in the future, I think that it would be beneficial to abstract this behaviour from the repository and put the code to core module instead. This way we can be sure that the upgrade process will be the same for across all repositories. What do you think about that?
> >
> > Jarcec
Usually I prefer to keep implementations in the implementation class, but I guess this one can be moved up without much of an issue since we simply use other abstract methods from the same class which the child class must implement.
- Hari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/#review19074
-----------------------------------------------------------
On April 4, 2013, 4:29 a.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10281/
> -----------------------------------------------------------
>
> (Updated April 4, 2013, 4:29 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
>
>
> This addresses bug SQOOP-659.
> https://issues.apache.org/jira/browse/SQOOP-659
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
> common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
> core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 486635d
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
> spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
> spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
>
> Diff: https://reviews.apache.org/r/10281/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: SQOOP-659. Design metadata upgrade procedure
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/#review19074
-----------------------------------------------------------
Hi Hari,
thank you very much for working on this big JIRA. Your effort is greatly appreciated. It seems that current patch is implementing the work of upgrading the metadata structures in the Derby repository. As we are going to have much more repository implementations in the future, I think that it would be beneficial to abstract this behaviour from the repository and put the code to core module instead. This way we can be sure that the upgrade process will be the same for across all repositories. What do you think about that?
Jarcec
- Jarek Cecho
On April 4, 2013, 4:29 a.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10281/
> -----------------------------------------------------------
>
> (Updated April 4, 2013, 4:29 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
>
>
> This addresses bug SQOOP-659.
> https://issues.apache.org/jira/browse/SQOOP-659
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
> common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
> core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 486635d
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
> spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
> spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
>
> Diff: https://reviews.apache.org/r/10281/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: SQOOP-659. Design metadata upgrade procedure
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/#review19408
-----------------------------------------------------------
Hi Hari, the updated patch looks great, thank you very much for the refactoring. I do have just few final minor notes:
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorMetadataUpgrader.java
<https://reviews.apache.org/r/10281/#comment40117>
I would recommend transferring only the values. Status messages should not be transferred as we should do our own validation before serialization (not in scope of this jira though).
core/src/main/java/org/apache/sqoop/repository/Repository.java
<https://reviews.apache.org/r/10281/#comment40119>
I think that this guy will be running in different transaction as the main tx is not propagated here.
core/src/main/java/org/apache/sqoop/repository/Repository.java
<https://reviews.apache.org/r/10281/#comment40120>
I think that this guy will be running in different transaction as the main tx is not propagated here.
Jarcec
- Jarek Cecho
On April 18, 2013, 6:15 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10281/
> -----------------------------------------------------------
>
> (Updated April 18, 2013, 6:15 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
>
>
> This addresses bug SQOOP-659.
> https://issues.apache.org/jira/browse/SQOOP-659
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
> common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorMetadataUpgrader.java PRE-CREATION
> core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
> core/src/main/java/org/apache/sqoop/repository/Repository.java d6ec303
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 95f6570
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 32cef8a
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
> spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
> spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
>
> Diff: https://reviews.apache.org/r/10281/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: SQOOP-659. Design metadata upgrade procedure
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/#review19427
-----------------------------------------------------------
Ship it!
Hi Hari, I think that current state is good enough. Let's commit it and improve in follow up jiras. Could you please attach your patch to the jira?
Jarcec
- Jarek Cecho
On April 18, 2013, 10:07 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10281/
> -----------------------------------------------------------
>
> (Updated April 18, 2013, 10:07 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
>
>
> This addresses bug SQOOP-659.
> https://issues.apache.org/jira/browse/SQOOP-659
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
> common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorMetadataUpgrader.java PRE-CREATION
> core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
> core/src/main/java/org/apache/sqoop/repository/Repository.java d6ec303
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 95f6570
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 32cef8a
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
> spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
> spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
>
> Diff: https://reviews.apache.org/r/10281/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: SQOOP-659. Design metadata upgrade procedure
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/
-----------------------------------------------------------
(Updated April 18, 2013, 10:07 p.m.)
Review request for Sqoop.
Changes
-------
Added a missing @Override and inheritDoc javadoc.
Description
-------
Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
This addresses bug SQOOP-659.
https://issues.apache.org/jira/browse/SQOOP-659
Diffs (updated)
-----
common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorMetadataUpgrader.java PRE-CREATION
core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
core/src/main/java/org/apache/sqoop/repository/Repository.java d6ec303
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 95f6570
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 32cef8a
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
Diff: https://reviews.apache.org/r/10281/diff/
Testing
-------
Thanks,
Hari Shreedharan
Re: Review Request: SQOOP-659. Design metadata upgrade procedure
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/
-----------------------------------------------------------
(Updated April 18, 2013, 9:33 p.m.)
Review request for Sqoop.
Description
-------
Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
This addresses bug SQOOP-659.
https://issues.apache.org/jira/browse/SQOOP-659
Diffs (updated)
-----
common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorMetadataUpgrader.java PRE-CREATION
core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
core/src/main/java/org/apache/sqoop/repository/Repository.java d6ec303
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 95f6570
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 32cef8a
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
Diff: https://reviews.apache.org/r/10281/diff/
Testing
-------
Thanks,
Hari Shreedharan
Re: Review Request: SQOOP-659. Design metadata upgrade procedure
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/
-----------------------------------------------------------
(Updated April 18, 2013, 6:15 p.m.)
Review request for Sqoop.
Description
-------
Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
This addresses bug SQOOP-659.
https://issues.apache.org/jira/browse/SQOOP-659
Diffs (updated)
-----
common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorMetadataUpgrader.java PRE-CREATION
core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
core/src/main/java/org/apache/sqoop/repository/Repository.java d6ec303
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 95f6570
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 32cef8a
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
Diff: https://reviews.apache.org/r/10281/diff/
Testing
-------
Thanks,
Hari Shreedharan
Re: Review Request: SQOOP-659. Design metadata upgrade procedure
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/
-----------------------------------------------------------
(Updated April 18, 2013, 6:58 a.m.)
Review request for Sqoop.
Changes
-------
Include feedback from Jarcec. Some changes to the Generic updater. This is sort of a hack and can be updated later - but for now it is probably reasonable. Also added a private method to clone the forms from the connector so that the same forms do not get overwritten.
Description
-------
Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
This addresses bug SQOOP-659.
https://issues.apache.org/jira/browse/SQOOP-659
Diffs (updated)
-----
common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorMetadataUpgrader.java PRE-CREATION
core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
core/src/main/java/org/apache/sqoop/repository/Repository.java d6ec303
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 95f6570
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 32cef8a
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
Diff: https://reviews.apache.org/r/10281/diff/
Testing
-------
Thanks,
Hari Shreedharan
Re: Review Request: SQOOP-659. Design metadata upgrade procedure
Posted by Jarek Cecho <ja...@apache.org>.
> On April 17, 2013, 11:09 p.m., Jarek Cecho wrote:
> > Hi Hari,
> > thank you very much for your effort to get this in. I think that we're almost there!
>
> Hari Shreedharan wrote:
> Thanks for the review, Jarcec. I have posted an updated patch which fixes most of the issues you pointed out.
Thank you! I'll take a look shortly.
> On April 17, 2013, 11:09 p.m., Jarek Cecho wrote:
> > common/src/main/java/org/apache/sqoop/model/MJob.java, lines 121-127
> > <https://reviews.apache.org/r/10281/diff/5/?file=281617#file281617line121>
> >
> > I do not feel comfortable with exposing this interface. Model classes are (will be) part of public API, so I would prefer not to expose ability to switch forms in place and rather create new objects during the upgrade procedure.
>
> Hari Shreedharan wrote:
> If that is the case, we should remove the public constructors and move them to a builder/factory model - that clearly shows that these are not meant to be added/removed. Also we should probably return immutable lists in the getters (else I can change anything I want through that).
Agreed, we can cover those in follow up JIRA.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/#review19320
-----------------------------------------------------------
On April 18, 2013, 6:58 a.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10281/
> -----------------------------------------------------------
>
> (Updated April 18, 2013, 6:58 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
>
>
> This addresses bug SQOOP-659.
> https://issues.apache.org/jira/browse/SQOOP-659
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
> common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorMetadataUpgrader.java PRE-CREATION
> core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
> core/src/main/java/org/apache/sqoop/repository/Repository.java d6ec303
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 95f6570
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 32cef8a
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
> spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
> spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
>
> Diff: https://reviews.apache.org/r/10281/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: SQOOP-659. Design metadata upgrade procedure
Posted by Hari Shreedharan <hs...@cloudera.com>.
> On April 17, 2013, 11:09 p.m., Jarek Cecho wrote:
> > Hi Hari,
> > thank you very much for your effort to get this in. I think that we're almost there!
Thanks for the review, Jarcec. I have posted an updated patch which fixes most of the issues you pointed out.
> On April 17, 2013, 11:09 p.m., Jarek Cecho wrote:
> > common/src/main/java/org/apache/sqoop/model/MJob.java, lines 121-127
> > <https://reviews.apache.org/r/10281/diff/5/?file=281617#file281617line121>
> >
> > I do not feel comfortable with exposing this interface. Model classes are (will be) part of public API, so I would prefer not to expose ability to switch forms in place and rather create new objects during the upgrade procedure.
If that is the case, we should remove the public constructors and move them to a builder/factory model - that clearly shows that these are not meant to be added/removed. Also we should probably return immutable lists in the getters (else I can change anything I want through that).
> On April 17, 2013, 11:09 p.m., Jarek Cecho wrote:
> > repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java, line 634
> > <https://reviews.apache.org/r/10281/diff/5/?file=281625#file281625line634>
> >
> > Nit: The "WHERE" should not be intended on the same level as "ON".
This particular query was removed.
- Hari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/#review19320
-----------------------------------------------------------
On April 18, 2013, 6:58 a.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10281/
> -----------------------------------------------------------
>
> (Updated April 18, 2013, 6:58 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
>
>
> This addresses bug SQOOP-659.
> https://issues.apache.org/jira/browse/SQOOP-659
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
> common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorMetadataUpgrader.java PRE-CREATION
> core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
> core/src/main/java/org/apache/sqoop/repository/Repository.java d6ec303
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 95f6570
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 32cef8a
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
> spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
> spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
>
> Diff: https://reviews.apache.org/r/10281/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: SQOOP-659. Design metadata upgrade procedure
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/#review19320
-----------------------------------------------------------
Hi Hari,
thank you very much for your effort to get this in. I think that we're almost there!
common/src/main/java/org/apache/sqoop/model/MJob.java
<https://reviews.apache.org/r/10281/#comment40043>
I do not feel comfortable with exposing this interface. Model classes are (will be) part of public API, so I would prefer not to expose ability to switch forms in place and rather create new objects during the upgrade procedure.
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorMetadataUpgrader.java
<https://reviews.apache.org/r/10281/#comment39996>
I think that the upgrader should not be responsible for changing the form structures, just for transferring the actual values (e.g the upgrade method should just call "setValue()" on the forms).
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorMetadataUpgrader.java
<https://reviews.apache.org/r/10281/#comment39997>
I think that the upgrader should not be responsible for changing the form structures, just for transferring the actual values (e.g the upgrade method should just call "setValue()" on the forms).
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java
<https://reviews.apache.org/r/10281/#comment40042>
I would recommend to merge those two methods together as it do not make much sense to use them separately.
core/src/main/java/org/apache/sqoop/repository/Repository.java
<https://reviews.apache.org/r/10281/#comment40044>
What about just:
1) Get all connections for connector
2) Get all jobs for connector
3) Wipe out previous forms,inputs,connection inputs, job inputs
4) Update connector in the repository (Connector upgrade most likely will change form and input ids!)
5) for each connection call upgrader to transfer values and save output to the repository
6) for each job call upgrader to transfer values and save output to the repository
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
<https://reviews.apache.org/r/10281/#comment40048>
Nit: Extra indent.
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
<https://reviews.apache.org/r/10281/#comment40047>
Nit: Extra indent.
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
<https://reviews.apache.org/r/10281/#comment40046>
Nit: Extra indent.
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
<https://reviews.apache.org/r/10281/#comment40045>
Nit: The "WHERE" should not be intended on the same level as "ON".
Jarcec
- Jarek Cecho
On April 16, 2013, 8:58 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10281/
> -----------------------------------------------------------
>
> (Updated April 16, 2013, 8:58 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
>
>
> This addresses bug SQOOP-659.
> https://issues.apache.org/jira/browse/SQOOP-659
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
> common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorMetadataUpgrader.java PRE-CREATION
> core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
> core/src/main/java/org/apache/sqoop/repository/Repository.java d6ec303
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 95f6570
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 32cef8a
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
> spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
> spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
>
> Diff: https://reviews.apache.org/r/10281/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: SQOOP-659. Design metadata upgrade procedure
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/
-----------------------------------------------------------
(Updated April 16, 2013, 8:58 p.m.)
Review request for Sqoop.
Description
-------
Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
This addresses bug SQOOP-659.
https://issues.apache.org/jira/browse/SQOOP-659
Diffs (updated)
-----
common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorMetadataUpgrader.java PRE-CREATION
core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
core/src/main/java/org/apache/sqoop/repository/Repository.java d6ec303
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 95f6570
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 32cef8a
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
Diff: https://reviews.apache.org/r/10281/diff/
Testing
-------
Thanks,
Hari Shreedharan
Re: Review Request: SQOOP-659. Design metadata upgrade procedure
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/#review19168
-----------------------------------------------------------
Hi Hari,
thank you very much for working on this big JIRA. Please accept my apologies for so many review rounds. I've dive very deeply into the patch and I have several comments:
core/src/main/java/org/apache/sqoop/repository/Repository.java
<https://reviews.apache.org/r/10281/#comment39752>
I think that this method should be final. As this functionality should be exactly the same across all repository implementations, repositories should not be given chance to override it.
core/src/main/java/org/apache/sqoop/repository/Repository.java
<https://reviews.apache.org/r/10281/#comment39738>
I'm not sure that this will actually work. The updateConnector() will try to remove forms and inputs that are referenced by connection and job which I think will lead to SQLExceptions. Maybe we should consider adding tests to confirm that.
Also it seems that it doesn't make sense to use updateConnector() and udateConnectorForms() methods separately, so what about merging them together?
core/src/main/java/org/apache/sqoop/repository/Repository.java
<https://reviews.apache.org/r/10281/#comment39744>
I think that at this point we should not instantiate the Connector class, we should get the one that is already instantiated in the running server.
core/src/main/java/org/apache/sqoop/repository/Repository.java
<https://reviews.apache.org/r/10281/#comment39739>
I think that we should create here expected form structure from the Connector itself, rather than supplying empty forms, so that Upgrader will be responsible only for transferring actual values. E.g.:
new MConnection(connectorId,
newConnector.getConnectionForms(),
FrameworkManager.getFramework().getConnectionForms()
);
core/src/main/java/org/apache/sqoop/repository/Repository.java
<https://reviews.apache.org/r/10281/#comment39740>
I think that we should create here expected form structure from the Connector itself, rather than supplying empty forms, so that Upgrader will be responsible only for transferring actual values. E.g.:
new MJob(connectorId, oldJob.getType()
newConnector.getJobForms(oldJob.getType()),
FrameworkManager.getFramework().getJobForms(oldJob.getType())
);
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
<https://reviews.apache.org/r/10281/#comment39751>
Nit: Would you mind using same indentation and position of + sign as other queries in the file?
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
<https://reviews.apache.org/r/10281/#comment39750>
Nit: Would you mind using same indentation and position of + sign as other queries in the file?
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
<https://reviews.apache.org/r/10281/#comment39748>
Nit: Would you mind either increase the intend here or put the "=?" on previous line?
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java
<https://reviews.apache.org/r/10281/#comment39746>
Nit: Would you mind to increase the indent here or put the "ON" clause on the same line?
spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java
<https://reviews.apache.org/r/10281/#comment39753>
I do have second thoughs on the Upgrader interface. We are passing entire Connection/Job object to the connector which is actually a security hole as the connector should not even see that. I would suggest to change the interface to pass only the ConnectionForms/JobForms for the connector itself.
Jarcec
- Jarek Cecho
On April 14, 2013, 9:34 a.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10281/
> -----------------------------------------------------------
>
> (Updated April 14, 2013, 9:34 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
>
>
> This addresses bug SQOOP-659.
> https://issues.apache.org/jira/browse/SQOOP-659
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
> common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
> core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
> core/src/main/java/org/apache/sqoop/repository/Repository.java d6ec303
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 95f6570
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 32cef8a
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
> spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
> spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
>
> Diff: https://reviews.apache.org/r/10281/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: SQOOP-659. Design metadata upgrade procedure
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/
-----------------------------------------------------------
(Updated April 14, 2013, 9:34 a.m.)
Review request for Sqoop.
Changes
-------
Keep same connector id during upgrade.
Description
-------
Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
This addresses bug SQOOP-659.
https://issues.apache.org/jira/browse/SQOOP-659
Diffs (updated)
-----
common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
core/src/main/java/org/apache/sqoop/repository/Repository.java d6ec303
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepoError.java 95f6570
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 32cef8a
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
Diff: https://reviews.apache.org/r/10281/diff/
Testing
-------
Thanks,
Hari Shreedharan
Re: Review Request: SQOOP-659. Design metadata upgrade procedure
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/#review19122
-----------------------------------------------------------
Hi Hari,
thank you very much for including all my comments. I do have one final note and I think we're ready to commit the first version of upgrader:
core/src/main/java/org/apache/sqoop/repository/Repository.java
<https://reviews.apache.org/r/10281/#comment39681>
I'm afraid that we can't allow changing the connector/connection/job id during an upgrade as they might be referenced in external systems like oozie.
Jarcec
- Jarek Cecho
On April 12, 2013, 7:25 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10281/
> -----------------------------------------------------------
>
> (Updated April 12, 2013, 7:25 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
>
>
> This addresses bug SQOOP-659.
> https://issues.apache.org/jira/browse/SQOOP-659
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
> common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
> core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
> core/src/main/java/org/apache/sqoop/repository/Repository.java d6ec303
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 486635d
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
> spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
> spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
>
> Diff: https://reviews.apache.org/r/10281/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: SQOOP-659. Design metadata upgrade procedure
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/
-----------------------------------------------------------
(Updated April 12, 2013, 7:25 p.m.)
Review request for Sqoop.
Changes
-------
Move the implementation to Repository level
Description
-------
Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
This addresses bug SQOOP-659.
https://issues.apache.org/jira/browse/SQOOP-659
Diffs (updated)
-----
common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
core/src/main/java/org/apache/sqoop/repository/Repository.java d6ec303
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 486635d
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
Diff: https://reviews.apache.org/r/10281/diff/
Testing
-------
Thanks,
Hari Shreedharan
Re: Review Request: SQOOP-659. Design metadata upgrade procedure
Posted by Hari Shreedharan <hs...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/
-----------------------------------------------------------
(Updated April 12, 2013, 4:54 p.m.)
Review request for Sqoop.
Changes
-------
Incorporate review feedback from Vasanth Kumar and Jarcec.
Description
-------
Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
This addresses bug SQOOP-659.
https://issues.apache.org/jira/browse/SQOOP-659
Diffs (updated)
-----
common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 486635d
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
Diff: https://reviews.apache.org/r/10281/diff/
Testing
-------
Thanks,
Hari Shreedharan
Re: Review Request: SQOOP-659. Design metadata upgrade procedure
Posted by Hari Shreedharan <hs...@cloudera.com>.
> On April 9, 2013, 10 p.m., vasanthkumar wrote:
> > core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java, line 132
> > <https://reviews.apache.org/r/10281/diff/1/?file=277863#file277863line132>
> >
> > Can be
> >
> > mConnector.getVersion().compareTo(result.getVersion()) > 0) ?
Thanks, great catch!
- Hari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/#review18896
-----------------------------------------------------------
On April 12, 2013, 4:54 p.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10281/
> -----------------------------------------------------------
>
> (Updated April 12, 2013, 4:54 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
>
>
> This addresses bug SQOOP-659.
> https://issues.apache.org/jira/browse/SQOOP-659
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
> common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
> core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 486635d
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
> spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
> spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
>
> Diff: https://reviews.apache.org/r/10281/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hari Shreedharan
>
>
Re: Review Request: SQOOP-659. Design metadata upgrade procedure
Posted by rj...@gmail.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10281/#review18896
-----------------------------------------------------------
core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java
<https://reviews.apache.org/r/10281/#comment39363>
Can be
mConnector.getVersion().compareTo(result.getVersion()) > 0) ?
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
<https://reviews.apache.org/r/10281/#comment39364>
How the old forms are handled while upgrade to new forms? Upgraded with empty forms?
- vasanthkumar
On April 4, 2013, 4:29 a.m., Hari Shreedharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10281/
> -----------------------------------------------------------
>
> (Updated April 4, 2013, 4:29 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Metadata upgrade for connector upgrades. This is an initial patch soliciting feedback. Limited testing has been done. I will add some unit tests once I get feedback.
>
>
> This addresses bug SQOOP-659.
> https://issues.apache.org/jira/browse/SQOOP-659
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/sqoop/model/MConnection.java 36dca42
> common/src/main/java/org/apache/sqoop/model/MJob.java a53f04e
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnector.java c315e48
> core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 32df1e5
> core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java ca51313
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java 486635d
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaQuery.java ea458ac
> spi/src/main/java/org/apache/sqoop/connector/spi/MetadataUpgrader.java PRE-CREATION
> spi/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java 540303a
>
> Diff: https://reviews.apache.org/r/10281/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Hari Shreedharan
>
>