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


Ship it!




Lgtm.

I also ran the unit and 3rd party tests successfully.


src/java/org/apache/sqoop/mapreduce/parquet/ParquetJobConfiguratorFactoryProvider.java
Lines 27 (patched)
<https://reviews.apache.org/r/67268/#comment286157>

    You could throw an AssertionError here, as recommended in Effective Java :)
    
    http://www.informit.com/articles/article.aspx?p=1216151&seqNum=4


- Fero Szabo


On May 23, 2018, 3:34 p.m., Szabolcs Vasas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67268/
> -----------------------------------------------------------
> 
> (Updated May 23, 2018, 3:34 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3319
>     https://issues.apache.org/jira/browse/SQOOP-3319
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> Kite Dataset API is used in many places in the code to read/write Parquet 
> files and configure MR jobs.
> 
> The goal of this JIRA is to introduce an implementation agnostic interface 
> for Parquet reading/writing and extract the code using Kite Dataset API into 
> separate classes implementing this interface. The benefit of this refactoring 
> is that it enables us introducing a new Parquet reading/writing 
> implementation which does not use Kite but plugs in easily.
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/avro/AvroUtil.java 
> 603cc631c9c45e3bc86f8c401da29cb1ba50d417 
>   src/java/org/apache/sqoop/manager/ConnManager.java 
> d7d6279a17c72c2d65a1d6db1539853a8246e143 
>   src/java/org/apache/sqoop/manager/CubridManager.java 
> e27f616c2aad60f66e59065354f30985418fef9e 
>   src/java/org/apache/sqoop/manager/Db2Manager.java 
> 7ff68ce015d8db0a9b3b9a627ad75e94e2bf51c2 
>   src/java/org/apache/sqoop/manager/DirectPostgresqlManager.java 
> c05e1c191fa071ac3f80f3d9316e83c0c99716ec 
>   src/java/org/apache/sqoop/manager/MainframeManager.java 
> a6002ef47e604e029e3f1197ad8282bb48953c53 
>   src/java/org/apache/sqoop/manager/MySQLManager.java 
> 2d177071204f6c62c0862c9df33debed2184e034 
>   src/java/org/apache/sqoop/manager/OracleManager.java 
> b7005d467557df682a0045c1ebbb1c1efe41099a 
>   src/java/org/apache/sqoop/manager/SQLServerManager.java 
> d57a4935d465e7b75228475e2078e580fd88e92e 
>   src/java/org/apache/sqoop/manager/SqlManager.java 
> 4572098831e1482d32979957f4a4406c087cfc1c 
>   src/java/org/apache/sqoop/manager/oracle/OraOopConnManager.java 
> 10524e3a721bd40289ffaeb9368faa7188e8b195 
>   src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java 
> a5962ba44282fc3ae48de23860de0992586e549a 
>   src/java/org/apache/sqoop/mapreduce/ImportJobBase.java 
> fb5d0541fa685d90b267db775f67de4c9f4f1902 
>   src/java/org/apache/sqoop/mapreduce/JdbcCallExportJob.java 
> b7eea93611e50e922342ffbe4d566c6aa9a51bb1 
>   src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 
> 37198363580d8ab4ed1fcc287bd2d8a2182c0fad 
>   src/java/org/apache/sqoop/mapreduce/JdbcUpdateExportJob.java 
> 86069c4619b03a35fc4b902fa943594f68cd4eb9 
>   src/java/org/apache/sqoop/mapreduce/JdbcUpsertExportJob.java 
> 9a8c17a98b66f8c57c0f96347b3a17fc922b47d1 
>   src/java/org/apache/sqoop/mapreduce/MergeJob.java 
> bb21b64da9a2d296be54657cbd0129636fa0a4c8 
>   src/java/org/apache/sqoop/mapreduce/MergeParquetReducer.java 
> caa4f5f760b9c2be604c89937ba7ad0a4bfa99a0 
>   src/java/org/apache/sqoop/mapreduce/ParquetExportMapper.java 
> 2bc0cba1466092b31f2263fd64a7d456177cfb2d 
>   src/java/org/apache/sqoop/mapreduce/ParquetImportMapper.java 
> 35ab495790d5d80b5f9bf8de92a5b61cd0eb6b2e 
>   src/java/org/apache/sqoop/mapreduce/ParquetJob.java 
> 46047733cce29ae11d227eab79280ed9ee6a84b5 
>   src/java/org/apache/sqoop/mapreduce/mainframe/MainframeImportJob.java 
> 7e975c7bbadde0fba5a09798c952be0da7d44ea9 
>   src/java/org/apache/sqoop/mapreduce/parquet/ParquetConstants.java 
> PRE-CREATION 
>   
> src/java/org/apache/sqoop/mapreduce/parquet/ParquetExportJobConfigurator.java 
> PRE-CREATION 
>   
> src/java/org/apache/sqoop/mapreduce/parquet/ParquetImportJobConfigurator.java 
> PRE-CREATION 
>   
> src/java/org/apache/sqoop/mapreduce/parquet/ParquetJobConfiguratorFactory.java
>  PRE-CREATION 
>   
> src/java/org/apache/sqoop/mapreduce/parquet/ParquetJobConfiguratorFactoryProvider.java
>  PRE-CREATION 
>   
> src/java/org/apache/sqoop/mapreduce/parquet/ParquetMergeJobConfigurator.java 
> PRE-CREATION 
>   
> src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteMergeParquetReducer.java 
> PRE-CREATION 
>   
> src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteParquetExportJobConfigurator.java
>  PRE-CREATION 
>   
> src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteParquetImportJobConfigurator.java
>  PRE-CREATION 
>   
> src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteParquetImportMapper.java 
> PRE-CREATION 
>   
> src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteParquetJobConfiguratorFactory.java
>  PRE-CREATION 
>   
> src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteParquetMergeJobConfigurator.java
>  PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/postgresql/PostgreSQLCopyExportJob.java 
> e4b1350077068d5c3118699993258bab174e19b7 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 
> 783651a495092bd5dbaae5b62fce45c98d4a96c3 
>   src/java/org/apache/sqoop/tool/ImportTool.java 
> ee79d8b7e3b090c0a35dae2b84b1dfd13199a7d8 
>   src/java/org/apache/sqoop/tool/MergeTool.java 
> 311fee8d6d335e33a5fc14d98c94f26f146553eb 
>   src/test/org/apache/sqoop/TestParquetImport.java 
> 0f9c7f3e9bb589b6985179bb06d5b3d37e5a1820 
>   src/test/org/apache/sqoop/hive/TestHiveImport.java 
> 77674db89e3513bb30854dfda60c53ec02b6d336 
>   src/test/org/apache/sqoop/mapreduce/TestJdbcExportJob.java 
> a900b1c92e3ae50d149314c3274c9ac82c37274e 
>   src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeImportJob.java 
> a133e580d95ca366f8eab17374155d5c5e8bb0f1 
> 
> 
> Diff: https://reviews.apache.org/r/67268/diff/1/
> 
> 
> Testing
> -------
> 
> Ran unit and third party tests.
> 
> 
> Thanks,
> 
> Szabolcs Vasas
> 
>

Reply via email to