You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Colin Ma <ju...@intel.com> on 2015/08/05 09:38:26 UTC

Review Request 37121: SQOOP-2461: Add sqoop-repository-mysql to support MySql for the metadata repository

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

Review request for Sqoop.


Repository: sqoop-sqoop2


Description
-------

The sqoop-repository-mysql should be implemented with the sqoop-repository-comm.


Diffs
-----

  common-test/src/main/java/org/apache/sqoop/common/test/db/MySQLProvider.java 3083ee6 
  common/src/main/java/org/apache/sqoop/error/code/MySqlRepoError.java PRE-CREATION 
  pom.xml d52c4f6 
  repository/pom.xml c63595c 
  repository/repository-mysql/pom.xml PRE-CREATION 
  repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepoConstants.java PRE-CREATION 
  repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepositoryHandler.java PRE-CREATION 
  repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaCreateQuery.java PRE-CREATION 
  repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaQuery.java PRE-CREATION 
  repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MysqlRepositoryInsertUpdateDeleteSelectQuery.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestCase.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestUtils.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestConnectorHandling.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestDriverHandling.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestHandler.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestJobHandling.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestLinkHandling.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestStructure.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestSubmissionHandling.java PRE-CREATION 
  repository/repository-mysql/src/test/resource/testng.xml PRE-CREATION 
  server/pom.xml aabefc0 

Diff: https://reviews.apache.org/r/37121/diff/


Testing
-------


Thanks,

Colin Ma


Re: Review Request 37121: SQOOP-2461: Add sqoop-repository-mysql to support MySql for the metadata repository

Posted by Colin Ma <ju...@intel.com>.

> On Aug. 8, 2015, 7:04 p.m., Jarek Cecho wrote:
> > Thanks for the patch Colin! I've just preliminary skimmed through the patch and I have few comments:
> > 
> > 1) Please clean up the trailing whitespaces.
> > 
> > 2) I would suggest to use SQL_MODE to solve the problem with escaping object names with double quotes [1]
> > 
> > Links:
> > 1: https://dev.mysql.com/doc/refman/5.0/en/sql-mode.html

Thanks for the comments, the new patch is based on SQL_MODE = ANSI_QUOTES.


> On Aug. 8, 2015, 7:04 p.m., Jarek Cecho wrote:
> > repository/repository-mysql/pom.xml, lines 48-51
> > <https://reviews.apache.org/r/37121/diff/1/?file=1032305#file1032305line48>
> >
> >     I think that we need to mark this as a provided dependency right? Otherwise we will package the JDBC driver which we can't do due to incompatible licenses.

This dependency is for the integration test, and I add <scope>test<scope> in the new patch, thanks for the comment.


> On Aug. 8, 2015, 7:04 p.m., Jarek Cecho wrote:
> > repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MysqlRepositoryInsertUpdateDeleteSelectQuery.java, lines 20-79
> > <https://reviews.apache.org/r/37121/diff/1/?file=1032310#file1032310line20>
> >
> >     Let's do * import here.

In the new patch, 11 constants are imported, so I don't use the * here, and list the import one by one.
If you think * is more suitable in the new patch, I'll update it.


- Colin


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


On Aug. 10, 2015, 8:35 a.m., Colin Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37121/
> -----------------------------------------------------------
> 
> (Updated Aug. 10, 2015, 8:35 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> The sqoop-repository-mysql should be implemented with the sqoop-repository-comm.
> 
> 
> Diffs
> -----
> 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java dd4e546 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/MySQLProvider.java 3083ee6 
>   common/src/main/java/org/apache/sqoop/error/code/MySqlRepoError.java PRE-CREATION 
>   pom.xml 7bcb212 
>   repository/pom.xml c63595c 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepoUtils.java 73293c0 
>   repository/repository-mysql/pom.xml PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepoConstants.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaQuery.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MysqlRepositoryInsertUpdateDeleteSelectQuery.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestCase.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestUtils.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestConnectorHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestDriverHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestHandler.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestJobHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestLinkHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestStructure.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestSubmissionHandling.java PRE-CREATION 
>   server/pom.xml aabefc0 
> 
> Diff: https://reviews.apache.org/r/37121/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colin Ma
> 
>


Re: Review Request 37121: SQOOP-2461: Add sqoop-repository-mysql to support MySql for the metadata repository

Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37121/#review94643
-----------------------------------------------------------


Thanks for the patch Colin! I've just preliminary skimmed through the patch and I have few comments:

1) Please clean up the trailing whitespaces.

2) I would suggest to use SQL_MODE to solve the problem with escaping object names with double quotes [1]

Links:
1: https://dev.mysql.com/doc/refman/5.0/en/sql-mode.html


common-test/src/main/java/org/apache/sqoop/common/test/db/MySQLProvider.java (lines 96 - 107)
<https://reviews.apache.org/r/37121/#comment149230>

    Please create a first class method in DatabaseProvider named "dropDatabase" rather then re-using dropSchema to dropDatabase and implementing this only for MySQL.



common/src/main/java/org/apache/sqoop/error/code/MySqlRepoError.java (line 31)
<https://reviews.apache.org/r/37121/#comment149231>

    Nit: Please end this line with comma "," and pu the semicolon on it's own line. 
    
    This way future addition of error codes is simpler and doesn't require to change to any of the existing lines.



repository/repository-mysql/pom.xml (lines 48 - 51)
<https://reviews.apache.org/r/37121/#comment149232>

    I think that we need to mark this as a provided dependency right? Otherwise we will package the JDBC driver which we can't do due to incompatible licenses.



repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepositoryHandler.java (lines 157 - 177)
<https://reviews.apache.org/r/37121/#comment149233>

    Can't we just do ON DUPLICATE KEY UPDATE?
    
    https://dev.mysql.com/doc/refman/5.0/en/insert-on-duplicate.html



repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MysqlRepositoryInsertUpdateDeleteSelectQuery.java (lines 20 - 79)
<https://reviews.apache.org/r/37121/#comment149234>

    Let's do * import here.



repository/repository-mysql/src/test/resource/testng.xml (lines 19 - 62)
<https://reviews.apache.org/r/37121/#comment149235>

    I don't think that we need to be so heavy with our own suite.xml file right? We have it only for the integration tests in test module to avoid the cost of starting all the providiers on every class. I don't see the same benefits here, so let's perhaps use the default suite runner?


Jarcec

- Jarek Cecho


On Aug. 5, 2015, 7:38 a.m., Colin Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37121/
> -----------------------------------------------------------
> 
> (Updated Aug. 5, 2015, 7:38 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> The sqoop-repository-mysql should be implemented with the sqoop-repository-comm.
> 
> 
> Diffs
> -----
> 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/MySQLProvider.java 3083ee6 
>   common/src/main/java/org/apache/sqoop/error/code/MySqlRepoError.java PRE-CREATION 
>   pom.xml d52c4f6 
>   repository/pom.xml c63595c 
>   repository/repository-mysql/pom.xml PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepoConstants.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaQuery.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MysqlRepositoryInsertUpdateDeleteSelectQuery.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestCase.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestUtils.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestConnectorHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestDriverHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestHandler.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestJobHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestLinkHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestStructure.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestSubmissionHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/resource/testng.xml PRE-CREATION 
>   server/pom.xml aabefc0 
> 
> Diff: https://reviews.apache.org/r/37121/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colin Ma
> 
>


Re: Review Request 37121: SQOOP-2461: Add sqoop-repository-mysql to support MySql for the metadata repository

Posted by Colin Ma <ju...@intel.com>.

> On Aug. 13, 2015, 8:41 p.m., Abraham Elmahrek wrote:
> > repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepositoryHandler.java, line 84
> > <https://reviews.apache.org/r/37121/diff/2/?file=1035984#file1035984line84>
> >
> >     Let's move this to CommonRepoHandler?

For this method, first release with Deby went out without system table, so there has some addition code for DerbyRepositoryHandler. If possible, I'll create new JIRA to refactor this.


> On Aug. 13, 2015, 8:41 p.m., Abraham Elmahrek wrote:
> > repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepositoryHandler.java, line 181
> > <https://reviews.apache.org/r/37121/diff/2/?file=1035984#file1035984line181>
> >
> >     I think we do this in all 3 of our repository handlers. Let's pull this into the CommonRepositoryHandler in a separate Jira?

You're right, this method should be put in CommonRepositoryHandler, I'll create new JIRA to trace this.


> On Aug. 13, 2015, 8:41 p.m., Abraham Elmahrek wrote:
> > repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaCreateQuery.java, lines 31-39
> > <https://reviews.apache.org/r/37121/diff/2/?file=1035985#file1035985line31>
> >
> >     This info isn't necessary any more given we have it in the repository docs.
> >     
> >     Maybe we need a separate Jira to remove from the other places as well?

I'll remove the information for this class.
For the other places, I'll create a separate JIRA to remove them.


- Colin


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


On Aug. 10, 2015, 8:35 a.m., Colin Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37121/
> -----------------------------------------------------------
> 
> (Updated Aug. 10, 2015, 8:35 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> The sqoop-repository-mysql should be implemented with the sqoop-repository-comm.
> 
> 
> Diffs
> -----
> 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java dd4e546 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/MySQLProvider.java 3083ee6 
>   common/src/main/java/org/apache/sqoop/error/code/MySqlRepoError.java PRE-CREATION 
>   pom.xml 7bcb212 
>   repository/pom.xml c63595c 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepoUtils.java 73293c0 
>   repository/repository-mysql/pom.xml PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepoConstants.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaQuery.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MysqlRepositoryInsertUpdateDeleteSelectQuery.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestCase.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestUtils.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestConnectorHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestDriverHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestHandler.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestJobHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestLinkHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestStructure.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestSubmissionHandling.java PRE-CREATION 
>   server/pom.xml aabefc0 
> 
> Diff: https://reviews.apache.org/r/37121/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colin Ma
> 
>


Re: Review Request 37121: SQOOP-2461: Add sqoop-repository-mysql to support MySql for the metadata repository

Posted by Abraham Elmahrek <ab...@cloudera.com>.

> On Aug. 13, 2015, 8:41 p.m., Abraham Elmahrek wrote:
> > repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepositoryHandler.java, line 84
> > <https://reviews.apache.org/r/37121/diff/2/?file=1035984#file1035984line84>
> >
> >     Let's move this to CommonRepoHandler?
> 
> Colin Ma wrote:
>     For this method, first release with Deby went out without system table, so there has some addition code for DerbyRepositoryHandler. If possible, I'll create new JIRA to refactor this.

Sounds good!


> On Aug. 13, 2015, 8:41 p.m., Abraham Elmahrek wrote:
> > repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepositoryHandler.java, line 181
> > <https://reviews.apache.org/r/37121/diff/2/?file=1035984#file1035984line181>
> >
> >     I think we do this in all 3 of our repository handlers. Let's pull this into the CommonRepositoryHandler in a separate Jira?
> 
> Colin Ma wrote:
>     You're right, this method should be put in CommonRepositoryHandler, I'll create new JIRA to trace this.

Sounds good!


> On Aug. 13, 2015, 8:41 p.m., Abraham Elmahrek wrote:
> > repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaCreateQuery.java, lines 31-39
> > <https://reviews.apache.org/r/37121/diff/2/?file=1035985#file1035985line31>
> >
> >     This info isn't necessary any more given we have it in the repository docs.
> >     
> >     Maybe we need a separate Jira to remove from the other places as well?
> 
> Colin Ma wrote:
>     I'll remove the information for this class.
>     For the other places, I'll create a separate JIRA to remove them.

Sounds good!


- Abraham


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


On Aug. 14, 2015, 6:35 a.m., Colin Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37121/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2015, 6:35 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> The sqoop-repository-mysql should be implemented with the sqoop-repository-comm.
> 
> 
> Diffs
> -----
> 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java dd4e546 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/MySQLProvider.java 3083ee6 
>   common/src/main/java/org/apache/sqoop/error/code/MySqlRepoError.java PRE-CREATION 
>   pom.xml 7bcb212 
>   repository/pom.xml c63595c 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepoUtils.java 73293c0 
>   repository/repository-mysql/pom.xml PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepoConstants.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaQuery.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MysqlRepositoryInsertUpdateDeleteSelectQuery.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestCase.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestUtils.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestConnectorHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestDriverHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestHandler.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestJobHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestLinkHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestStructure.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestSubmissionHandling.java PRE-CREATION 
>   server/pom.xml aabefc0 
> 
> Diff: https://reviews.apache.org/r/37121/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colin Ma
> 
>


Re: Review Request 37121: SQOOP-2461: Add sqoop-repository-mysql to support MySql for the metadata repository

Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37121/#review95324
-----------------------------------------------------------



repository/repository-mysql/pom.xml (line 51)
<https://reviews.apache.org/r/37121/#comment150230>

    I don't think the version is necessary.



repository/repository-mysql/pom.xml (lines 60 - 64)
<https://reviews.apache.org/r/37121/#comment150233>

    The Java MySQL JDBC connector has a GPL license. As long as we don't package/distribute it, I think we're OK. I don't think we're currently distributing test jars.



repository/repository-mysql/pom.xml (lines 73 - 75)
<https://reviews.apache.org/r/37121/#comment150234>

    I don't think this part is necessary.



repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepositoryHandler.java (line 84)
<https://reviews.apache.org/r/37121/#comment150229>

    Let's move this to CommonRepoHandler?



repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepositoryHandler.java (line 181)
<https://reviews.apache.org/r/37121/#comment150227>

    I think we do this in all 3 of our repository handlers. Let's pull this into the CommonRepositoryHandler in a separate Jira?



repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaCreateQuery.java (lines 31 - 39)
<https://reviews.apache.org/r/37121/#comment150228>

    This info isn't necessary any more given we have it in the repository docs.
    
    Maybe we need a separate Jira to remove from the other places as well?


- Abraham Elmahrek


On Aug. 10, 2015, 8:35 a.m., Colin Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37121/
> -----------------------------------------------------------
> 
> (Updated Aug. 10, 2015, 8:35 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> The sqoop-repository-mysql should be implemented with the sqoop-repository-comm.
> 
> 
> Diffs
> -----
> 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java dd4e546 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/MySQLProvider.java 3083ee6 
>   common/src/main/java/org/apache/sqoop/error/code/MySqlRepoError.java PRE-CREATION 
>   pom.xml 7bcb212 
>   repository/pom.xml c63595c 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepoUtils.java 73293c0 
>   repository/repository-mysql/pom.xml PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepoConstants.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaQuery.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MysqlRepositoryInsertUpdateDeleteSelectQuery.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestCase.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestUtils.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestConnectorHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestDriverHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestHandler.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestJobHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestLinkHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestStructure.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestSubmissionHandling.java PRE-CREATION 
>   server/pom.xml aabefc0 
> 
> Diff: https://reviews.apache.org/r/37121/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colin Ma
> 
>


Re: Review Request 37121: SQOOP-2461: Add sqoop-repository-mysql to support MySql for the metadata repository

Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37121/#review95890
-----------------------------------------------------------

Ship it!


Ship It!

- Abraham Elmahrek


On Aug. 14, 2015, 6:35 a.m., Colin Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37121/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2015, 6:35 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> The sqoop-repository-mysql should be implemented with the sqoop-repository-comm.
> 
> 
> Diffs
> -----
> 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java dd4e546 
>   common-test/src/main/java/org/apache/sqoop/common/test/db/MySQLProvider.java 3083ee6 
>   common/src/main/java/org/apache/sqoop/error/code/MySqlRepoError.java PRE-CREATION 
>   pom.xml 7bcb212 
>   repository/pom.xml c63595c 
>   repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepoUtils.java 73293c0 
>   repository/repository-mysql/pom.xml PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepoConstants.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepositoryHandler.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaCreateQuery.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaQuery.java PRE-CREATION 
>   repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MysqlRepositoryInsertUpdateDeleteSelectQuery.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestCase.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestUtils.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestConnectorHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestDriverHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestHandler.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestJobHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestLinkHandling.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestStructure.java PRE-CREATION 
>   repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestSubmissionHandling.java PRE-CREATION 
>   server/pom.xml aabefc0 
> 
> Diff: https://reviews.apache.org/r/37121/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colin Ma
> 
>


Re: Review Request 37121: SQOOP-2461: Add sqoop-repository-mysql to support MySql for the metadata repository

Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37121/
-----------------------------------------------------------

(Updated Aug. 14, 2015, 6:35 a.m.)


Review request for Sqoop.


Repository: sqoop-sqoop2


Description
-------

The sqoop-repository-mysql should be implemented with the sqoop-repository-comm.


Diffs (updated)
-----

  common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java dd4e546 
  common-test/src/main/java/org/apache/sqoop/common/test/db/MySQLProvider.java 3083ee6 
  common/src/main/java/org/apache/sqoop/error/code/MySqlRepoError.java PRE-CREATION 
  pom.xml 7bcb212 
  repository/pom.xml c63595c 
  repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepoUtils.java 73293c0 
  repository/repository-mysql/pom.xml PRE-CREATION 
  repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepoConstants.java PRE-CREATION 
  repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepositoryHandler.java PRE-CREATION 
  repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaCreateQuery.java PRE-CREATION 
  repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaQuery.java PRE-CREATION 
  repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MysqlRepositoryInsertUpdateDeleteSelectQuery.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestCase.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestUtils.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestConnectorHandling.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestDriverHandling.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestHandler.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestJobHandling.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestLinkHandling.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestStructure.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestSubmissionHandling.java PRE-CREATION 
  server/pom.xml aabefc0 

Diff: https://reviews.apache.org/r/37121/diff/


Testing
-------


Thanks,

Colin Ma


Re: Review Request 37121: SQOOP-2461: Add sqoop-repository-mysql to support MySql for the metadata repository

Posted by Colin Ma <ju...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37121/
-----------------------------------------------------------

(Updated Aug. 10, 2015, 8:35 a.m.)


Review request for Sqoop.


Repository: sqoop-sqoop2


Description
-------

The sqoop-repository-mysql should be implemented with the sqoop-repository-comm.


Diffs (updated)
-----

  common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java dd4e546 
  common-test/src/main/java/org/apache/sqoop/common/test/db/MySQLProvider.java 3083ee6 
  common/src/main/java/org/apache/sqoop/error/code/MySqlRepoError.java PRE-CREATION 
  pom.xml 7bcb212 
  repository/pom.xml c63595c 
  repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepoUtils.java 73293c0 
  repository/repository-mysql/pom.xml PRE-CREATION 
  repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepoConstants.java PRE-CREATION 
  repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepositoryHandler.java PRE-CREATION 
  repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaCreateQuery.java PRE-CREATION 
  repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaQuery.java PRE-CREATION 
  repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MysqlRepositoryInsertUpdateDeleteSelectQuery.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestCase.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestUtils.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestConnectorHandling.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestDriverHandling.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestHandler.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestJobHandling.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestLinkHandling.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestStructure.java PRE-CREATION 
  repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestSubmissionHandling.java PRE-CREATION 
  server/pom.xml aabefc0 

Diff: https://reviews.apache.org/r/37121/diff/


Testing
-------


Thanks,

Colin Ma