You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Veena Basavaraj <vb...@cloudera.com> on 2014/10/22 01:56:28 UTC
Review Request 27004: SQOOP-1554:Add NullConfigurationClass to
support use cases that do not have a particular type of config
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27004/
-----------------------------------------------------------
Review request for Sqoop.
Repository: sqoop-sqoop2
Description
-------
yes tests pass.
Diffs
-----
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java e63e464
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java 74b1cb8
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java 2c8b6c8
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsInitializer.java bb5e353
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java 660418d
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java f40459f
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java 5d48a29
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfiguration.java c0cd336
connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java c6d2f90
connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java 552a751
connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestPartitioner.java 9d177ec
connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyConfig.java PRE-CREATION
connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyJobConfiguration.java PRE-CREATION
connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyLinkConfiguration.java PRE-CREATION
execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 6d0dcb4
execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 665a65b
spi/src/main/java/org/apache/sqoop/job/etl/Extractor.java d6c186d
spi/src/main/java/org/apache/sqoop/job/etl/Initializer.java 5c48fc3
spi/src/main/java/org/apache/sqoop/job/etl/Partitioner.java 57507df
Diff: https://reviews.apache.org/r/27004/diff/
Testing
-------
Thanks,
Veena Basavaraj
Re: Review Request 27004: SQOOP-1554:Add NullConfigurationClass to
support use cases that do not have a particular type of config
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27004/#review57964
-----------------------------------------------------------
Ship it!
Ship It!
- Jarek Cecho
On Oct. 22, 2014, 8:46 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27004/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2014, 8:46 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see JIRA for details.
>
> also it fixes a lot of warnings in the code base.
>
>
> Diffs
> -----
>
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java e63e464
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java 74b1cb8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java 2c8b6c8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsFromDestroyer.java PRE-CREATION
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsFromInitializer.java PRE-CREATION
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsInitializer.java bb5e353
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java 660418d
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java f40459f
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsToDestroyer.java PRE-CREATION
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsToInitializer.java PRE-CREATION
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java 5d48a29
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfiguration.java c0cd336
> connector/connector-hdfs/src/main/resources/hdfs-connector-config.properties 9b8c6ba
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java c6d2f90
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java 552a751
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestPartitioner.java 9d177ec
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyConfiguration.java PRE-CREATION
> core/src/main/java/org/apache/sqoop/driver/JobManager.java 51e562c
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 6d0dcb4
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 665a65b
> spi/src/main/java/org/apache/sqoop/job/etl/Destroyer.java a133106
> spi/src/main/java/org/apache/sqoop/job/etl/Extractor.java d6c186d
> spi/src/main/java/org/apache/sqoop/job/etl/Initializer.java 5c48fc3
> spi/src/main/java/org/apache/sqoop/job/etl/Loader.java cc32ada
> spi/src/main/java/org/apache/sqoop/job/etl/Partitioner.java 57507df
>
> Diff: https://reviews.apache.org/r/27004/diff/
>
>
> Testing
> -------
>
> yes tests pass.
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 27004: SQOOP-1554:Add NullConfigurationClass to
support use cases that do not have a particular type of config
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Oct. 23, 2014, 9:32 p.m., Qian Xu wrote:
> > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java, line 80
> > <https://reviews.apache.org/r/27004/diff/4/?file=728986#file728986line80>
> >
> > The patch contains two things: (1) add To/From into class names (2) use `EmptyConfiguration` instead of an empty `LinkConfiguration`.
> >
> > The code itself looks good. The only concern is that if later HDFS has some linked configuration, we have to change a lot of class references including unittests.
> >
> > How about create `LinkConfiguration` as an alias of `EmptyConfiguration`?
not sure I understand alias.
but if the requirements change then of curs code will change and its test:)
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27004/#review58240
-----------------------------------------------------------
On Oct. 22, 2014, 1:46 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27004/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2014, 1:46 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see JIRA for details.
>
> also it fixes a lot of warnings in the code base.
>
>
> Diffs
> -----
>
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java e63e464
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java 74b1cb8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java 2c8b6c8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsFromDestroyer.java PRE-CREATION
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsFromInitializer.java PRE-CREATION
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsInitializer.java bb5e353
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java 660418d
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java f40459f
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsToDestroyer.java PRE-CREATION
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsToInitializer.java PRE-CREATION
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java 5d48a29
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfiguration.java c0cd336
> connector/connector-hdfs/src/main/resources/hdfs-connector-config.properties 9b8c6ba
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java c6d2f90
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java 552a751
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestPartitioner.java 9d177ec
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyConfiguration.java PRE-CREATION
> core/src/main/java/org/apache/sqoop/driver/JobManager.java 51e562c
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 6d0dcb4
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 665a65b
> spi/src/main/java/org/apache/sqoop/job/etl/Destroyer.java a133106
> spi/src/main/java/org/apache/sqoop/job/etl/Extractor.java d6c186d
> spi/src/main/java/org/apache/sqoop/job/etl/Initializer.java 5c48fc3
> spi/src/main/java/org/apache/sqoop/job/etl/Loader.java cc32ada
> spi/src/main/java/org/apache/sqoop/job/etl/Partitioner.java 57507df
>
> Diff: https://reviews.apache.org/r/27004/diff/
>
>
> Testing
> -------
>
> yes tests pass.
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 27004: SQOOP-1554:Add NullConfigurationClass to
support use cases that do not have a particular type of config
Posted by Qian Xu <sx...@googlemail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27004/#review58240
-----------------------------------------------------------
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java
<https://reviews.apache.org/r/27004/#comment99192>
The patch contains two things: (1) add To/From into class names (2) use `EmptyConfiguration` instead of an empty `LinkConfiguration`.
The code itself looks good. The only concern is that if later HDFS has some linked configuration, we have to change a lot of class references including unittests.
How about create `LinkConfiguration` as an alias of `EmptyConfiguration`?
- Qian Xu
On Oct. 23, 2014, 4:46 a.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27004/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2014, 4:46 a.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see JIRA for details.
>
> also it fixes a lot of warnings in the code base.
>
>
> Diffs
> -----
>
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java e63e464
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java 74b1cb8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java 2c8b6c8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsFromDestroyer.java PRE-CREATION
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsFromInitializer.java PRE-CREATION
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsInitializer.java bb5e353
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java 660418d
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java f40459f
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsToDestroyer.java PRE-CREATION
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsToInitializer.java PRE-CREATION
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java 5d48a29
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfiguration.java c0cd336
> connector/connector-hdfs/src/main/resources/hdfs-connector-config.properties 9b8c6ba
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java c6d2f90
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java 552a751
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestPartitioner.java 9d177ec
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyConfiguration.java PRE-CREATION
> core/src/main/java/org/apache/sqoop/driver/JobManager.java 51e562c
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 6d0dcb4
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 665a65b
> spi/src/main/java/org/apache/sqoop/job/etl/Destroyer.java a133106
> spi/src/main/java/org/apache/sqoop/job/etl/Extractor.java d6c186d
> spi/src/main/java/org/apache/sqoop/job/etl/Initializer.java 5c48fc3
> spi/src/main/java/org/apache/sqoop/job/etl/Loader.java cc32ada
> spi/src/main/java/org/apache/sqoop/job/etl/Partitioner.java 57507df
>
> Diff: https://reviews.apache.org/r/27004/diff/
>
>
> Testing
> -------
>
> yes tests pass.
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 27004: SQOOP-1554:Add NullConfigurationClass to
support use cases that do not have a particular type of config
Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27004/
-----------------------------------------------------------
(Updated Oct. 22, 2014, 1:46 p.m.)
Review request for Sqoop.
Changes
-------
fix ws
Repository: sqoop-sqoop2
Description
-------
see JIRA for details.
also it fixes a lot of warnings in the code base.
Diffs (updated)
-----
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java e63e464
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java 74b1cb8
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java 2c8b6c8
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsFromDestroyer.java PRE-CREATION
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsFromInitializer.java PRE-CREATION
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsInitializer.java bb5e353
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java 660418d
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java f40459f
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsToDestroyer.java PRE-CREATION
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsToInitializer.java PRE-CREATION
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java 5d48a29
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfiguration.java c0cd336
connector/connector-hdfs/src/main/resources/hdfs-connector-config.properties 9b8c6ba
connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java c6d2f90
connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java 552a751
connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestPartitioner.java 9d177ec
connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyConfiguration.java PRE-CREATION
core/src/main/java/org/apache/sqoop/driver/JobManager.java 51e562c
execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 6d0dcb4
execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 665a65b
spi/src/main/java/org/apache/sqoop/job/etl/Destroyer.java a133106
spi/src/main/java/org/apache/sqoop/job/etl/Extractor.java d6c186d
spi/src/main/java/org/apache/sqoop/job/etl/Initializer.java 5c48fc3
spi/src/main/java/org/apache/sqoop/job/etl/Loader.java cc32ada
spi/src/main/java/org/apache/sqoop/job/etl/Partitioner.java 57507df
Diff: https://reviews.apache.org/r/27004/diff/
Testing
-------
yes tests pass.
Thanks,
Veena Basavaraj
Re: Review Request 27004: SQOOP-1554:Add NullConfigurationClass to
support use cases that do not have a particular type of config
Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27004/
-----------------------------------------------------------
(Updated Oct. 22, 2014, 1:18 p.m.)
Review request for Sqoop.
Changes
-------
integration tests pass as well.!
added some java docs to explain the configurations used in the 5 steps of the job lifecyle.
Repository: sqoop-sqoop2
Description
-------
see JIRA for details.
also it fixes a lot of warnings in the code base.
Diffs (updated)
-----
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java e63e464
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java 74b1cb8
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java 2c8b6c8
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsFromDestroyer.java PRE-CREATION
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsFromInitializer.java PRE-CREATION
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsInitializer.java bb5e353
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java 660418d
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java f40459f
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsToDestroyer.java PRE-CREATION
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsToInitializer.java PRE-CREATION
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java 5d48a29
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfiguration.java c0cd336
connector/connector-hdfs/src/main/resources/hdfs-connector-config.properties 9b8c6ba
connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java c6d2f90
connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java 552a751
connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestPartitioner.java 9d177ec
connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyConfiguration.java PRE-CREATION
core/src/main/java/org/apache/sqoop/driver/JobManager.java 51e562c
execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 6d0dcb4
execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 665a65b
spi/src/main/java/org/apache/sqoop/job/etl/Destroyer.java a133106
spi/src/main/java/org/apache/sqoop/job/etl/Extractor.java d6c186d
spi/src/main/java/org/apache/sqoop/job/etl/Initializer.java 5c48fc3
spi/src/main/java/org/apache/sqoop/job/etl/Loader.java cc32ada
spi/src/main/java/org/apache/sqoop/job/etl/Partitioner.java 57507df
Diff: https://reviews.apache.org/r/27004/diff/
Testing
-------
yes tests pass.
Thanks,
Veena Basavaraj
Re: Review Request 27004: SQOOP-1554:Add NullConfigurationClass to
support use cases that do not have a particular type of config
Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27004/#review57800
-----------------------------------------------------------
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
<https://reviews.apache.org/r/27004/#comment98673>
removed this in the lastes patch, had used of some other testing
spi/src/main/java/org/apache/sqoop/job/etl/Initializer.java
<https://reviews.apache.org/r/27004/#comment98672>
NOTE: these had tabs than spaces hence the formatting.
- Veena Basavaraj
On Oct. 21, 2014, 10:23 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27004/
> -----------------------------------------------------------
>
> (Updated Oct. 21, 2014, 10:23 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see JIRA for details.
>
> also it fixes a lot of warnings in the code base.
>
>
> Diffs
> -----
>
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java e63e464
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java 74b1cb8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java 2c8b6c8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsInitializer.java bb5e353
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java 660418d
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java f40459f
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java 5d48a29
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfiguration.java c0cd336
> connector/connector-hdfs/src/main/resources/hdfs-connector-config.properties 9b8c6ba
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java c6d2f90
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java 552a751
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestPartitioner.java 9d177ec
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyConfiguration.java PRE-CREATION
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 6d0dcb4
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 665a65b
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java aa58850
> spi/src/main/java/org/apache/sqoop/job/etl/Extractor.java d6c186d
> spi/src/main/java/org/apache/sqoop/job/etl/Initializer.java 5c48fc3
> spi/src/main/java/org/apache/sqoop/job/etl/Partitioner.java 57507df
>
> Diff: https://reviews.apache.org/r/27004/diff/
>
>
> Testing
> -------
>
> yes tests pass.
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 27004: SQOOP-1554:Add NullConfigurationClass to
support use cases that do not have a particular type of config
Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27004/
-----------------------------------------------------------
(Updated Oct. 21, 2014, 10:23 p.m.)
Review request for Sqoop.
Repository: sqoop-sqoop2
Description
-------
see JIRA for details.
also it fixes a lot of warnings in the code base.
Diffs (updated)
-----
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java e63e464
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java 74b1cb8
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java 2c8b6c8
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsInitializer.java bb5e353
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java 660418d
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java f40459f
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java 5d48a29
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfiguration.java c0cd336
connector/connector-hdfs/src/main/resources/hdfs-connector-config.properties 9b8c6ba
connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java c6d2f90
connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java 552a751
connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestPartitioner.java 9d177ec
connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyConfiguration.java PRE-CREATION
execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 6d0dcb4
execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 665a65b
repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java aa58850
spi/src/main/java/org/apache/sqoop/job/etl/Extractor.java d6c186d
spi/src/main/java/org/apache/sqoop/job/etl/Initializer.java 5c48fc3
spi/src/main/java/org/apache/sqoop/job/etl/Partitioner.java 57507df
Diff: https://reviews.apache.org/r/27004/diff/
Testing
-------
yes tests pass.
Thanks,
Veena Basavaraj
Re: Review Request 27004: SQOOP-1554:Add NullConfigurationClass to
support use cases that do not have a particular type of config
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Oct. 21, 2014, 7:06 p.m., Qian Xu wrote:
> > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java, line 80
> > <https://reviews.apache.org/r/27004/diff/1/?file=728318#file728318line80>
> >
> > Your removed file LinkConfiguratoin has a note about DERBYREPO-0008. If no input elements defined in a configuration, derby will complain.
> >
> > I suggest fix this issue first, and then replace all empty link configuration with EmptyLinkConfiguration.
good point, I need to fix the derby code first locally I had this change, I will add it back and create a JIRA to allow null configs.
For now I will have a emptyInput.
/*if (config.getInputs().size() == 0) {
throw new SqoopException(DerbyRepoError.DERBYREPO_0008,
"connector-" + configConnectorId
+ "; config: " + config
);
}*/
I dont like that we enforce this in the derby code and each repository implementation can deviate and do its own thing with things like this
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27004/#review57729
-----------------------------------------------------------
On Oct. 21, 2014, 4:59 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27004/
> -----------------------------------------------------------
>
> (Updated Oct. 21, 2014, 4:59 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see JIRA for details.
>
> also it fixes a lot of warnings in the code base.
>
>
> Diffs
> -----
>
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java e63e464
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java 74b1cb8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java 2c8b6c8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsInitializer.java bb5e353
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java 660418d
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java f40459f
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java 5d48a29
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfiguration.java c0cd336
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java c6d2f90
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java 552a751
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestPartitioner.java 9d177ec
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyConfig.java PRE-CREATION
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyJobConfiguration.java PRE-CREATION
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyLinkConfiguration.java PRE-CREATION
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 6d0dcb4
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 665a65b
> spi/src/main/java/org/apache/sqoop/job/etl/Extractor.java d6c186d
> spi/src/main/java/org/apache/sqoop/job/etl/Initializer.java 5c48fc3
> spi/src/main/java/org/apache/sqoop/job/etl/Partitioner.java 57507df
>
> Diff: https://reviews.apache.org/r/27004/diff/
>
>
> Testing
> -------
>
> yes tests pass.
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 27004: SQOOP-1554:Add NullConfigurationClass to
support use cases that do not have a particular type of config
Posted by Qian Xu <sx...@googlemail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27004/#review57729
-----------------------------------------------------------
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java
<https://reviews.apache.org/r/27004/#comment98560>
Your removed file LinkConfiguratoin has a note about DERBYREPO-0008. If no input elements defined in a configuration, derby will complain.
I suggest fix this issue first, and then replace all empty link configuration with EmptyLinkConfiguration.
- Qian Xu
On Oct. 22, 2014, 7:59 a.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27004/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2014, 7:59 a.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see JIRA for details.
>
> also it fixes a lot of warnings in the code base.
>
>
> Diffs
> -----
>
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java e63e464
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java 74b1cb8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java 2c8b6c8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsInitializer.java bb5e353
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java 660418d
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java f40459f
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java 5d48a29
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfiguration.java c0cd336
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java c6d2f90
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java 552a751
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestPartitioner.java 9d177ec
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyConfig.java PRE-CREATION
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyJobConfiguration.java PRE-CREATION
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyLinkConfiguration.java PRE-CREATION
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 6d0dcb4
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 665a65b
> spi/src/main/java/org/apache/sqoop/job/etl/Extractor.java d6c186d
> spi/src/main/java/org/apache/sqoop/job/etl/Initializer.java 5c48fc3
> spi/src/main/java/org/apache/sqoop/job/etl/Partitioner.java 57507df
>
> Diff: https://reviews.apache.org/r/27004/diff/
>
>
> Testing
> -------
>
> yes tests pass.
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 27004: SQOOP-1554:Add NullConfigurationClass to
support use cases that do not have a particular type of config
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Oct. 21, 2014, 8:56 p.m., Jarek Cecho wrote:
> > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java, line 25
> > <https://reviews.apache.org/r/27004/diff/1/?file=728319#file728319line25>
> >
> > HDFS is actually using the Job configuration, so we should probably use the correct class here, right?
> >
> > (I've seen the same on multiple places, but I've highlighted it only here)
there is no code in the destroyer that uses the configs.
> On Oct. 21, 2014, 8:56 p.m., Jarek Cecho wrote:
> > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyJobConfiguration.java, lines 29-34
> > <https://reviews.apache.org/r/27004/diff/1/?file=728330#file728330line29>
> >
> > Why do we need the empty body here? It seems weird. Wouldn't be simpler to just have the class completely empty?
> >
> > Also considering that we have a high level entity of configuration, I would just create "EmptyConfiguration" class and use it at any place where Link/Job are required. It seems to me that "EmptyJobConfiguration" and "EmptyLinkConfiguration" are just copy&paste.
I did this for a reason, not because I wanted to copy paste.
In a typed java system such as this, it does not allow the same class to be used. try it out yourself before saying so.
If there is way to do it, I am more than happy to remove classes
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27004/#review57740
-----------------------------------------------------------
On Oct. 21, 2014, 4:59 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27004/
> -----------------------------------------------------------
>
> (Updated Oct. 21, 2014, 4:59 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see JIRA for details.
>
> also it fixes a lot of warnings in the code base.
>
>
> Diffs
> -----
>
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java e63e464
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java 74b1cb8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java 2c8b6c8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsInitializer.java bb5e353
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java 660418d
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java f40459f
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java 5d48a29
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfiguration.java c0cd336
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java c6d2f90
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java 552a751
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestPartitioner.java 9d177ec
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyConfig.java PRE-CREATION
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyJobConfiguration.java PRE-CREATION
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyLinkConfiguration.java PRE-CREATION
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 6d0dcb4
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 665a65b
> spi/src/main/java/org/apache/sqoop/job/etl/Extractor.java d6c186d
> spi/src/main/java/org/apache/sqoop/job/etl/Initializer.java 5c48fc3
> spi/src/main/java/org/apache/sqoop/job/etl/Partitioner.java 57507df
>
> Diff: https://reviews.apache.org/r/27004/diff/
>
>
> Testing
> -------
>
> yes tests pass.
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 27004: SQOOP-1554:Add NullConfigurationClass to
support use cases that do not have a particular type of config
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Oct. 21, 2014, 8:56 p.m., Jarek Cecho wrote:
> > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyJobConfiguration.java, lines 29-34
> > <https://reviews.apache.org/r/27004/diff/1/?file=728330#file728330line29>
> >
> > Why do we need the empty body here? It seems weird. Wouldn't be simpler to just have the class completely empty?
> >
> > Also considering that we have a high level entity of configuration, I would just create "EmptyConfiguration" class and use it at any place where Link/Job are required. It seems to me that "EmptyJobConfiguration" and "EmptyLinkConfiguration" are just copy&paste.
>
> Veena Basavaraj wrote:
> I did this for a reason, not because I wanted to copy paste.
>
> In a typed java system such as this, it does not allow the same class to be used. try it out yourself before saying so.
>
> If there is way to do it, I am more than happy to remove classes
I am not sure why I had compile errors before, my bad, it actually works with a single class too. uploaded a new RB
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27004/#review57740
-----------------------------------------------------------
On Oct. 21, 2014, 4:59 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27004/
> -----------------------------------------------------------
>
> (Updated Oct. 21, 2014, 4:59 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see JIRA for details.
>
> also it fixes a lot of warnings in the code base.
>
>
> Diffs
> -----
>
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java e63e464
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java 74b1cb8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java 2c8b6c8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsInitializer.java bb5e353
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java 660418d
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java f40459f
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java 5d48a29
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfiguration.java c0cd336
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java c6d2f90
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java 552a751
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestPartitioner.java 9d177ec
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyConfig.java PRE-CREATION
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyJobConfiguration.java PRE-CREATION
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyLinkConfiguration.java PRE-CREATION
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 6d0dcb4
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 665a65b
> spi/src/main/java/org/apache/sqoop/job/etl/Extractor.java d6c186d
> spi/src/main/java/org/apache/sqoop/job/etl/Initializer.java 5c48fc3
> spi/src/main/java/org/apache/sqoop/job/etl/Partitioner.java 57507df
>
> Diff: https://reviews.apache.org/r/27004/diff/
>
>
> Testing
> -------
>
> yes tests pass.
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 27004: SQOOP-1554:Add NullConfigurationClass to
support use cases that do not have a particular type of config
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Oct. 21, 2014, 8:56 p.m., Jarek Cecho wrote:
> > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java, line 25
> > <https://reviews.apache.org/r/27004/diff/1/?file=728319#file728319line25>
> >
> > HDFS is actually using the Job configuration, so we should probably use the correct class here, right?
> >
> > (I've seen the same on multiple places, but I've highlighted it only here)
>
> Veena Basavaraj wrote:
> there is no code in the destroyer that uses the configs.
>
> Veena Basavaraj wrote:
> Its empty class!
>
> public class HdfsDestroyer extends Destroyer<EmptyConfiguration, EmptyConfiguration> {
> /**
> * Callback to clean up after job execution.
> *
> * @param context Destroyer context
> * @param linkConfig Link configuration object
> * @param jobConfig Job configuration object
> */
> @Override
> public void destroy(DestroyerContext context, EmptyConfiguration linkConfig, EmptyConfiguration jobConfig) {
>
> }
> }
good learning for me that it should take a from or to job config. Hence added the corresponding initializer/Destroyers for from and to.
asa far as supporting empty initializer/ destroyer to make job easy for conenctor developers for I have added another ticket.
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27004/#review57740
-----------------------------------------------------------
On Oct. 21, 2014, 10:23 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27004/
> -----------------------------------------------------------
>
> (Updated Oct. 21, 2014, 10:23 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see JIRA for details.
>
> also it fixes a lot of warnings in the code base.
>
>
> Diffs
> -----
>
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java e63e464
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java 74b1cb8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java 2c8b6c8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsInitializer.java bb5e353
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java 660418d
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java f40459f
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java 5d48a29
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfiguration.java c0cd336
> connector/connector-hdfs/src/main/resources/hdfs-connector-config.properties 9b8c6ba
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java c6d2f90
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java 552a751
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestPartitioner.java 9d177ec
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyConfiguration.java PRE-CREATION
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 6d0dcb4
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 665a65b
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java aa58850
> spi/src/main/java/org/apache/sqoop/job/etl/Extractor.java d6c186d
> spi/src/main/java/org/apache/sqoop/job/etl/Initializer.java 5c48fc3
> spi/src/main/java/org/apache/sqoop/job/etl/Partitioner.java 57507df
>
> Diff: https://reviews.apache.org/r/27004/diff/
>
>
> Testing
> -------
>
> yes tests pass.
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 27004: SQOOP-1554:Add NullConfigurationClass to
support use cases that do not have a particular type of config
Posted by Veena Basavaraj <vb...@cloudera.com>.
> On Oct. 21, 2014, 8:56 p.m., Jarek Cecho wrote:
> > connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java, line 25
> > <https://reviews.apache.org/r/27004/diff/1/?file=728319#file728319line25>
> >
> > HDFS is actually using the Job configuration, so we should probably use the correct class here, right?
> >
> > (I've seen the same on multiple places, but I've highlighted it only here)
>
> Veena Basavaraj wrote:
> there is no code in the destroyer that uses the configs.
Its empty class!
public class HdfsDestroyer extends Destroyer<EmptyConfiguration, EmptyConfiguration> {
/**
* Callback to clean up after job execution.
*
* @param context Destroyer context
* @param linkConfig Link configuration object
* @param jobConfig Job configuration object
*/
@Override
public void destroy(DestroyerContext context, EmptyConfiguration linkConfig, EmptyConfiguration jobConfig) {
}
}
- Veena
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27004/#review57740
-----------------------------------------------------------
On Oct. 21, 2014, 4:59 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27004/
> -----------------------------------------------------------
>
> (Updated Oct. 21, 2014, 4:59 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see JIRA for details.
>
> also it fixes a lot of warnings in the code base.
>
>
> Diffs
> -----
>
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java e63e464
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java 74b1cb8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java 2c8b6c8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsInitializer.java bb5e353
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java 660418d
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java f40459f
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java 5d48a29
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfiguration.java c0cd336
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java c6d2f90
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java 552a751
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestPartitioner.java 9d177ec
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyConfig.java PRE-CREATION
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyJobConfiguration.java PRE-CREATION
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyLinkConfiguration.java PRE-CREATION
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 6d0dcb4
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 665a65b
> spi/src/main/java/org/apache/sqoop/job/etl/Extractor.java d6c186d
> spi/src/main/java/org/apache/sqoop/job/etl/Initializer.java 5c48fc3
> spi/src/main/java/org/apache/sqoop/job/etl/Partitioner.java 57507df
>
> Diff: https://reviews.apache.org/r/27004/diff/
>
>
> Testing
> -------
>
> yes tests pass.
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 27004: SQOOP-1554:Add NullConfigurationClass to
support use cases that do not have a particular type of config
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27004/#review57740
-----------------------------------------------------------
Thank you for taking stab at this one Veena!
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java
<https://reviews.apache.org/r/27004/#comment98582>
HDFS is actually using the Job configuration, so we should probably use the correct class here, right?
(I've seen the same on multiple places, but I've highlighted it only here)
connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyJobConfiguration.java
<https://reviews.apache.org/r/27004/#comment98583>
Why do we need the empty body here? It seems weird. Wouldn't be simpler to just have the class completely empty?
Also considering that we have a high level entity of configuration, I would just create "EmptyConfiguration" class and use it at any place where Link/Job are required. It seems to me that "EmptyJobConfiguration" and "EmptyLinkConfiguration" are just copy&paste.
Jarcec
- Jarek Cecho
On Oct. 21, 2014, 11:59 p.m., Veena Basavaraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27004/
> -----------------------------------------------------------
>
> (Updated Oct. 21, 2014, 11:59 p.m.)
>
>
> Review request for Sqoop.
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> see JIRA for details.
>
> also it fixes a lot of warnings in the code base.
>
>
> Diffs
> -----
>
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java e63e464
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java 74b1cb8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java 2c8b6c8
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsInitializer.java bb5e353
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java 660418d
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java f40459f
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java 5d48a29
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfiguration.java c0cd336
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java c6d2f90
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java 552a751
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestPartitioner.java 9d177ec
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyConfig.java PRE-CREATION
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyJobConfiguration.java PRE-CREATION
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyLinkConfiguration.java PRE-CREATION
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 6d0dcb4
> execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 665a65b
> spi/src/main/java/org/apache/sqoop/job/etl/Extractor.java d6c186d
> spi/src/main/java/org/apache/sqoop/job/etl/Initializer.java 5c48fc3
> spi/src/main/java/org/apache/sqoop/job/etl/Partitioner.java 57507df
>
> Diff: https://reviews.apache.org/r/27004/diff/
>
>
> Testing
> -------
>
> yes tests pass.
>
>
> Thanks,
>
> Veena Basavaraj
>
>
Re: Review Request 27004: SQOOP-1554:Add NullConfigurationClass to
support use cases that do not have a particular type of config
Posted by Veena Basavaraj <vb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27004/
-----------------------------------------------------------
(Updated Oct. 21, 2014, 4:59 p.m.)
Review request for Sqoop.
Repository: sqoop-sqoop2
Description (updated)
-------
see JIRA for details.
also it fixes a lot of warnings in the code base.
Diffs
-----
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConnector.java e63e464
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsDestroyer.java 74b1cb8
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java 2c8b6c8
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsInitializer.java bb5e353
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java 660418d
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartitioner.java f40459f
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfig.java 5d48a29
connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/LinkConfiguration.java c0cd336
connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java c6d2f90
connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java 552a751
connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestPartitioner.java 9d177ec
connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyConfig.java PRE-CREATION
connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyJobConfiguration.java PRE-CREATION
connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/EmptyLinkConfiguration.java PRE-CREATION
execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMapReduce.java 6d0dcb4
execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 665a65b
spi/src/main/java/org/apache/sqoop/job/etl/Extractor.java d6c186d
spi/src/main/java/org/apache/sqoop/job/etl/Initializer.java 5c48fc3
spi/src/main/java/org/apache/sqoop/job/etl/Partitioner.java 57507df
Diff: https://reviews.apache.org/r/27004/diff/
Testing (updated)
-------
yes tests pass.
Thanks,
Veena Basavaraj