Re: Sqoop Meetup Page

[Jarek Cecho]

Thank you for volunteering Sumit!

It seems that there are no objections, so let me contact you off the list and 
figure out how to move the ownership :)

Jarcec

> On Nov 21, 2017, at 2:03 PM, Sumit Sarkar  wrote:
> 
> If there is interest and the group is fine with sponsorship, I’m open to 
> organize the meetup using resources from my employer (Progress DataDirect) 
> who serves on the JDBC Expert Group and develops commercial JDBC drivers for 
> use with Apache Sqoop 
> .
>  We also use Sqoop in house to ingest customer data into our data lake.
>  
> Let me know,
> Sumit
> 919-461-4284
> Schedule a chat about data connectivity: https://calendly.com/sasinsumit/ 
> 
>  
> From: jar...@apache.org [mailto:jar...@apache.org] 
> Sent: Monday, November 20, 2017 10:35 AM
> To: u...@sqoop.apache.org; dev@sqoop.apache.org
> Subject: Sqoop Meetup Page
>  
> Dear Sqoop users and developers,,
> as you might know, Sqoop have it’s own Sqoop meetup page:
> 
> https://www.meetup.com/Sqoop-User-Meetup/ 
> 
> 
> I’m currently the main organizer, but due to my day job keeping me 
> sufficiently occupied, I’m not doing the meetup justice. As a result last 
> meetup we organized through this page was more then three years ago (October 
> 2014). Hence I wonder if it’s still useful to keep it around - and I’m 
> curious what the community think at large? Do you see value in resurrecting 
> the page?
> 
> Jarcec

Re: Review Request 51159: SQOOP-2998: Sqoop2: Strings with a ' (single quote) read from hdfs may be improperly escaped

[Jarek Cecho]

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


Ship it!




Ship It!

- Jarek Cecho


On Aug. 17, 2016, 6:30 a.m., Abraham Fine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51159/
> ---
> 
> (Updated Aug. 17, 2016, 6:30 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2998
> https://issues.apache.org/jira/browse/SQOOP-2998
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Sqoop2: Strings with a ' (single quote) read from hdfs may be improperly 
> escaped
> 
> 
> Diffs
> -
> 
>   
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java
>  7cef93c35ba87840bc6d49a8c7167b3721c6be6a 
>   
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/hdfsWriter/GenericHdfsWriter.java
>  31023e7360300f00c3528cfb33170ed4e9e8ab6f 
>   
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/hdfsWriter/HdfsParquetWriter.java
>  4ec813b0f263df2cc8f08781bc40946341eda0c9 
>   
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/hdfsWriter/HdfsSequenceWriter.java
>  dcce861784301ca62bf18319639e06f2604d3362 
>   
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/hdfsWriter/HdfsTextWriter.java
>  384e3309138a0f7fd525febbf3452b654ff0e5d0 
>   
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java
>  cbd555a8e4deb44f162bda14e721d0fc60ad9425 
>   test/src/main/java/org/apache/sqoop/test/data/Cities.java 
> f2c69bb406e5cc78705910b94842bc3b39f5126a 
>   
> test/src/test/java/org/apache/sqoop/integration/connector/hdfs/AppendModeTest.java
>  68855257cc4d64a3fbbf72f79e36343e724e 
>   
> test/src/test/java/org/apache/sqoop/integration/connector/hdfs/FromHDFSToHDFSTest.java
>  c6ce1e879e70d80ac8089313281d5adf08f4ed69 
>   
> test/src/test/java/org/apache/sqoop/integration/connector/hdfs/HdfsIncrementalReadTest.java
>  37306e2e21780085a2fe8aa8541c04e398712678 
>   
> test/src/test/java/org/apache/sqoop/integration/connector/hdfs/NullValueTest.java
>  1e8c688c28c1114e08f9941536a75656167e2a7f 
>   
> test/src/test/java/org/apache/sqoop/integration/connector/hdfs/OutputDirectoryTest.java
>  330da56fa04fb2ecd84cbdc643b0cc1bf097b313 
>   
> test/src/test/java/org/apache/sqoop/integration/connector/hdfs/ParquetTest.java
>  d55563dd62f0cb1eb2b1d4f2bf4325ee1d9fda52 
>   test/src/test/java/org/apache/sqoop/integration/connector/hdfs/S3Test.java 
> c8576999c175243df96071a7535499eaf4c3d8f9 
>   
> test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/FromHDFSToRDBMSTest.java
>  933bc08da00aacae2e1d5cd52a463cc52c7c8c3d 
>   
> test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/FromRDBMSToHDFSTest.java
>  7e6609183378d5374e86d0b6c1d94509ca8f5f8e 
>   
> test/src/test/java/org/apache/sqoop/integration/connector/kite/FromRDBMSToKiteTest.java
>  10f36144d77c275fcad74397ca2f42fe67ab918f 
> 
> Diff: https://reviews.apache.org/r/51159/diff/
> 
> 
> Testing
> ---
> 
> yes
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>

Re: Review Request 51180: Sqoop ClassNotFoundException (org.apache.commons.lang3.StringUtils) is thrown when executing Oracle direct import map task

[Jarek Cecho]

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


Ship it!




Ship It!

- Jarek Cecho


On Aug. 17, 2016, 3:09 p.m., Szabolcs Vasas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51180/
> ---
> 
> (Updated Aug. 17, 2016, 3:09 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2995
> https://issues.apache.org/jira/browse/SQOOP-2995
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> Sqoop ClassNotFoundException (org.apache.commons.lang3.StringUtils) is thrown 
> when executing Oracle direct import map task
> 
> 
> Diffs
> -
> 
>   ivy.xml d84b88f 
>   ivy/libraries.properties 847b6e1 
> 
> Diff: https://reviews.apache.org/r/51180/diff/
> 
> 
> Testing
> ---
> 
> Testing Sqoop on a cluster.
> 
> 
> Thanks,
> 
> Szabolcs Vasas
> 
>

Re: Review Request 47278: SQOOP-2923: Sqoop2: Reword documentation to make it clear that the api endpoints fall under /sqoop

[Jarek Cecho]

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


Ship it!




Ship It!

- Jarek Cecho


On May 12, 2016, 1:09 a.m., Abraham Fine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47278/
> ---
> 
> (Updated May 12, 2016, 1:09 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2923
> https://issues.apache.org/jira/browse/SQOOP-2923
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Sqoop2: Reword documentation to make it clear that the api endpoints fall 
> under /sqoop
> 
> 
> Diffs
> -
> 
>   docs/src/site/sphinx/dev/RESTAPI.rst 7929b28 
>   docs/src/site/sphinx/security/API TLS-SSL.rst e3eed0c 
> 
> Diff: https://reviews.apache.org/r/47278/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>

Re: Review Request 46707: SQOOP-2915 changes

[Jarek Cecho]

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


Ship it!




Ship It!

- Jarek Cecho


On April 26, 2016, 5:59 p.m., Attila Szabo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46707/
> ---
> 
> (Updated April 26, 2016, 5:59 p.m.)
> 
> 
> Review request for Sqoop and Abraham Fine.
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> Changes implemented for SQOOP-2915
> 
> 
> Diffs
> -
> 
>   src/test/com/cloudera/sqoop/manager/OracleExportTest.java 7ee3d94 
>   src/test/com/cloudera/sqoop/manager/OracleUtils.java 655aa61 
>   src/test/org/apache/sqoop/manager/oracle/ExportTest.java 80b6536 
>   src/test/org/apache/sqoop/manager/oracle/OracleCallExportTest.java 44b2f9a 
>   src/test/org/apache/sqoop/manager/oracle/TimestampDataTest.java f338595 
> 
> Diff: https://reviews.apache.org/r/46707/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Attila Szabo
> 
>

Re: Review Request 46063: SQOOP-2900

[Jarek Cecho]

> On April 18, 2016, 8:50 p.m., Jarek Cecho wrote:
> > repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java,
> >  lines 323-330
> > <https://reviews.apache.org/r/46063/diff/4/?file=1348704#file1348704line323>
> >
> > I understand the additional joins here, but is it intentional to get 
> > rid of the ORDER BY?
> 
> Abraham Fine wrote:
> it was intentional. we never care about the order of the output from this 
> query. it simply is not needed and i did not feel it worthy of another jira. 
> let me know if you disagree.

Thanks for the quick answers. I know that we have several queries where the 
order actually matters, but this one seems to be used only on the 
encryption/decryption where the order indeed shouldn't matter.


- Jarek


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


On April 18, 2016, 8:35 p.m., Abraham Fine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46063/
> ---
> 
> (Updated April 18, 2016, 8:35 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2900
> https://issues.apache.org/jira/browse/SQOOP-2900
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Support encrypting map inputs with sensitive fields
> 
> 
> Diffs
> -
> 
>   
> common/src/main/java/org/apache/sqoop/json/util/ConfigInputSerialization.java 
> 4ce977cf43822314c4d34b0fb48ceffd80eb9bd8 
>   
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
>  4cae8113b2a935cc294922108f8c573e5f090513 
>   
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java
>  91187225c3cecf69727e32ab40df41138faa86cd 
>   
> test/src/test/java/org/apache/sqoop/integration/tools/RepositoryEncryptionToolTest.java
>  5f588f5ac167bf0f1f4309deafe655653528b36e 
>   
> tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryEncryptionTool.java 
> d0eab40c4a83c8c536c0af26bd91565d49f2661c 
> 
> Diff: https://reviews.apache.org/r/46063/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>

Re: Review Request 46063: SQOOP-2900

[Jarek Cecho]

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




repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java
 (lines 323 - 330)
<https://reviews.apache.org/r/46063/#comment192852>

I understand the additional joins here, but is it intentional to get rid of 
the ORDER BY?


Jarcec

- Jarek Cecho


On April 18, 2016, 8:35 p.m., Abraham Fine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46063/
> ---
> 
> (Updated April 18, 2016, 8:35 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2900
> https://issues.apache.org/jira/browse/SQOOP-2900
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Support encrypting map inputs with sensitive fields
> 
> 
> Diffs
> -
> 
>   
> common/src/main/java/org/apache/sqoop/json/util/ConfigInputSerialization.java 
> 4ce977cf43822314c4d34b0fb48ceffd80eb9bd8 
>   
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
>  4cae8113b2a935cc294922108f8c573e5f090513 
>   
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java
>  91187225c3cecf69727e32ab40df41138faa86cd 
>   
> test/src/test/java/org/apache/sqoop/integration/tools/RepositoryEncryptionToolTest.java
>  5f588f5ac167bf0f1f4309deafe655653528b36e 
>   
> tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryEncryptionTool.java 
> d0eab40c4a83c8c536c0af26bd91565d49f2661c 
> 
> Diff: https://reviews.apache.org/r/46063/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>

Re: Review Request 45860: SQOOP-2891

[Jarek Cecho]

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


Ship it!




Ship It!

- Jarek Cecho


On April 11, 2016, 11:46 p.m., Abraham Fine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45860/
> ---
> 
> (Updated April 11, 2016, 11:46 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2891
> https://issues.apache.org/jira/browse/SQOOP-2891
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Repository encryption documentation
> 
> 
> Diffs
> -
> 
>   docs/src/site/sphinx/admin/Tools.rst 
> 36fc231b5febc94d4860611e62d187bbac552df0 
>   docs/src/site/sphinx/security/API TLS-SSL.rst PRE-CREATION 
>   docs/src/site/sphinx/security/Encryption.rst 
> 45ffc1787c7ade9a81efaf463baa89c8ed0c90e8 
>   docs/src/site/sphinx/security/RepositoryEncryption.rst PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45860/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>

Re: Review Request 45860: SQOOP-2891

[Jarek Cecho]

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




docs/src/site/sphinx/security/Encryption.rst (lines 17 - 19)
<https://reviews.apache.org/r/45860/#comment191201>

Can we break the document to two guides - one for SSL another for 
Repository encryption please? This will create two different topics in TOC and 
such, so it will be much more visible :)



docs/src/site/sphinx/security/Encryption.rst (lines 136 - 140)
<https://reviews.apache.org/r/45860/#comment191202>

Or the passwrod generator: ::
 org.a

(e.g. you can save the empty line and line with just ::)



docs/src/site/sphinx/security/Encryption.rst (lines 191 - 192)
<https://reviews.apache.org/r/45860/#comment191203>

Can we document all the tools in one section of the guide please? 

We can always refer from here to the tools guide if needed. We have 
something like that for example in section about upgrades where we're referring 
to the upgrade tool.



docs/src/site/sphinx/security/Encryption.rst (line 201)
<https://reviews.apache.org/r/45860/#comment191204>

s/which should/which will/


Jarcec

- Jarek Cecho


On April 8, 2016, 7:53 p.m., Abraham Fine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45860/
> ---
> 
> (Updated April 8, 2016, 7:53 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2891
> https://issues.apache.org/jira/browse/SQOOP-2891
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Repository encryption documentation
> 
> 
> Diffs
> -
> 
>   common/src/main/java/org/apache/sqoop/error/code/CommonRepositoryError.java 
> dd52c820a0c069e506863a34d5dbdcb0a2f903f0 
>   core/src/main/java/org/apache/sqoop/driver/Driver.java 
> 2d1d73ec27db3d24c100947ec982652b03fc4e9e 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 
> 44ab41b9575524456420993b062be9a40e91b829 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 
> 94f5e6f17beb0cdae8a5eebf1d8be6625aefc14e 
>   core/src/main/java/org/apache/sqoop/repository/MasterKeyManager.java 
> df84d540f8c6ede765709443ff4e2f5117f6847b 
>   core/src/main/java/org/apache/sqoop/repository/Repository.java 
> c2e3c749413296fa23ae90c1aac85440fcf7a2b1 
>   core/src/main/java/org/apache/sqoop/security/SecurityError.java 
> 2ba849c3abcc667e901aa4ec55d8c467316f1dee 
>   core/src/test/java/org/apache/sqoop/repository/TestMasterKeyManager.java 
> f9579bf3dc7b74149487f6a892792afe58a75e83 
>   dist/src/main/conf/sqoop.properties 
> 58b60fd192f3332dc7109ad7fc344ebb61e6e9a4 
>   docs/src/site/sphinx/admin/Tools.rst 
> 36fc231b5febc94d4860611e62d187bbac552df0 
>   docs/src/site/sphinx/security/Encryption.rst 
> 6471a07be46dc4c10e5ac40a0159d14fb9543b02 
>   docs/src/site/sphinx/security/RepositoryEncryption.rst PRE-CREATION 
>   
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
>  a9168aee6379cb1b367e475d9bdc352be82aa482 
>   
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java
>  d527290908131cda899f407ce439b77b9d167752 
>   
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaCreateQuery.java
>  b4f66bb61e3112c66cdf8a6bed659262ff63a55e 
>   test/pom.xml 4bac683d9497d7c6f24d1db4bd385efc3be566a3 
>   
> test/src/test/java/org/apache/sqoop/integration/tools/RepositoryDumpLoadToolTest.java
>  c1a10bc32fc093b0a121bc4a2df363e3dbedd485 
>   
> test/src/test/java/org/apache/sqoop/integration/tools/RepositoryEncryptionToolTest.java
>  PRE-CREATION 
>   tools/src/main/java/org/apache/sqoop/tools/tool/BuiltinTools.java 
> 13a2c5f234840e422eb67db63d17645a865e23e9 
>   
> tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryEncryptionTool.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45860/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>

Re: Review Request 45559: SQOOP-2890

[Jarek Cecho]

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



Nice work, thank you!


core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java (line 
483)
<https://reviews.apache.org/r/45559/#comment191200>

It seems that the other create* methods are using method setPersistanceId 
rather then returning the id as a return value. Can we perhaps do the same here?


Jarcec

- Jarek Cecho


On April 5, 2016, 10:28 p.m., Abraham Fine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45559/
> ---
> 
> (Updated April 5, 2016, 10:28 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2890
> https://issues.apache.org/jira/browse/SQOOP-2890
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Provide tooling to encrypt non-encrypted repository and rotate keys
> 
> 
> Diffs
> -
> 
>   common/src/main/java/org/apache/sqoop/error/code/CommonRepositoryError.java 
> dd52c82 
>   core/src/main/java/org/apache/sqoop/driver/Driver.java 2d1d73e 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 44ab41b 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 
> 94f5e6f 
>   core/src/main/java/org/apache/sqoop/repository/MasterKeyManager.java 
> df84d54 
>   core/src/main/java/org/apache/sqoop/repository/Repository.java c2e3c74 
>   core/src/main/java/org/apache/sqoop/security/SecurityError.java 2ba849c 
>   core/src/test/java/org/apache/sqoop/repository/TestMasterKeyManager.java 
> f9579bf 
>   dist/src/main/conf/sqoop.properties 58b60fd 
>   
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
>  a9168ae 
>   
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java
>  d527290 
>   
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaCreateQuery.java
>  b4f66bb 
>   test/pom.xml 4bac683 
>   
> test/src/test/java/org/apache/sqoop/integration/tools/RepositoryDumpLoadToolTest.java
>  c1a10bc 
>   
> test/src/test/java/org/apache/sqoop/integration/tools/RepositoryEncryptionToolTest.java
>  PRE-CREATION 
>   tools/src/main/java/org/apache/sqoop/tools/tool/BuiltinTools.java 13a2c5f 
>   
> tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryEncryptionTool.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45559/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>

Re: Review Request 44317: SQOOP-2561

[Jarek Cecho]

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


Ship it!




Ship It!

- Jarek Cecho


On March 29, 2016, 4:49 a.m., vishnu  s nair wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44317/
> ---
> 
> (Updated March 29, 2016, 4:49 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2561
> https://issues.apache.org/jira/browse/SQOOP-2561
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> Special Character removal from Column name as avro data results in
> duplicate column and fails the import
> 
> Solution:  Instead of removing the special character, replace it with
> underscore
> 
> 
> Diffs
> -
> 
>   src/java/org/apache/sqoop/avro/AvroUtil.java 90cc9d0 
>   src/test/com/cloudera/sqoop/TestAvroImport.java 00d7a95 
> 
> Diff: https://reviews.apache.org/r/44317/diff/
> 
> 
> Testing
> ---
> 
> Test case for the same is exceuted successfully.
> 
> 
> File Attachments
> 
> 
> 0001-SQOOP-2561.patch
>   
> https://reviews.apache.org/media/uploaded/files/2016/03/29/a11e1fb7-b6ca-4a2d-9914-cd3049fb9e7f__0001-SQOOP-2561.patch
> 
> 
> Thanks,
> 
> vishnu  s nair
> 
>

Re: Review Request 44984: [WIP] Sqoop2: Encrypt sensitive information in the repository

[Jarek Cecho]

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




common/src/main/java/org/apache/sqoop/model/MMasterKey.java (line 36)
<https://reviews.apache.org/r/44984/#comment187852>

Do we want to be leaking the secret when serializing to String?



core/src/main/java/org/apache/sqoop/repository/RepositoryEncryptionManager.java 
(line 46)
<https://reviews.apache.org/r/44984/#comment187853>

I'm wondering - what is the value of introducing a new manager over merging 
that functionality to RepositoryManager itself?



repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
 (lines 439 - 443)
<https://reviews.apache.org/r/44984/#comment187856>

I would probably recommend to use the same query to retrieve both 
encrypted/unecrypted variant - it will be less headache to maintain all those 
queries.



repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
 (line 2276)
<https://reviews.apache.org/r/44984/#comment187854>

For data verification I would use normal exception rathern then assert. 
We're using assert more to verify code rather then data.


- Jarek Cecho


On March 23, 2016, 7:27 p.m., Abraham Fine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44984/
> ---
> 
> (Updated March 23, 2016, 7:27 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2887
> https://issues.apache.org/jira/browse/SQOOP-2887
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> proof of concept regarding sqoop repository encryption, exceptions are not 
> handled properly and there is plenty of cleanup to do. but you should get the 
> basic idea
> 
> 
> Diffs
> -
> 
>   
> common-test/src/main/java/org/apache/sqoop/common/test/db/DerbyProvider.java 
> 839e5618e3cd86a1df24b53b04aae3721b56ed27 
>   common/src/main/java/org/apache/sqoop/error/code/CommonRepositoryError.java 
> 37eb04aaa30f24577ff755e361e6d0a9d47be37c 
>   common/src/main/java/org/apache/sqoop/model/MMasterKey.java PRE-CREATION 
>   core/src/main/java/org/apache/sqoop/core/SqoopServer.java 
> 80a7b88ac4704972f1de0ddaf21869feedba4aa8 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 
> 5b70f951c98dec7d20ff446f52418234fd98e454 
>   core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 
> feab2ad8360b7d978b263dc0da1d2ff2578f0555 
>   core/src/main/java/org/apache/sqoop/repository/Repository.java 
> 03989a3c053edfc51364e9fb368ec970c1dfecee 
>   
> core/src/main/java/org/apache/sqoop/repository/RepositoryEncryptionManager.java
>  PRE-CREATION 
>   core/src/main/java/org/apache/sqoop/security/SecurityConstants.java 
> 0241c8692fef65866160e0bec53cc5c2da1c17e5 
>   core/src/main/java/org/apache/sqoop/security/SecurityError.java 
> 988e425a041a5829ad95508a1e9b20d0992c868e 
>   
> core/src/test/java/org/apache/sqoop/repository/TestRepositoryEncryptionManager.java
>  PRE-CREATION 
>   dist/src/main/conf/sqoop.properties 
> 767d3f2b3ddf8ca978830e37a321d9defbafc57d 
>   
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
>  15cc41b83adc59c266cb899a19edfcc5fc2ee343 
>   
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java
>  ae16b8517daf4d40fd2ebf4a473e16e9083740d6 
>   
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositorySchemaConstants.java
>  d1940e82c02039c144c58a80b3da07f6d2e38b27 
>   
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
>  ee5e8d10f16365fa55574a819501b6ea48ffe30a 
>   
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaCreateQuery.java
>  177003671863244a094abdb6f4a0184d38e79823 
>   
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbySchemaUpgradeQuery.java
>  5081b82ae2cbbd06cef9061b333dd4ec0b61b815 
>   
> repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/DerbyTestCase.java
>  e4cca07fd062a64ac161929943dbd70b23a01c7f 
>   
> repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepositoryHandler.java
>  2c74c323bf64f5c4b120aa745b1e7208e4ddfb3d 
>   
> repository/repository-mysql/src/main/java/org/apache/sq

Re: Review Request 44541: SQOOP-2881: Sqoop2: EnrichOraOop Connector resource file

[Jarek Cecho]

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

(Updated March 22, 2016, 8:37 p.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2881
https://issues.apache.org/jira/browse/SQOOP-2881


Repository: sqoop-sqoop2


Description
---

Please see parent JIRA for details.


Diffs (updated)
-

  
connector/connector-oracle-jdbc/src/main/resources/oracle-jdbc-connector-config.properties
 d99ab9b 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 44317: SQOOP-2561

[Jarek Cecho]

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



One formatting comment and otherwise +1.


src/test/com/cloudera/sqoop/TestAvroImport.java (lines 252 - 274)
<https://reviews.apache.org/r/44317/#comment187205>

The intendation is completely off and there is a whitespace character.


- Jarek Cecho


On March 11, 2016, 4:49 a.m., vishnu  s nair wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44317/
> ---
> 
> (Updated March 11, 2016, 4:49 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2561
> https://issues.apache.org/jira/browse/SQOOP-2561
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> Special Character removal from Column name as avro data results in
> duplicate column and fails the import
> 
> Solution:  Instead of removing the special character, replace it with
> underscore
> 
> 
> Diffs
> -
> 
>   src/java/org/apache/sqoop/avro/AvroUtil.java 90cc9d0 
>   src/test/com/cloudera/sqoop/TestAvroImport.java 00d7a95 
> 
> Diff: https://reviews.apache.org/r/44317/diff/
> 
> 
> Testing
> ---
> 
> Test case for the same is exceuted successfully.
> 
> 
> File Attachments
> 
> 
> D:\Users\vsnair\workspace\sqoop_trunk
>   
> https://reviews.apache.org/media/uploaded/files/2016/03/11/61b2bb9e-47f9-4e9a-9a94-fe8fd03b02e7__0001-SQOOP-2561.patch
> 
> 
> Thanks,
> 
> vishnu  s nair
> 
>

Re: Review Request 44316: Fix For Issue SQOOP-2846

[Jarek Cecho]

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


Ship it!




Ship It!

- Jarek Cecho


On March 3, 2016, 7:05 a.m., vishnu  s nair wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44316/
> ---
> 
> (Updated March 3, 2016, 7:05 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2846
> https://issues.apache.org/jira/browse/SQOOP-2846
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> After analyzing the issue, it is found that export with update key is
> not implemneted properly in case of avro and parqute file formats.
> In order to correct the issue SQOOP-2846, export with update option is
> implemented for the mentioned file formats. For this appropriate
> mapper, input format and output format classes were used in the
> implementation.
> 
> 
> Diffs
> -
> 
>   src/java/org/apache/sqoop/mapreduce/JdbcUpdateExportJob.java 8fa420e 
>   src/test/com/cloudera/sqoop/TestAvroExport.java 137a6e1 
>   src/test/com/cloudera/sqoop/TestParquetExport.java 86b40fb 
>   src/test/com/cloudera/sqoop/testutil/ExportJobTestCase.java 9a6e8da 
> 
> Diff: https://reviews.apache.org/r/44316/diff/
> 
> 
> Testing
> ---
> 
> Updated test cases were executed successfully and the same is also
> incorporated in the patch.
> 
> 
> Thanks,
> 
> vishnu  s nair
> 
>

Re: Review Request 44316: Fix For Issue SQOOP-2846

[Jarek Cecho]

> On March 21, 2016, 3:49 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/mapreduce/JdbcUpdateExportJob.java, line 233
> > <https://reviews.apache.org/r/44316/diff/1/?file=1278752#file1278752line233>
> >
> > Nit: Trailing whitespace.

Since I've slowed down this patch for so long, I've fixed the whitespace issue 
and will commit it.


- Jarek


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


On March 3, 2016, 7:05 a.m., vishnu  s nair wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44316/
> ---
> 
> (Updated March 3, 2016, 7:05 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2846
> https://issues.apache.org/jira/browse/SQOOP-2846
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> After analyzing the issue, it is found that export with update key is
> not implemneted properly in case of avro and parqute file formats.
> In order to correct the issue SQOOP-2846, export with update option is
> implemented for the mentioned file formats. For this appropriate
> mapper, input format and output format classes were used in the
> implementation.
> 
> 
> Diffs
> -
> 
>   src/java/org/apache/sqoop/mapreduce/JdbcUpdateExportJob.java 8fa420e 
>   src/test/com/cloudera/sqoop/TestAvroExport.java 137a6e1 
>   src/test/com/cloudera/sqoop/TestParquetExport.java 86b40fb 
>   src/test/com/cloudera/sqoop/testutil/ExportJobTestCase.java 9a6e8da 
> 
> Diff: https://reviews.apache.org/r/44316/diff/
> 
> 
> Testing
> ---
> 
> Updated test cases were executed successfully and the same is also
> incorporated in the patch.
> 
> 
> Thanks,
> 
> vishnu  s nair
> 
>

Re: Review Request 44316: Fix For Issue SQOOP-2846

[Jarek Cecho]

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




src/java/org/apache/sqoop/mapreduce/JdbcUpdateExportJob.java (line 233)
<https://reviews.apache.org/r/44316/#comment187177>

Nit: Trailing whitespace.


- Jarek Cecho


On March 3, 2016, 7:05 a.m., vishnu  s nair wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44316/
> ---
> 
> (Updated March 3, 2016, 7:05 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2846
> https://issues.apache.org/jira/browse/SQOOP-2846
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> After analyzing the issue, it is found that export with update key is
> not implemneted properly in case of avro and parqute file formats.
> In order to correct the issue SQOOP-2846, export with update option is
> implemented for the mentioned file formats. For this appropriate
> mapper, input format and output format classes were used in the
> implementation.
> 
> 
> Diffs
> -
> 
>   src/java/org/apache/sqoop/mapreduce/JdbcUpdateExportJob.java 8fa420e 
>   src/test/com/cloudera/sqoop/TestAvroExport.java 137a6e1 
>   src/test/com/cloudera/sqoop/TestParquetExport.java 86b40fb 
>   src/test/com/cloudera/sqoop/testutil/ExportJobTestCase.java 9a6e8da 
> 
> Diff: https://reviews.apache.org/r/44316/diff/
> 
> 
> Testing
> ---
> 
> Updated test cases were executed successfully and the same is also
> incorporated in the patch.
> 
> 
> Thanks,
> 
> vishnu  s nair
> 
>

Review Request 45050: SQOOP-2888: Test case ShowCommandTest is failing after SQOOP-2848

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2888
https://issues.apache.org/jira/browse/SQOOP-2888


Repository: sqoop-sqoop2


Description
---

The problem is that our client calls {{/v1/job?cname=generic-jdbc-connector}} 
when the change in SQOOP-2848 made this invalid and 
only{{/v1/job/all?cname=generic-jdbc-connector}} is allowed.


Diffs
-

  client/src/main/java/org/apache/sqoop/client/request/JobResourceRequest.java 
f64f1cc 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 44716: SQOOP-2880

[Jarek Cecho]

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


Ship it!




Ship It!

- Jarek Cecho


On March 11, 2016, 8:11 p.m., Attila Szabo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44716/
> ---
> 
> (Updated March 11, 2016, 8:11 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> Submitted changes for SQOOP-2880
> 
> 
> Diffs
> -
> 
>   src/java/org/apache/sqoop/SqoopOptions.java 17751c7 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 50dd67d 
>   src/java/org/apache/sqoop/tool/ImportTool.java ad1d48b 
>   src/java/org/apache/sqoop/util/AppendUtils.java 0102ee7 
>   src/test/com/cloudera/sqoop/TestSqoopOptions.java f4660e5 
>   src/test/com/cloudera/sqoop/orm/TestParseMethods.java f222dd7 
> 
> Diff: https://reviews.apache.org/r/44716/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Attila Szabo
> 
>

Re: Review Request 44716: SQOOP-2880

[Jarek Cecho]

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



Thanks for taking this patch up Attila!

Generally I like the approach, just few comments:


src/java/org/apache/sqoop/SqoopOptions.java (line 109)
<https://reviews.apache.org/r/44716/#comment185393>

Let's name it as "temporary.dirRoot" - the "sqoop" prefix is not important 
here and I really want to get rid of the "test" portion as this propertly is 
not meant for test at all :)



src/java/org/apache/sqoop/tool/BaseSqoopTool.java (line 165)
<https://reviews.apache.org/r/44716/#comment185394>

Might I suggest to rename the argument to "temporary-rootdir"?

It's still descriptive and yet shorter :)



src/java/org/apache/sqoop/util/AppendUtils.java 
<https://reviews.apache.org/r/44716/#comment185395>

For users who are using this property already, we should provide a 
"backward" compatible way. Perhaps the default value in SqoopOptions should be 
System.getProperty("sqoop.test.import.rootDir", "_sqoop"); rather then just 
"_sqoop"?


Jarcec

- Jarek Cecho


On March 11, 2016, 7:21 p.m., Attila Szabo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44716/
> -------
> 
> (Updated March 11, 2016, 7:21 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> Submitted changes for SQOOP-2880
> 
> 
> Diffs
> -
> 
>   src/java/org/apache/sqoop/SqoopOptions.java 17751c7 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 50dd67d 
>   src/java/org/apache/sqoop/tool/ImportTool.java ad1d48b 
>   src/java/org/apache/sqoop/util/AppendUtils.java 0102ee7 
>   src/test/com/cloudera/sqoop/TestSqoopOptions.java f4660e5 
>   src/test/com/cloudera/sqoop/orm/TestParseMethods.java f222dd7 
> 
> Diff: https://reviews.apache.org/r/44716/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Attila Szabo
> 
>

Re: Review Request 44317: SQOOP-2561

[Jarek Cecho]

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



Overall, I'm fine with the patch as discussed on the JIRA. I have just one 
comment - it seems that current version of patch  breaks some other tests:

Testcase: testNonstandardCharactersInColumnName took 0.964 sec
FAILED
expected:<AVRO[]1> but was:<AVRO[_]1>
junit.framework.ComparisonFailure: expected:<AVRO[]1> but was:<AVRO[_]1>
at com.cloudera.sqoop.TestAvroImport.checkField(TestAvroImport.java:277)
at 
com.cloudera.sqoop.TestAvroImport.testNonstandardCharactersInColumnName(TestAvroImport.java:226)

Could you take a look?

- Jarek Cecho


On March 3, 2016, 7:15 a.m., vishnu  s nair wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44317/
> ---
> 
> (Updated March 3, 2016, 7:15 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2561
> https://issues.apache.org/jira/browse/SQOOP-2561
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> Special Character removal from Column name as avro data results in
> duplicate column and fails the import
> 
> Solution:  Instead of removing the special character, replace it with
> underscore
> 
> 
> Diffs
> -
> 
>   src/java/org/apache/sqoop/avro/AvroUtil.java 90cc9d0 
>   src/test/com/cloudera/sqoop/TestAvroImport.java 00d7a95 
> 
> Diff: https://reviews.apache.org/r/44317/diff/
> 
> 
> Testing
> ---
> 
> Test case for the same is exceuted successfully.
> 
> 
> Thanks,
> 
> vishnu  s nair
> 
>

Review Request 44595: SQOOP-2495: Sqoop2: Provide simple test that can validate if connector is reasonably formed

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2495
https://issues.apache.org/jira/browse/SQOOP-2495


Repository: sqoop-sqoop2


Description
---

On internal hackathon we we're hacking Sqoop 2 connector with [~singhashish] 
and we went through few troubles that we should address.

We have a lot of requirements for Sqoop connectors that are only documented but 
not enforced by the code, for example:

* Resource bundles need to have names for all properties in configuration 
objects
* Configuration objects needs to be properly annotated

If either of those is incorrect then we happily load the connector just to 
throw some random exceptions during runtime. We should provide simple test case 
that all connectors can reuse to validate that connector is properly formed as 
Sqoop expects.

(Which still doesn't mean that the connector will work as we can't guarantee 
that extractor/loader is properly implemented. But we can at least help people 
to not see random exceptions such as those described in SQOOP-2494).


Diffs
-

  connector/connector-ftp/pom.xml a330266 
  
connector/connector-ftp/src/test/java/org/apache/sqoop/connector/ftp/TestFtpConnector.java
 PRE-CREATION 
  connector/connector-generic-jdbc/pom.xml 8d054c1 
  
connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestGenericJdbcConnector.java
 cc1c58f 
  connector/connector-hdfs/pom.xml 37cf3fa 
  
connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestHdfsConnector.java
 b41bd5a 
  connector/connector-kafka/pom.xml 5f41181 
  
connector/connector-kafka/src/main/java/org/apache/sqoop/connector/kafka/KafkaConnector.java
 2b03fa0 
  
connector/connector-kafka/src/test/java/org/apache/sqoop/connector/kafka/TestKafkaConnector.java
 PRE-CREATION 
  connector/connector-kite/pom.xml a492c5b 
  
connector/connector-kite/src/test/java/org/apache/sqoop/connector/kite/TestKiteConnector.java
 c28f697 
  connector/connector-oracle-jdbc/pom.xml 4262cb2 
  
connector/connector-oracle-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/oracle/TestOracleJdbcConnector.java
 PRE-CREATION 
  connector/connector-sdk-test/pom.xml PRE-CREATION 
  
connector/connector-sdk-test/src/main/java/org/apache/sqoop/connector/spi/SqoopConnectorAsserts.java
 PRE-CREATION 
  connector/connector-sftp/pom.xml 8db1af5 
  
connector/connector-sftp/src/test/java/org/apache/sqoop/connector/sftp/TestSftpConnector.java
 PRE-CREATION 
  connector/pom.xml 7340b37 
  pom.xml 891b2c9 

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


Testing
---


Thanks,

Jarek Cecho

Review Request 44586: SQOOP-2883: Sqoop2: Update model classes to represent new constant for connector resource bundles

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2883
https://issues.apache.org/jira/browse/SQOOP-2883


Repository: sqoop-sqoop2


Description
---

As part of the parent umbrella JIRA, I've made few changes to the resource 
bundles that connectors are providing:

# Added a new key for example that we should add to 
[{{MNamedElement}}|https://github.com/apache/sqoop/blob/sqoop2/common/src/main/java/org/apache/sqoop/model/MNamedElement.java]
# Added a new key {{connector.name}} with human readable connector name. The 
constant should be referred somewhere in the code.


Diffs
-

  common/src/main/java/org/apache/sqoop/model/MNamedElement.java 2e4dab3 
  common/src/test/java/org/apache/sqoop/model/TestMNamedElement.java 8556302 
  
connector/connector-sdk/src/main/java/org/apache/sqoop/connector/spi/SqoopConnector.java
 6733906 

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


Testing
---


Thanks,

Jarek Cecho

Review Request 44585: SQOOP-2882: Sqoop2: Enrich SFTP Connector resource file

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2882
https://issues.apache.org/jira/browse/SQOOP-2882


Repository: sqoop-sqoop2


Description
---

Please see parent JIRA for details.


Diffs
-

  connector/connector-sftp/src/main/resources/sftp-connector-config.properties 
c56c8e0 

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


Testing
---


Thanks,

Jarek Cecho

Review Request 44541: SQOOP-2881: Sqoop2: EnrichOraOop Connector resource file

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2881
https://issues.apache.org/jira/browse/SQOOP-2881


Repository: sqoop-sqoop2


Description
---

Please see parent JIRA for details.


Diffs
-

  
connector/connector-oracle-jdbc/src/main/resources/oracle-jdbc-connector-config.properties
 d99ab9b 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 44489: SQOOP-2876: Sqoop2: Document TLS support

[Jarek Cecho]

> On March 8, 2016, 3:58 p.m., Jarek Cecho wrote:
> > docs/src/site/sphinx/security/Encryption.rst, lines 17-19
> > <https://reviews.apache.org/r/44489/diff/1/?file=1287108#file1287108line17>
> >
> > There will be multiple Encryption points that we will have to 
> > eventually cover. Would it make sense to rename this section to be clear 
> > that it's only for the REST interface?
> 
> Abraham Fine wrote:
> i think we can do that when we add them

Fine with me.


- Jarek


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


On March 8, 2016, 8:44 p.m., Abraham Fine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44489/
> ---
> 
> (Updated March 8, 2016, 8:44 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2876
> https://issues.apache.org/jira/browse/SQOOP-2876
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> SQOOP-2876: Sqoop2: Document TLS support
> 
> 
> Diffs
> -
> 
>   docs/src/site/sphinx/security/Encryption.rst PRE-CREATION 
>   docs/src/site/sphinx/security/SecurityGuideOnSqoop2.rst 
> 7194d3bb72f58a952cc9250a372b823412afe0cd 
> 
> Diff: https://reviews.apache.org/r/44489/diff/
> 
> 
> Testing
> ---
> 
> n/a
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>

Re: Review Request 44489: SQOOP-2876: Sqoop2: Document TLS support

[Jarek Cecho]

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



I've reviewed only the doc changes, the rest is being reviewed as part of 
SQOOP-2844 :)


docs/src/site/sphinx/security/Encryption.rst (lines 17 - 19)
<https://reviews.apache.org/r/44489/#comment184575>

There will be multiple Encryption points that we will have to eventually 
cover. Would it make sense to rename this section to be clear that it's only 
for the REST interface?



docs/src/site/sphinx/security/Encryption.rst (lines 28 - 29)
<https://reviews.apache.org/r/44489/#comment184577>

Can you please make this a real link and hide the URL from the generated 
text?

Here is an example of whow is link made in index.rst page:

Sqoop is licensed under `Apache Software License v2 
<http://www.apache.org/licenses/LICENSE-2.0>`_.



docs/src/site/sphinx/security/Encryption.rst (line 35)
<https://reviews.apache.org/r/44489/#comment184578>

Please enclose the filename in ``.



docs/src/site/sphinx/security/Encryption.rst (line 43)
<https://reviews.apache.org/r/44489/#comment184579>

Please make this proper link.



docs/src/site/sphinx/security/Encryption.rst (lines 80 - 82)
<https://reviews.apache.org/r/44489/#comment184581>

Please enclose all file names in ``.


ve

- Jarek Cecho


On March 8, 2016, 1:05 a.m., Abraham Fine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44489/
> ---
> 
> (Updated March 8, 2016, 1:05 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2876
> https://issues.apache.org/jira/browse/SQOOP-2876
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> SQOOP-2876: Sqoop2: Document TLS support
> 
> 
> Diffs
> -
> 
>   common/src/main/java/org/apache/sqoop/utils/ProcessUtils.java PRE-CREATION 
>   docs/src/site/sphinx/security/Encryption.rst PRE-CREATION 
>   docs/src/site/sphinx/security/SecurityGuideOnSqoop2.rst 
> 7194d3bb72f58a952cc9250a372b823412afe0cd 
>   server/src/main/java/org/apache/sqoop/server/SqoopJettyServer.java 
> 4696a8764420075bb6f7659b41f9fe81e59f7d8a 
>   shell/src/main/java/org/apache/sqoop/shell/SetCommand.java 
> 3feaac3061a8e6008ef021dff8022ea79a2c2ba7 
>   shell/src/main/java/org/apache/sqoop/shell/SetServerFunction.java 
> e430f9d13c8d6f7b637c923c3609e072582290b8 
>   shell/src/main/java/org/apache/sqoop/shell/SetTruststoreFunction.java 
> PRE-CREATION 
>   shell/src/main/java/org/apache/sqoop/shell/ShellEnvironment.java 
> 80ac935e3992c1c5e977a6d06abb6f049ba7d423 
>   shell/src/main/java/org/apache/sqoop/shell/core/Constants.java 
> 8af53f21cb3a8a3555ca25fc65e2b6c5dfca3a2c 
>   shell/src/main/resources/shell-resource.properties 
> 630c31d77a0a96525ef0f3a8ad20b64b5bb24c72 
> 
> Diff: https://reviews.apache.org/r/44489/diff/
> 
> 
> Testing
> ---
> 
> n/a
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>

Review Request 44483: SQOOP-2877: Sqoop2: Enrich Kite Connector resource file

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2877
https://issues.apache.org/jira/browse/SQOOP-2877


Repository: sqoop-sqoop2


Description
---

Please see parent JIRA for details.


Diffs
-

  connector/connector-kite/src/main/resources/kite-connector-config.properties 
f384f3d 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 44410: SQOOP-2870: Sqoop2: RESTiliency: Add tests for DriverHandler

[Jarek Cecho]

> On March 7, 2016, 9:03 p.m., Abraham Fine wrote:
> > test/src/test/java/org/apache/sqoop/integration/server/rest/DriverRestTest.java,
> >  lines 29-33
> > <https://reviews.apache.org/r/44410/diff/1/?file=1281394#file1281394line29>
> >
> > perhaps there would be value in testing the contents of the response 
> > somehow?
> > 
> > is there some text that we know would always be in the serialized 
> > version of the driver?

Yeah good idea. I've added few strings to verify in my next iteration. Let me 
know if that is sufficient from your perspective.


- Jarek


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


On March 7, 2016, 11:11 p.m., Jarek Cecho wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44410/
> ---
> 
> (Updated March 7, 2016, 11:11 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2870
> https://issues.apache.org/jira/browse/SQOOP-2870
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> I would like to addd small test case covering {{DriverHandler}} module 
> similarly as we're building for other REST endpoints.
> 
> 
> Diffs
> -
> 
>   server/src/main/java/org/apache/sqoop/handler/DriverRequestHandler.java 
> 95a3291 
>   
> test/src/test/java/org/apache/sqoop/integration/server/rest/DriverRestTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44410/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Re: Review Request 44410: SQOOP-2870: Sqoop2: RESTiliency: Add tests for DriverHandler

[Jarek Cecho]

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

(Updated March 7, 2016, 11:11 p.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2870
https://issues.apache.org/jira/browse/SQOOP-2870


Repository: sqoop-sqoop2


Description
---

I would like to addd small test case covering {{DriverHandler}} module 
similarly as we're building for other REST endpoints.


Diffs (updated)
-

  server/src/main/java/org/apache/sqoop/handler/DriverRequestHandler.java 
95a3291 
  
test/src/test/java/org/apache/sqoop/integration/server/rest/DriverRestTest.java 
PRE-CREATION 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 44103: SQOOP-2855: Sqoop2: Enrich Generic JDBC Connector resource file

[Jarek Cecho]

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

(Updated March 7, 2016, 11:03 p.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2855
https://issues.apache.org/jira/browse/SQOOP-2855


Repository: sqoop-sqoop2


Description
---

See parent JIRA for details of what is being done here.


Diffs (updated)
-

  
connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-config.properties
 8256beb 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 44103: SQOOP-2855: Sqoop2: Enrich Generic JDBC Connector resource file

[Jarek Cecho]

> On March 5, 2016, 3:21 a.m., Abraham Fine wrote:
> > connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-config.properties,
> >  line 51
> > <https://reviews.apache.org/r/44103/diff/1/?file=1272653#file1272653line51>
> >
> > examples may be helpful here with respect to "how" these should be 
> > entered
> 
> Jarek Cecho wrote:
> I would prefer to stay away from "how" as the text can be used in several 
> different scenarios:
> 
> * Command line (where the "how" would make sense)
> * In documentation (purpose of those changes)
> * In Hue application (GUI)
> 
> Abraham Fine wrote:
> i should have been more clean. i meant an example

Got it. Will add the "example" property.


- Jarek


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


On Feb. 26, 2016, 9:46 p.m., Jarek Cecho wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44103/
> ---
> 
> (Updated Feb. 26, 2016, 9:46 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2855
> https://issues.apache.org/jira/browse/SQOOP-2855
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> See parent JIRA for details of what is being done here.
> 
> 
> Diffs
> -
> 
>   
> connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-config.properties
>  8256beb 
> 
> Diff: https://reviews.apache.org/r/44103/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Review Request 44429: SQOOP-2872: Sqoop2: Enrich Kafka Connector resource file

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2872
https://issues.apache.org/jira/browse/SQOOP-2872


Repository: sqoop-sqoop2


Description
---

Please see parent JIRA for more details.


Diffs
-

  
connector/connector-kafka/src/main/resources/kafka-connector-config.properties 
d3e1e6f 

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


Testing
---


Thanks,

Jarek Cecho

Review Request 44428: SQOOP-2871: Sqoop2: Enrich FTP Connector resource file

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2871
https://issues.apache.org/jira/browse/SQOOP-2871


Repository: sqoop-sqoop2


Description
---

Please see parent JIRA for details.


Diffs
-

  connector/connector-ftp/src/main/resources/ftp-connector-config.properties 
d84157f 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 44103: SQOOP-2855: Sqoop2: Enrich Generic JDBC Connector resource file

[Jarek Cecho]

> On March 5, 2016, 3:21 a.m., Abraham Fine wrote:
> > connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-config.properties,
> >  line 51
> > <https://reviews.apache.org/r/44103/diff/1/?file=1272653#file1272653line51>
> >
> > examples may be helpful here with respect to "how" these should be 
> > entered

I would prefer to stay away from "how" as the text can be used in several 
different scenarios:

* Command line (where the "how" would make sense)
* In documentation (purpose of those changes)
* In Hue application (GUI)


- Jarek


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


On Feb. 26, 2016, 9:46 p.m., Jarek Cecho wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44103/
> -------
> 
> (Updated Feb. 26, 2016, 9:46 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2855
> https://issues.apache.org/jira/browse/SQOOP-2855
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> See parent JIRA for details of what is being done here.
> 
> 
> Diffs
> -
> 
>   
> connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-config.properties
>  8256beb 
> 
> Diff: https://reviews.apache.org/r/44103/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Re: Review Request 44097: SQOOP-2853: Sqoop2: Refactor TableDisplayer to be used in document generation

[Jarek Cecho]

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

(Updated March 5, 2016, 4:52 p.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2853
https://issues.apache.org/jira/browse/SQOOP-2853


Repository: sqoop-sqoop2


Description
---

I would like to refactor the existing 
{{[TableDisplayer|https://github.com/apache/sqoop/blob/sqoop2/shell/src/main/java/org/apache/sqoop/shell/utils/TableDisplayer.java]}},
 so that it can generate tables for documentation as well. Right now that class 
is too tight with the way shell works.


Diffs (updated)
-

  shell/src/main/java/org/apache/sqoop/shell/ShellEnvironment.java 36d0712 
  shell/src/main/java/org/apache/sqoop/shell/ShowConnectorFunction.java d973499 
  shell/src/main/java/org/apache/sqoop/shell/ShowJobFunction.java ebbfe82 
  shell/src/main/java/org/apache/sqoop/shell/ShowLinkFunction.java 25bd1db 
  shell/src/main/java/org/apache/sqoop/shell/ShowPrincipalFunction.java a450aaf 
  shell/src/main/java/org/apache/sqoop/shell/ShowPrivilegeFunction.java 2cf6972 
  shell/src/main/java/org/apache/sqoop/shell/ShowRoleFunction.java 6b61921 
  shell/src/main/java/org/apache/sqoop/shell/ShowSubmissionFunction.java 
8989913 
  shell/src/main/java/org/apache/sqoop/shell/utils/TableDisplayer.java 51030d0 

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


Testing
---


Thanks,

Jarek Cecho

Review Request 44410: SQOOP-2870: Sqoop2: RESTiliency: Add tests for DriverHandler

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2870
https://issues.apache.org/jira/browse/SQOOP-2870


Repository: sqoop-sqoop2


Description
---

I would like to addd small test case covering {{DriverHandler}} module 
similarly as we're building for other REST endpoints.


Diffs
-

  server/src/main/java/org/apache/sqoop/handler/DriverRequestHandler.java 
95a3291 
  
test/src/test/java/org/apache/sqoop/integration/server/rest/DriverRestTest.java 
PRE-CREATION 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 44097: SQOOP-2853: Sqoop2: Refactor TableDisplayer to be used in document generation

[Jarek Cecho]

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

(Updated March 4, 2016, 10:45 p.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2853
https://issues.apache.org/jira/browse/SQOOP-2853


Repository: sqoop-sqoop2


Description
---

I would like to refactor the existing 
{{[TableDisplayer|https://github.com/apache/sqoop/blob/sqoop2/shell/src/main/java/org/apache/sqoop/shell/utils/TableDisplayer.java]}},
 so that it can generate tables for documentation as well. Right now that class 
is too tight with the way shell works.


Diffs (updated)
-

  shell/src/main/java/org/apache/sqoop/shell/ShellEnvironment.java 36d0712 
  shell/src/main/java/org/apache/sqoop/shell/ShowConnectorFunction.java d973499 
  shell/src/main/java/org/apache/sqoop/shell/ShowJobFunction.java ebbfe82 
  shell/src/main/java/org/apache/sqoop/shell/ShowLinkFunction.java 25bd1db 
  shell/src/main/java/org/apache/sqoop/shell/ShowPrincipalFunction.java a450aaf 
  shell/src/main/java/org/apache/sqoop/shell/ShowPrivilegeFunction.java 2cf6972 
  shell/src/main/java/org/apache/sqoop/shell/ShowRoleFunction.java 6b61921 
  shell/src/main/java/org/apache/sqoop/shell/ShowSubmissionFunction.java 
8989913 
  shell/src/main/java/org/apache/sqoop/shell/utils/TableDisplayer.java 51030d0 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 43783: SQOOP-2848: Sqoop2: RESTiliency: Simplify JobRequestHandler.getJobs similarly as was done for getLinks

[Jarek Cecho]

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

(Updated March 4, 2016, 10:42 p.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2848
https://issues.apache.org/jira/browse/SQOOP-2848


Repository: sqoop-sqoop2


Description
---

I would like to simplify the {{JobRequestHandler.getJobs}} similarly as we've 
changed for {{LinkRequestHandler.getLinks}} back in SQOOP-2670.


Diffs (updated)
-

  server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java 3070059 
  test/src/test/java/org/apache/sqoop/integration/server/rest/JobRestTest.java 
PRE-CREATION 
  test/src/test/java/org/apache/sqoop/integration/server/rest/RestTest.java 
5b3a7bf 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 43783: SQOOP-2848: Sqoop2: RESTiliency: Simplify JobRequestHandler.getJobs similarly as was done for getLinks

[Jarek Cecho]

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

(Updated March 4, 2016, 10:39 p.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2848
https://issues.apache.org/jira/browse/SQOOP-2848


Repository: sqoop-sqoop2


Description
---

I would like to simplify the {{JobRequestHandler.getJobs}} similarly as we've 
changed for {{LinkRequestHandler.getLinks}} back in SQOOP-2670.


Diffs (updated)
-

  server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java 3070059 
  test/src/test/java/org/apache/sqoop/integration/server/rest/RestTest.java 
5b3a7bf 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 43783: SQOOP-2848: Sqoop2: RESTiliency: Simplify JobRequestHandler.getJobs similarly as was done for getLinks

[Jarek Cecho]

> On March 1, 2016, 8:44 p.m., Abraham Fine wrote:
> > server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java, line 
> > 276
> > <https://reviews.apache.org/r/43783/diff/2/?file=1263214#file1263214line276>
> >
> > perhaps this "all" should be stored in a constant somewhere as it is a 
> > special case?
> > 
> > also, slightly unrelated, do you know if we have a mechanism to prevent 
> > the creation of connectors or links named "all"?

Excellent points

1) I should do the change everywhere, so I'll cover it in separate JIRA: 
https://issues.apache.org/jira/browse/SQOOP-2867
2) Filled https://issues.apache.org/jira/browse/SQOOP-2868 to cover it.


> On March 1, 2016, 8:44 p.m., Abraham Fine wrote:
> > test/src/test/java/org/apache/sqoop/integration/server/rest/JobRestTest.java,
> >  line 37
> > <https://reviews.apache.org/r/43783/diff/2/?file=1263215#file1263215line37>
> >
> > nit: could we avoid duplicating this validator code?

Will incorporate to next version.


- Jarek


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


On Feb. 19, 2016, 9:17 p.m., Jarek Cecho wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43783/
> ---
> 
> (Updated Feb. 19, 2016, 9:17 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2848
> https://issues.apache.org/jira/browse/SQOOP-2848
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> I would like to simplify the {{JobRequestHandler.getJobs}} similarly as we've 
> changed for {{LinkRequestHandler.getLinks}} back in SQOOP-2670.
> 
> 
> Diffs
> -
> 
>   server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java 
> 3070059 
>   
> test/src/test/java/org/apache/sqoop/integration/server/rest/JobRestTest.java 
> PRE-CREATION 
>   test/src/test/java/org/apache/sqoop/integration/server/rest/RestTest.java 
> 5b3a7bf 
> 
> Diff: https://reviews.apache.org/r/43783/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Review Request 44404: SQOOP-2866: Sqoop2: Add Abe Fine to committer list in our pom file

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2866
https://issues.apache.org/jira/browse/SQOOP-2866


Repository: sqoop-sqoop2


Description
---

Now when [~abrahamfine] is committer we should update our committer list in the 
root pom.xml file:

https://github.com/apache/sqoop/blob/sqoop2/pom.xml#L903


Diffs
-

  pom.xml 18b2711 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 43857: SQOOP-2545: Sqoop2: RESTiliency: Provide tests for non-existing end points

[Jarek Cecho]

> On March 1, 2016, 8:07 p.m., Abraham Fine wrote:
> > server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java, 
> > line 144
> > <https://reviews.apache.org/r/43857/diff/2/?file=1268499#file1268499line144>
> >
> > perhaps there is a better way of handling this. i can imagine multiple 
> > different servererrors mapping to different http status codes.
> > 
> > I would understand if you felt that this would be best suited for a 
> > different jira.

Yup, I would take that discussion to a separate JIRA - feel free to create one 
with your thoughts :)


- Jarek


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


On Feb. 24, 2016, 11:26 p.m., Jarek Cecho wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43857/
> ---
> 
> (Updated Feb. 24, 2016, 11:26 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2545
> https://issues.apache.org/jira/browse/SQOOP-2545
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> I would like to see () what happens when I call some random non-existing 
> REST URLs to our server.
> 
> 
> Diffs
> -
> 
>   server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java 
> b894e37 
>   server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java 
> fb4a99f 
>   
> test/src/test/java/org/apache/sqoop/integration/server/rest/ConnectorRestTest.java
>  839a4d0 
>   
> test/src/test/java/org/apache/sqoop/integration/server/rest/NonExistingRestTest.java
>  PRE-CREATION 
>   
> test/src/test/java/org/apache/sqoop/integration/server/rest/VersionRestTest.java
>  e4c89df 
> 
> Diff: https://reviews.apache.org/r/43857/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Re: Review Request 43857: SQOOP-2545: Sqoop2: RESTiliency: Provide tests for non-existing end points

[Jarek Cecho]

> On March 4, 2016, 12:29 a.m., Jarek Cecho wrote:
> > server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java, 
> > line 144
> > <https://reviews.apache.org/r/43857/diff/2/?file=1268499#file1268499line144>
> >
> > Yup, I would take that discussion to a separate JIRA - feel free to 
> > create one with your thoughts :)

Ignore this comment please.


- Jarek


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


On Feb. 24, 2016, 11:26 p.m., Jarek Cecho wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43857/
> ---
> 
> (Updated Feb. 24, 2016, 11:26 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2545
> https://issues.apache.org/jira/browse/SQOOP-2545
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> I would like to see () what happens when I call some random non-existing 
> REST URLs to our server.
> 
> 
> Diffs
> -
> 
>   server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java 
> b894e37 
>   server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java 
> fb4a99f 
>   
> test/src/test/java/org/apache/sqoop/integration/server/rest/ConnectorRestTest.java
>  839a4d0 
>   
> test/src/test/java/org/apache/sqoop/integration/server/rest/NonExistingRestTest.java
>  PRE-CREATION 
>   
> test/src/test/java/org/apache/sqoop/integration/server/rest/VersionRestTest.java
>  e4c89df 
> 
> Diff: https://reviews.apache.org/r/43857/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Re: Review Request 43857: SQOOP-2545: Sqoop2: RESTiliency: Provide tests for non-existing end points

[Jarek Cecho]

> On March 1, 2016, 8:07 p.m., Abraham Fine wrote:
> > server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java,
> >  line 48
> > <https://reviews.apache.org/r/43857/diff/2/?file=1268498#file1268498line48>
> >
> > why is this change only being made for the submissionrequesthandler?
> > 
> > don't we have something similar for the driverrequesthandler?

I'm trying to keep these patches rather small. I've did this change in 
Connector handler before and I will do them in DriverRequestHandler when I get 
to it. E.g. It's definitely in the scope of umbrella JIRA that I have.


- Jarek


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


On Feb. 24, 2016, 11:26 p.m., Jarek Cecho wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43857/
> ---
> 
> (Updated Feb. 24, 2016, 11:26 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2545
> https://issues.apache.org/jira/browse/SQOOP-2545
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> I would like to see () what happens when I call some random non-existing 
> REST URLs to our server.
> 
> 
> Diffs
> -
> 
>   server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java 
> b894e37 
>   server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java 
> fb4a99f 
>   
> test/src/test/java/org/apache/sqoop/integration/server/rest/ConnectorRestTest.java
>  839a4d0 
>   
> test/src/test/java/org/apache/sqoop/integration/server/rest/NonExistingRestTest.java
>  PRE-CREATION 
>   
> test/src/test/java/org/apache/sqoop/integration/server/rest/VersionRestTest.java
>  e4c89df 
> 
> Diff: https://reviews.apache.org/r/43857/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Review Request 44369: SQOOP-2865: Sqoop2: Increase test timeout from one hour

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2865
https://issues.apache.org/jira/browse/SQOOP-2865


Repository: sqoop-sqoop2


Description
---

Our integration tests are currently running with [one hour 
timeout|https://github.com/apache/sqoop/blob/sqoop2/pom.xml#L796]. That seems 
fine on my laptop, but Apache build slaves seems to be slower and the build is 
quite frequently timing out. I don't see a need to limit the number of tests as 
I would much rather have better quality then faster tests, so I would like to 
propose to increase the timeout.


Diffs
-

  pom.xml 87fa94f 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 43972: SQOOP-2845

[Jarek Cecho]

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


Ship it!




I'm overall +1, let's just see what's wrong with the precommit hook.

- Jarek Cecho


On Feb. 29, 2016, 9:36 p.m., Abraham Fine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43972/
> ---
> 
> (Updated Feb. 29, 2016, 9:36 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2845
> https://issues.apache.org/jira/browse/SQOOP-2845
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Sqoop2: Derive keystore password from a script passed to configuration
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/sqoop/security/SecurityConstants.java 
> 91a1b8bb0b4a266381faccd11617bba236c6b446 
>   server/src/main/java/org/apache/sqoop/server/SqoopJettyServer.java 
> 60368afc0af8b899696476607a5c0bc6d6084794 
>   server/src/main/java/org/apache/sqoop/server/common/ServerError.java 
> a644e9f10ba792a9c36d5b90db1b5d7f29569e43 
>   test/src/main/java/org/apache/sqoop/test/infrastructure/SqoopTestCase.java 
> e3fbe315e2b23735e2e31321b038dd88c5f99021 
>   
> test/src/test/java/org/apache/sqoop/integration/serverproperties/SslTest.java 
> 17503f3f24ce8344e96e6f7c60085f015eac38f7 
> 
> Diff: https://reviews.apache.org/r/43972/diff/
> 
> 
> Testing
> ---
> 
> yes
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>

Review Request 44303: SQOOP-2863 Properly escape column names for generated INSERT statements

[Jarek Cecho]

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

Review request for Sqoop.


Bugs: SQOOP-2863
https://issues.apache.org/jira/browse/SQOOP-2863


Repository: sqoop-trunk


Description
---

The patch looks higher then necessary as I've decided to refactore the export 
tests to properly escape column names to verify that everything works as it 
should - it wasn't strictly necessary, but it's better outcome overall.

There is one thing that I want to point out explicitly - this change is not 
fully backward compatible. For cases when users are using combination of export 
and --column argument - if they were depending on DB's default behavior of 
auto-lower/upper-casting, then this will break and they will have to manually 
edit the --column argument to contain names as they are persisted in the 
database catalog. I feel that this is reasonable thing to do as their arguments 
are essentially wrong, but I'm wondering what others think?


Diffs
-

  src/java/org/apache/sqoop/manager/ConnManager.java f98feb3 
  src/java/org/apache/sqoop/mapreduce/JdbcCallExportJob.java 2459698 
  src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 78df33c 
  src/java/org/apache/sqoop/mapreduce/JdbcUpdateExportJob.java 8fa420e 
  src/java/org/apache/sqoop/mapreduce/JdbcUpsertExportJob.java 0a9bf7f 
  
src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java 
117cc3f 
  src/test/com/cloudera/sqoop/TestAvroExport.java 137a6e1 
  src/test/com/cloudera/sqoop/TestExport.java 0b650af 
  src/test/com/cloudera/sqoop/TestParquetExport.java 86b40fb 
  src/test/com/cloudera/sqoop/testutil/ExportJobTestCase.java 9a6e8da 
  src/test/org/apache/sqoop/TestExportUsingProcedure.java 98ebf3c 

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


Testing
---

Unit tests passed and I've also verified few scenarios on real cluster against 
MySQL.


Thanks,

Jarek Cecho

Review Request 44302: SQOOP-2864 ClassWriter chokes on column names containing double quotes

[Jarek Cecho]

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

Review request for Sqoop.


Bugs: SQOOP-2864
https://issues.apache.org/jira/browse/SQOOP-2864


Repository: sqoop-trunk


Description
---

I've added code that is specifically handling double quotes. I'm anticipating 
that we might need to extend it in the future with more prolematic characters 
as we find out what else are popular RDBMS allowing.


Diffs
-

  src/java/org/apache/sqoop/orm/ClassWriter.java 95c655d 

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


Testing
---

Creating automated test for this one is super problematical because the 
build-in HSQDB that we're using for testing doesn't support double quotes in 
column names. I did however tested the code on real cluster against MySQL.


Thanks,

Jarek Cecho

Review Request 44212: SQOOP-2858 Sqoop export with Avro data using (--update-key and --update-mode allowinsert) fails

[Jarek Cecho]

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

Review request for Sqoop.


Bugs: SQOOP-2858
https://issues.apache.org/jira/browse/SQOOP-2858


Repository: sqoop-trunk


Description
---

I've added the missing code from JdbcExportJob to JdbcUpdateExportJob. There is 
huge opportunity to refactore those two classes to reuse code much better, but 
I did not wanted to do huge reactorings to that code, I've chose path of 
copy mostly.


Diffs
-

  src/java/org/apache/sqoop/mapreduce/ExportJobBase.java 32de92a 
  src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 78df33c 
  src/java/org/apache/sqoop/mapreduce/JdbcUpdateExportJob.java 8fa420e 
  src/test/com/cloudera/sqoop/TestAvroExport.java 137a6e1 
  src/test/com/cloudera/sqoop/TestParquetExport.java 86b40fb 

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


Testing
---

I've added new unit tests that are covering update mode for both Parquet and 
Avro.


Thanks,

Jarek Cecho

Review Request 44105: SQOOP-2856: Sqoop2: Enrich HDFS Connector resource file

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2856
https://issues.apache.org/jira/browse/SQOOP-2856


Repository: sqoop-sqoop2


Description
---

Please see parent JIRA for more details about what


Diffs
-

  connector/connector-hdfs/src/main/resources/hdfs-connector-config.properties 
69f50c1 

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


Testing
---


Thanks,

Jarek Cecho

Review Request 44097: SQOOP-2853: Sqoop2: Refactor TableDisplayer to be used in document generation

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2853
https://issues.apache.org/jira/browse/SQOOP-2853


Repository: sqoop-sqoop2


Description
---

I would like to refactor the existing 
{{[TableDisplayer|https://github.com/apache/sqoop/blob/sqoop2/shell/src/main/java/org/apache/sqoop/shell/utils/TableDisplayer.java]}},
 so that it can generate tables for documentation as well. Right now that class 
is too tight with the way shell works.


Diffs
-

  shell/src/main/java/org/apache/sqoop/shell/ShellEnvironment.java 36d0712 
  shell/src/main/java/org/apache/sqoop/shell/ShowConnectorFunction.java d973499 
  shell/src/main/java/org/apache/sqoop/shell/ShowJobFunction.java ebbfe82 
  shell/src/main/java/org/apache/sqoop/shell/ShowLinkFunction.java 25bd1db 
  shell/src/main/java/org/apache/sqoop/shell/ShowPrincipalFunction.java a450aaf 
  shell/src/main/java/org/apache/sqoop/shell/ShowPrivilegeFunction.java 2cf6972 
  shell/src/main/java/org/apache/sqoop/shell/ShowRoleFunction.java 6b61921 
  shell/src/main/java/org/apache/sqoop/shell/ShowSubmissionFunction.java 
8989913 
  shell/src/main/java/org/apache/sqoop/shell/utils/TableDisplayer.java 51030d0 
  shell/src/test/java/org/apache/sqoop/shell/TestShowCommand.java 49affa3 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 43711: Sqoop2: Enable SSL/TLS for API calls

[Jarek Cecho]

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


Ship it!




Ship It!

- Jarek Cecho


On Feb. 25, 2016, 12:04 a.m., Abraham Fine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43711/
> ---
> 
> (Updated Feb. 25, 2016, 12:04 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2843
> https://issues.apache.org/jira/browse/SQOOP-2843
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Sqoop2: Enable SSL/TLS for API calls
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/sqoop/security/SecurityConstants.java 
> 6f32e04be6fc47051e7c0cc90a881cb872cb3934 
>   dist/src/main/conf/sqoop.properties 
> 620146d5c8a2f736d5d139e6452b0165cbdfaa17 
>   server/src/main/java/org/apache/sqoop/server/SqoopJettyServer.java 
> 2c4cb7a13aacd1e4e3222d54fb3b1ef518aa2ebb 
>   server/src/main/java/org/apache/sqoop/server/common/ServerError.java 
> 1b021cf27defee1493ad620466a89b9fae1b5bb5 
>   test/pom.xml 1e88b34a684709e7a0ad8a71480e7bad37a94398 
>   test/src/main/java/org/apache/sqoop/test/kdc/MiniKdcRunner.java 
> 83f960b87987d1c1ed8f0161616e631f63f14543 
>   test/src/main/java/org/apache/sqoop/test/utils/SecurityUtils.java 
> PRE-CREATION 
>   test/src/main/java/org/apache/sqoop/test/utils/SqoopUtils.java 
> 6614b19932c304aa40500fcaedd974b1b136caec 
>   
> test/src/test/java/org/apache/sqoop/integration/connectorloading/BlacklistedConnectorTest.java
>  7f0575b49c58fcec1c69069f25929411a6c7ba12 
>   
> test/src/test/java/org/apache/sqoop/integration/connectorloading/ClasspathTest.java
>  87f0eb1966de4d7ea9b1c37b881cad0ae7a88fa4 
>   
> test/src/test/java/org/apache/sqoop/integration/connectorloading/ConnectorClasspathIsolationTest.java
>  a82a4daea607bee4e78bf2a649b7c9d5228448be 
>   
> test/src/test/java/org/apache/sqoop/integration/serverproperties/SslTest.java 
> PRE-CREATION 
>   test/src/test/resources/connector-loading-tests-suite.xml 
> c03fb4f008ec75a65efbd6ad6c4eb33d37099124 
> 
> Diff: https://reviews.apache.org/r/43711/diff/
> 
> 
> Testing
> ---
> 
> passing integration tests
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>

Re: Review Request 43857: SQOOP-2545: Sqoop2: RESTiliency: Provide tests for non-existing end points

[Jarek Cecho]

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

(Updated Feb. 24, 2016, 11:26 p.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2545
https://issues.apache.org/jira/browse/SQOOP-2545


Repository: sqoop-sqoop2


Description
---

I would like to see () what happens when I call some random non-existing 
REST URLs to our server.


Diffs (updated)
-

  server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java 
b894e37 
  server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java 
fb4a99f 
  
test/src/test/java/org/apache/sqoop/integration/server/rest/ConnectorRestTest.java
 839a4d0 
  
test/src/test/java/org/apache/sqoop/integration/server/rest/NonExistingRestTest.java
 PRE-CREATION 
  
test/src/test/java/org/apache/sqoop/integration/server/rest/VersionRestTest.java
 e4c89df 

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


Testing
---


Thanks,

Jarek Cecho

Review Request 43857: SQOOP-2545: Sqoop2: RESTiliency: Provide tests for non-existing end points

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2545
https://issues.apache.org/jira/browse/SQOOP-2545


Repository: sqoop-sqoop2


Description
---

I would like to see () what happens when I call some random non-existing 
REST URLs to our server.


Diffs
-

  server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java 
b894e37 
  server/src/main/java/org/apache/sqoop/server/SqoopProtocolServlet.java 
fb4a99f 
  
test/src/test/java/org/apache/sqoop/integration/server/rest/NonExistingRestTest.java
 PRE-CREATION 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 43783: SQOOP-2848: Sqoop2: RESTiliency: Simplify JobRequestHandler.getJobs similarly as was done for getLinks

[Jarek Cecho]

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

(Updated Feb. 19, 2016, 9:17 p.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2848
https://issues.apache.org/jira/browse/SQOOP-2848


Repository: sqoop-sqoop2


Description
---

I would like to simplify the {{JobRequestHandler.getJobs}} similarly as we've 
changed for {{LinkRequestHandler.getLinks}} back in SQOOP-2670.


Diffs (updated)
-

  server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java 3070059 
  test/src/test/java/org/apache/sqoop/integration/server/rest/JobRestTest.java 
PRE-CREATION 
  test/src/test/java/org/apache/sqoop/integration/server/rest/RestTest.java 
5b3a7bf 

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


Testing
---


Thanks,

Jarek Cecho

Review Request 43783: SQOOP-2848: Sqoop2: RESTiliency: Simplify JobRequestHandler.getJobs similarly as was done for getLinks

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2848
https://issues.apache.org/jira/browse/SQOOP-2848


Repository: sqoop-sqoop2


Description
---

I would like to simplify the {{JobRequestHandler.getJobs}} similarly as we've 
changed for {{LinkRequestHandler.getLinks}} back in SQOOP-2670.


Diffs
-

  server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java 3070059 
  test/pom.xml 134bca1 
  test/src/main/java/org/apache/sqoop/test/testng/ReconfigureLogListener.java 
PRE-CREATION 
  test/src/test/java/org/apache/sqoop/integration/server/rest/JobRestTest.java 
PRE-CREATION 
  test/src/test/java/org/apache/sqoop/integration/server/rest/RestTest.java 
5b3a7bf 
  test/src/test/resources/connector-loading-tests-suite.xml 02c1df3 
  test/src/test/resources/hive-tests-suite.xml 3e2b328 
  test/src/test/resources/integration-tests-suite.xml 73e0a77 
  test/src/test/resources/new-integration-tests-suite.xml 96a1320 
  test/src/test/resources/shell-tests-suite.xml e2c0b09 
  test/src/test/resources/tools-tests-suite.xml fee2121 
  test/src/test/resources/upgrade-tests-suite.xml 3318e67 

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


Testing
---


Thanks,

Jarek Cecho

Review Request 43780: SQOOP-2847 Sqoop --incremental + missing parent --target-dir reports success with no data

[Jarek Cecho]

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

Review request for Sqoop.


Bugs: SQOOP-2847
https://issues.apache.org/jira/browse/SQOOP-2847


Repository: sqoop-trunk


Description
---

The problem happened because we have fs.rename() call without checking return 
value. I've added checks for return value and also  added code that generates 
parent directories if they do not exists already.


Diffs
-

  src/java/org/apache/sqoop/tool/ImportTool.java 39af42c 
  src/test/com/cloudera/sqoop/TestIncrementalImport.java d28a187 

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


Testing
---

Added unit test and verified on real cluster.


Thanks,

Jarek Cecho

Re: Review Request 43711: Sqoop2: Enable SSL/TLS for API calls

[Jarek Cecho]

> On Feb. 19, 2016, 3:41 p.m., Jarek Cecho wrote:
> > test/src/test/resources/connector-loading-tests-suite.xml, lines 21-31
> > <https://reviews.apache.org/r/43711/diff/4/?file=1254900#file1254900line21>
> >
> > Out of curiosity: Why this module change?
> 
> Abraham Fine wrote:
> the ssltest needs to create its own instance of the sqoop server. by 
> leaving the test inside the "server" test suite, I run into issues due to the 
> suite shared instance of sqoop2 server that is running to support those tests 
> (from the sqoopinfrastructureprovider). The ConnectorLoadingTests suite is 
> different than our other suites in that it does not have a shared instance of 
> sqoop server (all of the connectorloadingtests involve launch the server with 
> different properites). 
> 
> Since we do something similar to the ConnectorLoadingTests in the ssltest 
> I decided to move the ssltest to that test suite. and since the ssltest has 
> nothing to do with connectorloading i thought it would be appropriate to 
> rename the suite to reflect what all of the tests had in common.

ACK make sense.


- Jarek


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


On Feb. 18, 2016, 6:45 p.m., Abraham Fine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43711/
> ---
> 
> (Updated Feb. 18, 2016, 6:45 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2843
> https://issues.apache.org/jira/browse/SQOOP-2843
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Sqoop2: Enable SSL/TLS for API calls
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/sqoop/security/SecurityConstants.java 
> 6f32e04be6fc47051e7c0cc90a881cb872cb3934 
>   dist/src/main/conf/sqoop.properties 
> 620146d5c8a2f736d5d139e6452b0165cbdfaa17 
>   server/src/main/java/org/apache/sqoop/server/SqoopJettyServer.java 
> 2c4cb7a13aacd1e4e3222d54fb3b1ef518aa2ebb 
>   server/src/main/java/org/apache/sqoop/server/common/ServerError.java 
> 1b021cf27defee1493ad620466a89b9fae1b5bb5 
>   test/src/main/java/org/apache/sqoop/test/kdc/MiniKdcRunner.java 
> 83f960b87987d1c1ed8f0161616e631f63f14543 
>   test/src/main/java/org/apache/sqoop/test/utils/SecurityUtils.java 
> PRE-CREATION 
>   test/src/main/java/org/apache/sqoop/test/utils/SqoopUtils.java 
> 6614b19932c304aa40500fcaedd974b1b136caec 
>   
> test/src/test/java/org/apache/sqoop/integration/connectorloading/BlacklistedConnectorTest.java
>  7f0575b49c58fcec1c69069f25929411a6c7ba12 
>   
> test/src/test/java/org/apache/sqoop/integration/connectorloading/ClasspathTest.java
>  87f0eb1966de4d7ea9b1c37b881cad0ae7a88fa4 
>   
> test/src/test/java/org/apache/sqoop/integration/connectorloading/ConnectorClasspathIsolationTest.java
>  a82a4daea607bee4e78bf2a649b7c9d5228448be 
>   
> test/src/test/java/org/apache/sqoop/integration/serverproperties/SslTest.java 
> PRE-CREATION 
>   test/src/test/resources/connector-loading-tests-suite.xml 
> 02c1df3aa7bc6f4fa2ddf94ce436bce5d07ceb72 
> 
> Diff: https://reviews.apache.org/r/43711/diff/
> 
> 
> Testing
> ---
> 
> passing integration tests
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>

Re: Review Request 43711: Sqoop2: Enable SSL/TLS for API calls

[Jarek Cecho]

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




core/src/main/java/org/apache/sqoop/security/SecurityConstants.java (lines 192 
- 201)
<https://reviews.apache.org/r/43711/#comment181282>

Nit: It seems that we either use dot to separate names or underscores 
rather then just removing spaces. Perhaps we should stay consistent?



test/src/test/resources/connector-loading-tests-suite.xml (lines 21 - 31)
<https://reviews.apache.org/r/43711/#comment181283>

Out of curiosity: Why this module change?


- Jarek Cecho


On Feb. 18, 2016, 6:45 p.m., Abraham Fine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43711/
> ---
> 
> (Updated Feb. 18, 2016, 6:45 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2843
> https://issues.apache.org/jira/browse/SQOOP-2843
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Sqoop2: Enable SSL/TLS for API calls
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/sqoop/security/SecurityConstants.java 
> 6f32e04be6fc47051e7c0cc90a881cb872cb3934 
>   dist/src/main/conf/sqoop.properties 
> 620146d5c8a2f736d5d139e6452b0165cbdfaa17 
>   server/src/main/java/org/apache/sqoop/server/SqoopJettyServer.java 
> 2c4cb7a13aacd1e4e3222d54fb3b1ef518aa2ebb 
>   server/src/main/java/org/apache/sqoop/server/common/ServerError.java 
> 1b021cf27defee1493ad620466a89b9fae1b5bb5 
>   test/src/main/java/org/apache/sqoop/test/kdc/MiniKdcRunner.java 
> 83f960b87987d1c1ed8f0161616e631f63f14543 
>   test/src/main/java/org/apache/sqoop/test/utils/SecurityUtils.java 
> PRE-CREATION 
>   test/src/main/java/org/apache/sqoop/test/utils/SqoopUtils.java 
> 6614b19932c304aa40500fcaedd974b1b136caec 
>   
> test/src/test/java/org/apache/sqoop/integration/connectorloading/BlacklistedConnectorTest.java
>  7f0575b49c58fcec1c69069f25929411a6c7ba12 
>   
> test/src/test/java/org/apache/sqoop/integration/connectorloading/ClasspathTest.java
>  87f0eb1966de4d7ea9b1c37b881cad0ae7a88fa4 
>   
> test/src/test/java/org/apache/sqoop/integration/connectorloading/ConnectorClasspathIsolationTest.java
>  a82a4daea607bee4e78bf2a649b7c9d5228448be 
>   
> test/src/test/java/org/apache/sqoop/integration/serverproperties/SslTest.java 
> PRE-CREATION 
>   test/src/test/resources/connector-loading-tests-suite.xml 
> 02c1df3aa7bc6f4fa2ddf94ce436bce5d07ceb72 
> 
> Diff: https://reviews.apache.org/r/43711/diff/
> 
> 
> Testing
> ---
> 
> passing integration tests
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>

Re: Review Request 43467: SQOOP-2832: Sqoop2: Precommit: Create log files for individual tests

[Jarek Cecho]

> On Feb. 18, 2016, 1:53 a.m., Colin Ma wrote:
> > test/src/main/java/org/apache/sqoop/test/testng/ReconfigureLogListener.java,
> >  line 99
> > <https://reviews.apache.org/r/43467/diff/3/?file=1252713#file1252713line99>
> >
> > Write a static field in non-static method cause the findbugs warning.
> > How about use System.currentTimeMillis() as the prefix?

I prefer the counter as this way we can keep the size of the number relatively 
small, whereas with timestimap it would have to be long number. I've added 
exception for ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD to the listener, so that 
findbugs doesn't worry about it anymore.


- Jarek


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


On Feb. 19, 2016, 3:32 p.m., Jarek Cecho wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43467/
> ---
> 
> (Updated Feb. 19, 2016, 3:32 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2832
> https://issues.apache.org/jira/browse/SQOOP-2832
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> I feel that debugging anything in our pre-commit hook is currently mission 
> impossible from several reasons. One of the main ones is that we're 
> generating one single log file that has [grown to 1GB in 
> size|https://builds.apache.org/job/PreCommit-SQOOP-Build/2185/artifact/test/target/surefire-reports/]
>  with all "test related" logs and I'm not even remotely able to find anything 
> there.
> 
> In normal case we would have one log per test class, but as we've refactored 
> our integration tests to run multiple classes inside single Suite, we're no 
> longer have that for free. We have done this change to cut the time it takes 
> to run the integration tests - before we were initializing mini clusters for 
> each class, which is extra ~40 seconds per class, so I don't think that 
> reverting it would be reasonable.
> 
> We should perhaps explore other ways how to get multiple log files.
> 
> 
> Diffs
> -
> 
>   test/pom.xml 134bca1 
>   test/src/main/java/org/apache/sqoop/test/testng/ReconfigureLogListener.java 
> PRE-CREATION 
>   test/src/test/resources/connector-loading-tests-suite.xml 02c1df3 
>   test/src/test/resources/hive-tests-suite.xml 3e2b328 
>   test/src/test/resources/integration-tests-suite.xml 73e0a77 
>   test/src/test/resources/new-integration-tests-suite.xml 96a1320 
>   test/src/test/resources/shell-tests-suite.xml e2c0b09 
>   test/src/test/resources/tools-tests-suite.xml fee2121 
>   test/src/test/resources/upgrade-tests-suite.xml 3318e67 
> 
> Diff: https://reviews.apache.org/r/43467/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Re: Review Request 43467: SQOOP-2832: Sqoop2: Precommit: Create log files for individual tests

[Jarek Cecho]

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

(Updated Feb. 19, 2016, 3:32 p.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2832
https://issues.apache.org/jira/browse/SQOOP-2832


Repository: sqoop-sqoop2


Description
---

I feel that debugging anything in our pre-commit hook is currently mission 
impossible from several reasons. One of the main ones is that we're generating 
one single log file that has [grown to 1GB in 
size|https://builds.apache.org/job/PreCommit-SQOOP-Build/2185/artifact/test/target/surefire-reports/]
 with all "test related" logs and I'm not even remotely able to find anything 
there.

In normal case we would have one log per test class, but as we've refactored 
our integration tests to run multiple classes inside single Suite, we're no 
longer have that for free. We have done this change to cut the time it takes to 
run the integration tests - before we were initializing mini clusters for each 
class, which is extra ~40 seconds per class, so I don't think that reverting it 
would be reasonable.

We should perhaps explore other ways how to get multiple log files.


Diffs (updated)
-

  test/pom.xml 134bca1 
  test/src/main/java/org/apache/sqoop/test/testng/ReconfigureLogListener.java 
PRE-CREATION 
  test/src/test/resources/connector-loading-tests-suite.xml 02c1df3 
  test/src/test/resources/hive-tests-suite.xml 3e2b328 
  test/src/test/resources/integration-tests-suite.xml 73e0a77 
  test/src/test/resources/new-integration-tests-suite.xml 96a1320 
  test/src/test/resources/shell-tests-suite.xml e2c0b09 
  test/src/test/resources/tools-tests-suite.xml fee2121 
  test/src/test/resources/upgrade-tests-suite.xml 3318e67 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 43467: SQOOP-2832: Sqoop2: Precommit: Create log files for individual tests

[Jarek Cecho]

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

(Updated Feb. 17, 2016, 4:12 p.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2832
https://issues.apache.org/jira/browse/SQOOP-2832


Repository: sqoop-sqoop2


Description
---

I feel that debugging anything in our pre-commit hook is currently mission 
impossible from several reasons. One of the main ones is that we're generating 
one single log file that has [grown to 1GB in 
size|https://builds.apache.org/job/PreCommit-SQOOP-Build/2185/artifact/test/target/surefire-reports/]
 with all "test related" logs and I'm not even remotely able to find anything 
there.

In normal case we would have one log per test class, but as we've refactored 
our integration tests to run multiple classes inside single Suite, we're no 
longer have that for free. We have done this change to cut the time it takes to 
run the integration tests - before we were initializing mini clusters for each 
class, which is extra ~40 seconds per class, so I don't think that reverting it 
would be reasonable.

We should perhaps explore other ways how to get multiple log files.


Diffs (updated)
-

  test/pom.xml 134bca1 
  test/src/main/java/org/apache/sqoop/test/testng/ReconfigureLogListener.java 
PRE-CREATION 
  test/src/test/resources/connector-loading-tests-suite.xml 02c1df3 
  test/src/test/resources/hive-tests-suite.xml 3e2b328 
  test/src/test/resources/integration-tests-suite.xml 73e0a77 
  test/src/test/resources/new-integration-tests-suite.xml 96a1320 
  test/src/test/resources/shell-tests-suite.xml e2c0b09 
  test/src/test/resources/tools-tests-suite.xml fee2121 
  test/src/test/resources/upgrade-tests-suite.xml 3318e67 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 43467: SQOOP-2832: Sqoop2: Precommit: Create log files for individual tests

[Jarek Cecho]

> On Feb. 17, 2016, 3:42 a.m., Colin Ma wrote:
> > test/src/test/resources/integration-tests-suite.xml, line 25
> > <https://reviews.apache.org/r/43467/diff/2/?file=1240433#file1240433line25>
> >
> > How about add the listener for all suite like: 
> > connector-loading-tests-suite.xml and new-integration-tests-suite.xml, etc.

Yes, I wanted to get feedback first :) Let me do that real quick.


- Jarek


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


On Feb. 17, 2016, 4:12 p.m., Jarek Cecho wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43467/
> ---
> 
> (Updated Feb. 17, 2016, 4:12 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2832
> https://issues.apache.org/jira/browse/SQOOP-2832
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> I feel that debugging anything in our pre-commit hook is currently mission 
> impossible from several reasons. One of the main ones is that we're 
> generating one single log file that has [grown to 1GB in 
> size|https://builds.apache.org/job/PreCommit-SQOOP-Build/2185/artifact/test/target/surefire-reports/]
>  with all "test related" logs and I'm not even remotely able to find anything 
> there.
> 
> In normal case we would have one log per test class, but as we've refactored 
> our integration tests to run multiple classes inside single Suite, we're no 
> longer have that for free. We have done this change to cut the time it takes 
> to run the integration tests - before we were initializing mini clusters for 
> each class, which is extra ~40 seconds per class, so I don't think that 
> reverting it would be reasonable.
> 
> We should perhaps explore other ways how to get multiple log files.
> 
> 
> Diffs
> -
> 
>   test/pom.xml 134bca1 
>   test/src/main/java/org/apache/sqoop/test/testng/ReconfigureLogListener.java 
> PRE-CREATION 
>   test/src/test/resources/connector-loading-tests-suite.xml 02c1df3 
>   test/src/test/resources/hive-tests-suite.xml 3e2b328 
>   test/src/test/resources/integration-tests-suite.xml 73e0a77 
>   test/src/test/resources/new-integration-tests-suite.xml 96a1320 
>   test/src/test/resources/shell-tests-suite.xml e2c0b09 
>   test/src/test/resources/tools-tests-suite.xml fee2121 
>   test/src/test/resources/upgrade-tests-suite.xml 3318e67 
> 
> Diff: https://reviews.apache.org/r/43467/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Re: Review Request 43315: SQOOP-2827: Sqoop2: Doc: Provide doc preprocessor facilities

[Jarek Cecho]

> On Feb. 8, 2016, 7:22 p.m., Abraham Fine wrote:
> > docs/pom.xml, line 72
> > <https://reviews.apache.org/r/43315/diff/1/?file=1237090#file1237090line72>
> >
> > what is the reason for this change? shouldn't the documentation be 
> > generated during the "site" goal?

So that is a maven playing badly on us. The "site" goal is actually not part of 
standard lifecycle, but an extra one (similarly to clean). So when one does 
"mvn package", the "site" goal is not necessarily called. And since our docs 
are primarily targetted for the being included in package, I've decided to move 
them to this goal to ensure that our binary package always have them. I could 
move the doc generation to "site" goal and update our "How to release guide" to 
call "mvn clean site package", but that seemed quite fragile to me. What do you 
think?


- Jarek


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


On Feb. 7, 2016, 6:10 p.m., Jarek Cecho wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43315/
> ---
> 
> (Updated Feb. 7, 2016, 6:10 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2827
> https://issues.apache.org/jira/browse/SQOOP-2827
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> In order to solve SQOOP-2826 I need facilities to execute pre-processing 
> steps while generating documentation.
> 
> 
> Diffs
> -
> 
>   docs/pom.xml c96a582 
>   docs/src/main/java/org/apache/sqoop/docs/generator/DocPreprocessor.java 
> PRE-CREATION 
>   
> docs/src/main/java/org/apache/sqoop/docs/generator/plugins/AbstractPlugin.java
>  PRE-CREATION 
>   
> docs/src/main/java/org/apache/sqoop/docs/generator/plugins/CopySourceToDestination.java
>  PRE-CREATION 
>   docs/src/main/resources/log4j.properties PRE-CREATION 
>   pom.xml ba0a243 
> 
> Diff: https://reviews.apache.org/r/43315/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Re: Review Request 43315: SQOOP-2827: Sqoop2: Doc: Provide doc preprocessor facilities

[Jarek Cecho]

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

(Updated Feb. 12, 2016, 10:05 p.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2827
https://issues.apache.org/jira/browse/SQOOP-2827


Repository: sqoop-sqoop2


Description
---

In order to solve SQOOP-2826 I need facilities to execute pre-processing steps 
while generating documentation.


Diffs (updated)
-

  docs/pom.xml c96a582 
  docs/src/main/java/org/apache/sqoop/docs/generator/DocPreprocessor.java 
PRE-CREATION 
  
docs/src/main/java/org/apache/sqoop/docs/generator/plugins/AbstractPlugin.java 
PRE-CREATION 
  
docs/src/main/java/org/apache/sqoop/docs/generator/plugins/CopySourceToDestination.java
 PRE-CREATION 
  docs/src/main/resources/log4j.properties PRE-CREATION 
  pom.xml ba0a243 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 43315: SQOOP-2827: Sqoop2: Doc: Provide doc preprocessor facilities

[Jarek Cecho]

> On Feb. 8, 2016, 7:22 p.m., Abraham Fine wrote:
> > docs/pom.xml, line 72
> > <https://reviews.apache.org/r/43315/diff/1/?file=1237090#file1237090line72>
> >
> > what is the reason for this change? shouldn't the documentation be 
> > generated during the "site" goal?
> 
> Jarek Cecho wrote:
> So that is a maven playing badly on us. The "site" goal is actually not 
> part of standard lifecycle, but an extra one (similarly to clean). So when 
> one does "mvn package", the "site" goal is not necessarily called. And since 
> our docs are primarily targetted for the being included in package, I've 
> decided to move them to this goal to ensure that our binary package always 
> have them. I could move the doc generation to "site" goal and update our "How 
> to release guide" to call "mvn clean site package", but that seemed quite 
> fragile to me. What do you think?

We had a small chat with Abe offline where he correctly pointed out that 
without this patch, the sphinx plugin runs executed via "mvn site" as well as 
"mvn package". However with this patch it's executed only for "mvn package".

I've took a deeper look and I think that I do understand what happened now. 
I've added a new plugin exec-maven-plugin that is doing the doc pregeneration. 
Sadly this plugin can't run inside "reporting" tag as the sphinx-maven-plugin 
is, so I put it into "build" tag. But then the sphinx-maven-plugin was executed 
before exec-maven-plugin (which is undesirable), so I've moved 
sphinx-maven-plugin from "reporting" to "build" and suddenly everything was 
working so I was happy.

I did not realized that by moving sphinx plugin from "reporting" tag, it's no 
longer called on "mvn site". I've uploaded new patch that adds execution of 
both exec-maven-plugin and sphinx-maven-plugin to be run inside "site" target 
in addition to "package" target that I put there before. So with the new 
version, the documentation is generate on both "mvn  site" and "mvn package" 
targets.


- Jarek


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


On Feb. 12, 2016, 10:05 p.m., Jarek Cecho wrote:
> 
> -------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43315/
> ---
> 
> (Updated Feb. 12, 2016, 10:05 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2827
> https://issues.apache.org/jira/browse/SQOOP-2827
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> In order to solve SQOOP-2826 I need facilities to execute pre-processing 
> steps while generating documentation.
> 
> 
> Diffs
> -
> 
>   docs/pom.xml c96a582 
>   docs/src/main/java/org/apache/sqoop/docs/generator/DocPreprocessor.java 
> PRE-CREATION 
>   
> docs/src/main/java/org/apache/sqoop/docs/generator/plugins/AbstractPlugin.java
>  PRE-CREATION 
>   
> docs/src/main/java/org/apache/sqoop/docs/generator/plugins/CopySourceToDestination.java
>  PRE-CREATION 
>   docs/src/main/resources/log4j.properties PRE-CREATION 
>   pom.xml ba0a243 
> 
> Diff: https://reviews.apache.org/r/43315/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Review Request 43547: SQOOP-2817: Sqoop2: NullValueTest fails when running against MySQL database

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2817
https://issues.apache.org/jira/browse/SQOOP-2817


Repository: sqoop-sqoop2


Description
---

None


Diffs
-

  docs/pom.xml c96a582 
  docs/src/main/java/org/apache/sqoop/docs/generator/DocPreprocessor.java 
PRE-CREATION 
  
docs/src/main/java/org/apache/sqoop/docs/generator/plugins/AbstractPlugin.java 
PRE-CREATION 
  
docs/src/main/java/org/apache/sqoop/docs/generator/plugins/CopySourceToDestination.java
 PRE-CREATION 
  docs/src/main/resources/log4j.properties PRE-CREATION 
  pom.xml ba0a243 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 43467: SQOOP-2832: Sqoop2: Precommit: Create log files for individual tests

[Jarek Cecho]

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

(Updated Feb. 11, 2016, 9:29 p.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2832
https://issues.apache.org/jira/browse/SQOOP-2832


Repository: sqoop-sqoop2


Description
---

I feel that debugging anything in our pre-commit hook is currently mission 
impossible from several reasons. One of the main ones is that we're generating 
one single log file that has [grown to 1GB in 
size|https://builds.apache.org/job/PreCommit-SQOOP-Build/2185/artifact/test/target/surefire-reports/]
 with all "test related" logs and I'm not even remotely able to find anything 
there.

In normal case we would have one log per test class, but as we've refactored 
our integration tests to run multiple classes inside single Suite, we're no 
longer have that for free. We have done this change to cut the time it takes to 
run the integration tests - before we were initializing mini clusters for each 
class, which is extra ~40 seconds per class, so I don't think that reverting it 
would be reasonable.

We should perhaps explore other ways how to get multiple log files.


Diffs (updated)
-

  test/pom.xml 134bca1 
  test/src/main/java/org/apache/sqoop/test/testng/ReconfigureLogListener.java 
PRE-CREATION 
  test/src/test/resources/integration-tests-suite.xml 73e0a77 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 43348: SQOOP-2828: Sqoop2: AvroIntermediateDataFormat should read Decimals as BigDecimals instead of Strings

[Jarek Cecho]

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


Ship it!




+1 as long as precommit hook will be happy.

- Jarek Cecho


On Feb. 8, 2016, 11:55 p.m., Abraham Fine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43348/
> ---
> 
> (Updated Feb. 8, 2016, 11:55 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2828
> https://issues.apache.org/jira/browse/SQOOP-2828
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> init
> 
> 
> Diffs
> -
> 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/AVROIntermediateDataFormat.java
>  e409fc1227c639cf8e04b7bf064854e9339bb77c 
>   
> connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestAVROIntermediateDataFormat.java
>  847572076ee24f35b4c6a270972fdcccbc8ca596 
> 
> Diff: https://reviews.apache.org/r/43348/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>

Review Request 43467: SQOOP-2832: Sqoop2: Precommit: Create log files for individual tests

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2832
https://issues.apache.org/jira/browse/SQOOP-2832


Repository: sqoop-sqoop2


Description
---

I feel that debugging anything in our pre-commit hook is currently mission 
impossible from several reasons. One of the main ones is that we're generating 
one single log file that has [grown to 1GB in 
size|https://builds.apache.org/job/PreCommit-SQOOP-Build/2185/artifact/test/target/surefire-reports/]
 with all "test related" logs and I'm not even remotely able to find anything 
there.

In normal case we would have one log per test class, but as we've refactored 
our integration tests to run multiple classes inside single Suite, we're no 
longer have that for free. We have done this change to cut the time it takes to 
run the integration tests - before we were initializing mini clusters for each 
class, which is extra ~40 seconds per class, so I don't think that reverting it 
would be reasonable.

We should perhaps explore other ways how to get multiple log files.


Diffs
-

  test/pom.xml 134bca1 
  test/src/main/java/org/apache/sqoop/test/testng/ReconfigureLogListener.java 
PRE-CREATION 
  test/src/test/resources/integration-tests-suite.xml 73e0a77 

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


Testing
---


Thanks,

Jarek Cecho

Review Request 43315: SQOOP-2827: Sqoop2: Doc: Provide doc preprocessor facilities

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2827
https://issues.apache.org/jira/browse/SQOOP-2827


Repository: sqoop-sqoop2


Description
---

In order to solve SQOOP-2826 I need facilities to execute pre-processing steps 
while generating documentation.


Diffs
-

  docs/pom.xml c96a582 
  docs/src/main/java/org/apache/sqoop/docs/generator/DocPreprocessor.java 
PRE-CREATION 
  
docs/src/main/java/org/apache/sqoop/docs/generator/plugins/AbstractPlugin.java 
PRE-CREATION 
  
docs/src/main/java/org/apache/sqoop/docs/generator/plugins/CopySourceToDestination.java
 PRE-CREATION 
  docs/src/main/resources/log4j.properties PRE-CREATION 
  pom.xml ba0a243 

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


Testing
---


Thanks,

Jarek Cecho

Review Request 43270: SQOOP-2822: Sqoop2: RESTiliency: Provide tests for Link POST action

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2822
https://issues.apache.org/jira/browse/SQOOP-2822


Repository: sqoop-sqoop2


Description
---

None


Diffs
-

  test/src/test/java/org/apache/sqoop/integration/server/rest/LinkRestTest.java 
c6f4eed 

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


Testing
---


Thanks,

Jarek Cecho

Review Request 43273: SQOOP-2823: Sqoop2: RESTiliency: Remove repetitive try-catch block for accessing POST and PUT request

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2823
https://issues.apache.org/jira/browse/SQOOP-2823


Repository: sqoop-sqoop2


Description
---

I've noticed that we have quite a few repetition of code like this one:

{code}
 RoleBean bean = new RoleBean();

try {
  JSONObject json = JSONUtils.parse(ctx.getRequest().getReader());
  bean.restore(json);
} catch (IOException e) {
  throw new SqoopException(ServerError.SERVER_0003, "Can't read request 
content", e);
}
{code}

It's relatively a lot of code just because {{ctx.getRequest()}} can throw an 
exception. I would like to propose to create a method in our {{ctx}} wrapper to 
provide the {{getReader}} method without any exception, which will simplify the 
code to:

{code}
bean.restore(JSONUtils.parse(ctx.getReader()));
{code}

We'll of course throw an exception on failure - but using our 
{{SqoopException}} mechanism.


Diffs
-

  
server/src/main/java/org/apache/sqoop/handler/AuthorizationRequestHandler.java 
6cd77e9 
  server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java 9dfcb0b 
  server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java 7d7f1de 
  server/src/main/java/org/apache/sqoop/server/RequestContext.java 2beac2b 

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


Testing
---


Thanks,

Jarek Cecho

Review Request 43173: SQOOP-2805: Sqoop2: Detect if both fixVersion and affectedVersions are empty and report it

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2805
https://issues.apache.org/jira/browse/SQOOP-2805


Repository: sqoop-sqoop2


Description
---

In order for the precommit hook to start we require that at least one of 
fixVersion/affectedVersion is filled with Sqoop 2 related version. We should 
improve our upload-patch script to verify that is indeed the case otherwise 
report to user that this needs to be changed.


Diffs
-

  dev-support/upload-patch.py 23bcf07 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 42776: SQOOP-2797: Sqoop2: Datatypes: Add Blob data type support for Derby

[Jarek Cecho]

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


Ship it!




Provided precommmit hook will be happy.

- Jarek Cecho


On Feb. 4, 2016, 3:02 a.m., Colin Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42776/
> ---
> 
> (Updated Feb. 4, 2016, 3:02 a.m.)
> 
> 
> Review request for Sqoop and Colin Ma.
> 
> 
> Bugs: SQOOP-2797
> https://issues.apache.org/jira/browse/SQOOP-2797
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Add Blob data type support for Derby
> 
> 
> Diffs
> -
> 
>   
> common-test/src/main/java/org/apache/sqoop/common/test/asserts/ProviderAsserts.java
>  ae1b60d 
>   
> common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java
>  afc5016 
>   
> common-test/src/main/java/org/apache/sqoop/common/test/db/DerbyProvider.java 
> 8f3e434 
>   
> common-test/src/main/java/org/apache/sqoop/common/test/db/types/DerbyTypeList.java
>  642651d 
>   common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java 
> 3a3f9e8 
>   common/src/main/java/org/apache/sqoop/schema/type/Blob.java PRE-CREATION 
>   common/src/main/java/org/apache/sqoop/schema/type/ColumnType.java 9e415bf 
>   
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java
>  0235f28 
>   
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/util/SqlTypesUtils.java
>  a6ffa7c 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java
>  fc25100 
> 
> Diff: https://reviews.apache.org/r/42776/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colin Ma
> 
>

Review Request 43171: SQOOP-2819: Use connector name instead of id in Repository.findJobsForConnector methods

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2819
https://issues.apache.org/jira/browse/SQOOP-2819


Repository: sqoop-sqoop2


Description
---

In SQOOP-2573 we've migrated {{Repository. findLinksForConnectorUpgrade}} and 
{{Repository. findLinksForConnector}} to use name instead of id. it seems that 
we forgot to do the same for {{Repository. findJobsForConnectorUpgrade}} and 
{{Repository. findJobsForConnector}} though.


Diffs
-

  core/src/main/java/org/apache/sqoop/repository/JdbcRepository.java 1b7cd2e 
  core/src/main/java/org/apache/sqoop/repository/JdbcRepositoryHandler.java 
7047be9 
  core/src/main/java/org/apache/sqoop/repository/Repository.java 1f7bcfd 
  core/src/test/java/org/apache/sqoop/repository/TestJdbcRepository.java 
d26ce71 
  
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryHandler.java
 5490324 
  
repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepositoryInsertUpdateDeleteSelectQuery.java
 58404d7 
  
repository/repository-derby/src/test/java/org/apache/sqoop/repository/derby/TestJobHandling.java
 ad8946b 
  
repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestJobHandling.java
 9109212 
  
repository/repository-postgresql/src/test/java/org/apache/sqoop/integration/repository/postgresql/TestJobHandling.java
 6636bd3 
  server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java 9dfcb0b 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 42776: SQOOP-2797: Sqoop2: Datatypes: Add Blob data type support for Derby

[Jarek Cecho]

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



Thanks for updating the JIRA description to correspond to "new" scope Colin :) 
I have few comments:


common-test/src/main/java/org/apache/sqoop/common/test/asserts/ProviderAsserts.java
 (lines 86 - 89)
<https://reviews.apache.org/r/42776/#comment179029>

I think that we can use assertEquals(byte [], byte[]): 
http://testng.org/javadoc/org/testng/Assert.html#assertEquals(byte[],%20byte[])

Also FYI: the for loop always tests first byte instead of using "i" :)



common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 
(line 310)
<https://reviews.apache.org/r/42776/#comment179030>

Can we update the javadoc that blob will be skipped?



connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java
 (lines 20 - 36)
<https://reviews.apache.org/r/42776/#comment179031>

Nit: Let's not move the imports.


Jarcec

- Jarek Cecho


On Feb. 3, 2016, 7:54 a.m., Colin Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42776/
> ---
> 
> (Updated Feb. 3, 2016, 7:54 a.m.)
> 
> 
> Review request for Sqoop and Colin Ma.
> 
> 
> Bugs: SQOOP-2797
> https://issues.apache.org/jira/browse/SQOOP-2797
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Add Blob data type support for Derby
> 
> 
> Diffs
> -
> 
>   
> common-test/src/main/java/org/apache/sqoop/common/test/asserts/ProviderAsserts.java
>  ae1b60d 
>   
> common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java
>  afc5016 
>   
> common-test/src/main/java/org/apache/sqoop/common/test/db/DerbyProvider.java 
> 8f3e434 
>   
> common-test/src/main/java/org/apache/sqoop/common/test/db/types/DerbyTypeList.java
>  642651d 
>   common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java 
> 3a3f9e8 
>   common/src/main/java/org/apache/sqoop/schema/type/Blob.java PRE-CREATION 
>   common/src/main/java/org/apache/sqoop/schema/type/ColumnType.java 9e415bf 
>   
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java
>  0235f28 
>   
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/util/SqlTypesUtils.java
>  a6ffa7c 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java
>  fc25100 
> 
> Diff: https://reviews.apache.org/r/42776/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colin Ma
> 
>

Re: Review Request 41971: Sqoop2: Enable kerberos for HadoopMiniCluster

[Jarek Cecho]

> On Jan. 6, 2016, 4:07 p.m., Jarek Cecho wrote:
> > test/src/main/java/org/apache/sqoop/test/hadoop/HadoopRunner.java, line 74
> > <https://reviews.apache.org/r/41971/diff/1/?file=1183938#file1183938line74>
> >
> > Why do we need to change the directory to /tmp/ ?
> > 
> > Aren't we by mistake using this for local path somewhere?
> 
> Dian Fu wrote:
> In kerberos enabled cluster, we will get exceptions like this with 
> "/mapreduce-job-io":
> ```
> 2016-01-07 10:00:46,381 INFO  
> [org.apache.hadoop.hdfs.server.namenode.FSNamesystem$DefaultAuditLogger.logAuditMessage(FSNamesystem.java:9332)]
>  allowed=falseugi=sqoopclient (auth:TOKEN) via 
> hadoop/bdpe20.sh.intel@example.com (auth:TOKEN)ip=/127.0.0.1   
> cmd=create  
> src=/mapreduce-job-io/org.apache.sqoop.integration.connector.hdfs.AppendModeTest/test/.577fcfcd-6ef8-491c-ad6c-c6f4983dfdf0/22200b07-c11b-4c39-b0f3-c8236b34f44e.txt
> dst=nullperm=null   proto=rpc
> 2016-01-07 10:00:46,381 DEBUG 
> [org.apache.hadoop.ipc.ProtobufRpcEngine$Server$ProtoBufRpcInvoker.call(ProtobufRpcEngine.java:634)]
>  Served: create queueTime= 1 procesingTime= 1 exception= 
> AccessControlException
> 2016-01-07 10:00:46,381 DEBUG 
> [org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1632)]
>  PrivilegedActionException as:sqoopclient (auth:TOKEN) via 
> hadoop/bdpe20.sh.intel@example.com (auth:TOKEN) 
> cause:org.apache.hadoop.security.AccessControlException: Permission denied: 
> user=sqoopclient, access=WRITE, inode="/":root:supergroup:drwxr-xr-x
> ```
> This reason to this exception is that MR job tries to create directory 
> "/mapreduce-job-io" with user **sqoopclient**. But only superuser is allowed 
> to create directory under "/". While I found a better way to handle this: 
> just create directory "mapreduce-job-io" with superuser and changes its 
> permission to "0777" during cluster initial phase.

Got it, I'm fine with /tmp/ then :)


- Jarek


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


On Jan. 29, 2016, 8:46 a.m., Dian Fu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41971/
> ---
> 
> (Updated Jan. 29, 2016, 8:46 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2744
> https://issues.apache.org/jira/browse/SQOOP-2744
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> The aim of this JIRA is to enable kerberos for HadoopMiniCluster.
> 
> 
> Diffs
> -
> 
>   test/pom.xml 644a9c7 
>   
> test/src/main/java/org/apache/sqoop/test/hadoop/HadoopMiniClusterRunner.java 
> 2c0c4e6 
>   test/src/main/java/org/apache/sqoop/test/infrastructure/SqoopTestCase.java 
> 74fe29b 
>   test/src/main/java/org/apache/sqoop/test/kdc/KdcRunner.java aa5c6fc 
>   test/src/main/java/org/apache/sqoop/test/kdc/MiniKdcRunner.java 299fd9f 
>   test/src/main/java/org/apache/sqoop/test/kdc/NoKerberosKdcRunner.java 
> 29a005e 
>   test/src/main/java/org/apache/sqoop/test/minicluster/SqoopMiniCluster.java 
> 4ff97e7 
>   
> test/src/test/java/org/apache/sqoop/integration/connector/hdfs/OutputDirectoryTest.java
>  e5e3d26 
>   
> test/src/test/java/org/apache/sqoop/integration/connector/hive/FromRDBMSToKiteHiveTest.java
>  ec9f733 
>   
> test/src/test/java/org/apache/sqoop/integration/connectorloading/BlacklistedConnectorTest.java
>  1c7a483 
>   
> test/src/test/java/org/apache/sqoop/integration/connectorloading/ClasspathTest.java
>  5e49064 
>   
> test/src/test/java/org/apache/sqoop/integration/connectorloading/ConnectorClasspathIsolationTest.java
>  bbbf749 
>   test/src/test/resources/hive-tests-suite.xml PRE-CREATION 
>   test/src/test/resources/integration-tests-suite.xml 1cf3299 
> 
> Diff: https://reviews.apache.org/r/41971/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dian Fu
> 
>

Re: Review Request 42776: SQOOP-2797: Sqoop2: Datatypes: Add Blob data type support for Derby

[Jarek Cecho]

> On Feb. 2, 2016, 8:54 p.m., Jarek Cecho wrote:
> > common/src/main/java/org/apache/sqoop/schema/type/Blob.java, line 20
> > <https://reviews.apache.org/r/42776/diff/2/?file=1221169#file1221169line20>
> >
> > What is the difference between Blob and Binary? Aren't they the same?
> 
> Colin Ma wrote:
> They are not same during the process. To extract the data, the Blob 
> should be transformed to binary[] as the following code:
> Blob blob = resultSet.getBlob(i + 1);
> byte[] bytes = blob.getBytes(1, (int)blob.length());
> 
> For Binary, it's unnecessary.

Got it. Changing our schema objects is however a huge change and should be done 
"by the way" on JIRA that is meant to add something to our test infrastructure. 
Let's take this one to a standalone JIRA and let's properly document (explain) 
why we are doing that.


- Jarek


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


On Feb. 3, 2016, 7:54 a.m., Colin Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42776/
> ---
> 
> (Updated Feb. 3, 2016, 7:54 a.m.)
> 
> 
> Review request for Sqoop and Colin Ma.
> 
> 
> Bugs: SQOOP-2797
> https://issues.apache.org/jira/browse/SQOOP-2797
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Add Blob data type support for Derby
> 
> 
> Diffs
> -
> 
>   
> common-test/src/main/java/org/apache/sqoop/common/test/asserts/ProviderAsserts.java
>  ae1b60d 
>   
> common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java
>  afc5016 
>   
> common-test/src/main/java/org/apache/sqoop/common/test/db/DerbyProvider.java 
> 8f3e434 
>   
> common-test/src/main/java/org/apache/sqoop/common/test/db/types/DerbyTypeList.java
>  642651d 
>   common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java 
> 3a3f9e8 
>   common/src/main/java/org/apache/sqoop/schema/type/Blob.java PRE-CREATION 
>   common/src/main/java/org/apache/sqoop/schema/type/ColumnType.java 9e415bf 
>   
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java
>  0235f28 
>   
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/util/SqlTypesUtils.java
>  a6ffa7c 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java
>  fc25100 
> 
> Diff: https://reviews.apache.org/r/42776/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colin Ma
> 
>

Re: Review Request 42785: SQOOP-2807: Sqoop2: Add admin user list to configuration file

[Jarek Cecho]

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



I have one high level comment - how does this work with the "usual" RBAC that 
we have (for example the Sentry conrete plugin)? Aren't we introducing yet 
another authorization framework by any chance?

- Jarek Cecho


On Jan. 26, 2016, 5:50 a.m., Colin Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42785/
> ---
> 
> (Updated Jan. 26, 2016, 5:50 a.m.)
> 
> 
> Review request for Sqoop and Colin Ma.
> 
> 
> Bugs: SQOOP-2807
> https://issues.apache.org/jira/browse/SQOOP-2807
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Add admin user list to configuration file, and Sqoop can do the admin 
> authorization according to the value.
> For example:
>  org.apache.sqoop.security.admins=admin1,admin2
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/sqoop/security/AuthorizationManager.java 
> 4afdf02 
>   core/src/main/java/org/apache/sqoop/security/SecurityConstants.java 6f32e04 
>   dist/src/main/conf/sqoop.properties 2895530 
> 
> Diff: https://reviews.apache.org/r/42785/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colin Ma
> 
>

Re: Review Request 42776: SQOOP-2797: Sqoop2: Datatypes: Add Blob data type support for Derby

[Jarek Cecho]

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




common-test/src/main/java/org/apache/sqoop/common/test/asserts/ProviderAsserts.java
 (line 25)
<https://reviews.apache.org/r/42776/#comment178661>

Nit: We're usually trying to avoid asterisk imports, so let's not make this 
change.



common-test/src/main/java/org/apache/sqoop/common/test/asserts/ProviderAsserts.java
 (lines 57 - 58)
<https://reviews.apache.org/r/42776/#comment178662>

Since BLOB is for "binary" data, would it make sense to use the getBytes() 
method and compare using bytes rather then casting to string?



common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 
(line 26)
<https://reviews.apache.org/r/42776/#comment178663>

Nit: Asterisk import.



common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 
(lines 321 - 325)
<https://reviews.apache.org/r/42776/#comment178664>

Aren't we missing case for BLOB here?



common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java 
(lines 335 - 336)
<https://reviews.apache.org/r/42776/#comment178665>

Why are we skipping Blob?



common-test/src/main/java/org/apache/sqoop/common/test/db/types/DerbyTypeList.java
 (lines 112 - 118)
<https://reviews.apache.org/r/42776/#comment178666>

Let's make the whole method throw "Exception" to avoid a need to use try {} 
catch blocks.



common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java (lines 
28 - 29)
<https://reviews.apache.org/r/42776/#comment178667>

Nit: Asterisk import.



common/src/main/java/org/apache/sqoop/schema/type/Blob.java (line 20)
<https://reviews.apache.org/r/42776/#comment178668>

What is the difference between Blob and Binary? Aren't they the same?



connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java
 (line 20)
<https://reviews.apache.org/r/42776/#comment178669>

Nit: Asterisk imports.



connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/util/SqlTypesUtils.java
 (line 20)
<https://reviews.apache.org/r/42776/#comment178670>

Nit: Asterisk imports.



connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/util/SqlTypesUtils.java
 (lines 81 - 87)
<https://reviews.apache.org/r/42776/#comment178671>

Why do we need to encode the BLOB differently?



test/src/main/java/org/apache/sqoop/test/infrastructure/SqoopTestCase.java 
(lines 123 - 128)
<https://reviews.apache.org/r/42776/#comment178672>

The changes in this file seems independent on the other changes. Can we 
perhaps submit them in separate JIRA?


- Jarek Cecho


On Jan. 26, 2016, 4:55 a.m., Colin Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42776/
> ---
> 
> (Updated Jan. 26, 2016, 4:55 a.m.)
> 
> 
> Review request for Sqoop and Colin Ma.
> 
> 
> Bugs: SQOOP-2797
> https://issues.apache.org/jira/browse/SQOOP-2797
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Add Blob data type support for Derby
> 
> 
> Diffs
> -
> 
>   
> common-test/src/main/java/org/apache/sqoop/common/test/asserts/ProviderAsserts.java
>  4e1ef6a 
>   
> common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java
>  afc5016 
>   
> common-test/src/main/java/org/apache/sqoop/common/test/db/types/DerbyTypeList.java
>  642651d 
>   common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java 
> 3a3f9e8 
>   common/src/main/java/org/apache/sqoop/schema/type/Blob.java PRE-CREATION 
>   common/src/main/java/org/apache/sqoop/schema/type/ColumnType.java 9e415bf 
>   
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java
>  0235f28 
>   
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/util/SqlTypesUtils.java
>  a6ffa7c 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java
>  9b0885a 
>   test/src/main/java/org/apache/sqoop/test/infrastructure/SqoopTestCase.java 
> 74fe29b 
> 
> Diff: https://reviews.apache.org/r/42776/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colin Ma
> 
>

Re: Review Request 42951: SQOOP-2808

[Jarek Cecho]

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


Ship it!




Ship It!

- Jarek Cecho


On Feb. 1, 2016, 9:58 p.m., Abraham Fine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42951/
> ---
> 
> (Updated Feb. 1, 2016, 9:58 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: sqoop-2808
> https://issues.apache.org/jira/browse/sqoop-2808
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Sqoop2: Integration tests should test rows with null values
> 
> 
> Diffs
> -
> 
>   
> common-test/src/main/java/org/apache/sqoop/common/test/asserts/ProviderAsserts.java
>  4e1ef6a2586ab7cf0582b42e7fe6a6d6b3fd5d50 
>   
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutor.java
>  7c943c2b44f444f8b8eab914acd2f312fedeeef8 
>   
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java
>  b430739a4ec9ea21ffbd913b544902a7c76a1161 
>   
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java
>  774221aaf5c8cdb8d26ca108fae239598b42229b 
>   
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java
>  7d2177fd89735b80e43d26d4ff9d89bf3cab64aa 
>   
> connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java
>  11fcef2a38209c79928f582cf8aa03e889247f22 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java
>  9b0885ada3a914719aeaa70417bca7f41e71257f 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/AVROIntermediateDataFormat.java
>  d78fa8b72ecfe62eeec240e01597e7f2a7e4dd76 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/JSONIntermediateDataFormat.java
>  8db4d3da2eae33772f6f3c82ce0a1f2f2127ae02 
>   
> connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestAVROIntermediateDataFormat.java
>  703ed0a071afe055ed5ad953ed47987ea96468b7 
>   
> connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java
>  040dbfcc9a8b6fea75e4e5f263cf125b123b00b2 
>   
> connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestJSONIntermediateDataFormat.java
>  bcc1f95911aa7c51fad9bb902264ca610e3bd0e3 
>   
> test/src/test/java/org/apache/sqoop/integration/connector/hdfs/NullValueTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42951/diff/
> 
> 
> Testing
> ---
> 
> yes
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>

Re: Review Request 42989: SQOOP-2811: Sqoop2: Extracting sequence files may result in duplicates

[Jarek Cecho]

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


Ship it!




Ship It!

- Jarek Cecho


On Jan. 29, 2016, 11:47 p.m., Abraham Fine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42989/
> ---
> 
> (Updated Jan. 29, 2016, 11:47 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2811
> https://issues.apache.org/jira/browse/SQOOP-2811
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Sqoop2: Extracting sequence files may result in duplicates
> 
> 
> Diffs
> -
> 
>   
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java
>  a813c479a07d68e14ed49936f642e762e5b37437 
>   
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsPartition.java
>  644de60581faf90ceb2fcef8d3e0544067791fcc 
>   
> connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/SqoopTaskAttemptContext.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42989/diff/
> 
> 
> Testing
> ---
> 
> yes
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>

Re: Review Request 42875: SQOOP-2802

[Jarek Cecho]

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


Ship it!




Ship It!

- Jarek Cecho


On Jan. 27, 2016, 10:26 p.m., Abraham Fine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42875/
> ---
> 
> (Updated Jan. 27, 2016, 10:26 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2802
> https://issues.apache.org/jira/browse/SQOOP-2802
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Sqoop2: Logging level should be debug for integration tests
> 
> 
> Diffs
> -
> 
>   dist/src/main/conf/sqoop.properties 
> 2895530f5c2facea03ef6ccbbc31d1932ca01fb2 
>   execution/mapreduce/src/main/java/org/apache/sqoop/job/MRJobConstants.java 
> 896819832426d46ba2ea8aa080b290e62de94598 
>   
> execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/MRConfigurationUtils.java
>  1e1b237bab221401b49f19a07e623a6f11639357 
>   execution/mapreduce/src/main/resources/META-INF/log4j.properties 
> 2a577141a1b23040fa8366813846d53592487d5c 
>   
> submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/Constants.java
>  93b072551cc2cf8ec52b41bc754ef0b3d9468459 
>   
> submission/mapreduce/src/main/java/org/apache/sqoop/submission/mapreduce/MapreduceSubmissionEngine.java
>  c03bf39af0d437b8b9fb71b8efb76c2a7148dd7c 
>   test/src/main/java/org/apache/sqoop/test/minicluster/SqoopMiniCluster.java 
> 4ff97e70827ad7cf67ddae90b71dbf86c23efd5e 
> 
> Diff: https://reviews.apache.org/r/42875/diff/
> 
> 
> Testing
> ---
> 
> yes
> 
> 
> Thanks,
> 
> Abraham Fine
> 
>

Re: Review Request 37348: Fix Bug: Sqoop import query does not recognize the PK of IBM DB2 table.The Jira number is SQOOP-2328

[Jarek Cecho]

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



Can we add test cases for the fix? I would like to make sure that we have the 
scenario covered, so that we won't regress in the future.

- Jarek Cecho


On Aug. 11, 2015, 9:09 a.m., Shashank Tandon wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37348/
> ---
> 
> (Updated Aug. 11, 2015, 9:09 a.m.)
> 
> 
> Review request for Sqoop and Venkat Ranganathan.
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> Currently Sqoop import query does not recognize the PK of IBM DB2 table.
> when any sqoop query is runs for DB2 table, it is not able to recognize the 
> primary key(PK) of that table, which is used as --split-by column implicitly. 
> And it becomes mandatory to give --split-by  explicitly in the sqoop 
> query. This is fixed in this sqoop patch.
> 
> 
> Diffs
> -
> 
>   src/java/org/apache/sqoop/manager/Db2Manager.java e39aa4c 
> 
> Diff: https://reviews.apache.org/r/37348/diff/
> 
> 
> Testing
> ---
> 
> Yes
> 
> 
> Thanks,
> 
> Shashank Tandon
> 
>

Re: Review Request 42446: SQOOP-2776: Sqoop2: Add web interface for thread dump

[Jarek Cecho]

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


Thanks for the patch Colin!

I was thinking about the approach and I have a small concern, so I wanted to 
run it by you (and the rest of the community). When I look at Hadoop and other 
components, the "debug" interface runs as a simple HTTP page(s) on completely 
different ports. E.g. the thread dump and other debug pages are not part of the 
normal public APIs. I believe that this is because by exposing the debug info 
on separate interface makes it easy to disable access to it (on firewall or 
even in component's own configuration). I'm concerned that by offering the 
debug info as part of our main REST interface we might be opening a security 
hole when users will start be concerned about leaking sensitive information. We 
could possibly solve that problem by using our authorization model to protect 
those calls, but I'm not sure if it's necessary. What do you think?

- Jarek Cecho


On Jan. 20, 2016, 3:18 a.m., Colin Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42446/
> ---
> 
> (Updated Jan. 20, 2016, 3:18 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Add web interface for thread dump, user can get the information from the 
> shell.
> 
> 
> Diffs
> -
> 
>   client/src/main/java/org/apache/sqoop/client/SqoopClient.java 1cf549e 
>   
> client/src/main/java/org/apache/sqoop/client/request/SqoopResourceRequests.java
>  33c90a8 
>   
> client/src/main/java/org/apache/sqoop/client/request/ThreadDumpResourceRequest.java
>  PRE-CREATION 
>   common/pom.xml 7237608 
>   common/src/main/java/org/apache/sqoop/json/ThreadDumpBean.java PRE-CREATION 
>   server/src/main/java/org/apache/sqoop/handler/ThreadDumpRequestHandler.java 
> PRE-CREATION 
>   server/src/main/java/org/apache/sqoop/server/SqoopJettyServer.java 2c4cb7a 
>   server/src/main/java/org/apache/sqoop/server/ThreadDumpServlet.java 
> PRE-CREATION 
>   shell/src/main/java/org/apache/sqoop/shell/ShowCommand.java eb8522a 
>   shell/src/main/java/org/apache/sqoop/shell/ShowThreadDumpFunction.java 
> PRE-CREATION 
>   shell/src/main/java/org/apache/sqoop/shell/core/Constants.java 9c57a2e 
>   test/src/test/java/org/apache/sqoop/integration/shell/ShowCommandTest.java 
> 9fd4811 
> 
> Diff: https://reviews.apache.org/r/42446/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colin Ma
> 
>

Re: Review Request 42495: SQOOP-2796: Sqoop2: Add cmd tool to automate patch upload process

[Jarek Cecho]

> On Jan. 19, 2016, 6:52 p.m., Abraham Fine wrote:
> > dev-support/upload-patch.py, line 194
> > <https://reviews.apache.org/r/42495/diff/1/?file=1201503#file1201503line194>
> >
> > i name my git branches after the jira. i imagine i am not the only one. 
> > couldn't we try to get the jira from the branch name?
> 
> Jarek Cecho wrote:
> That's good suggestion - would you mind if I'll do that in subsequent 
> JIRA?

https://issues.apache.org/jira/browse/SQOOP-2799


- Jarek


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


On Jan. 19, 2016, 11:13 a.m., Jarek Cecho wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42495/
> ---
> 
> (Updated Jan. 19, 2016, 11:13 a.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2796
> https://issues.apache.org/jira/browse/SQOOP-2796
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Our process to submit a contribution to Sqoop has quite a few steps:
> 
> # Clone repo
> # Work on the fix/feature/...
> # Generate text patch
> # Upload it to review board
> # Don't forget to publish the review board
> # Upload the patch to JIRA
> # Switch the JIRA to "Patch available"
> 
> Forgetting to do any of those steps will lead to a quite unnecessary delays  
> with accepting the patch so I would like to automate steps 3-7 with simple 
> script that one can call from the computer and all that will happen 
> "magically" automatically.
> 
> 
> Diffs
> -
> 
>   dev-support/upload-patch.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42495/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Re: Review Request 42495: SQOOP-2796: Sqoop2: Add cmd tool to automate patch upload process

[Jarek Cecho]

> On Jan. 20, 2016, 2:30 a.m., Colin Ma wrote:
> > dev-support/upload-patch.py, line 168
> > <https://reviews.apache.org/r/42495/diff/1/?file=1201503#file1201503line168>
> >
> > The patch name is the same, add some suffix will be easy to find the 
> > lastest one.
> > For example, in SQOOP-2792, all patches have the same name and the 
> > update time is yesterday, I don't know which one should be used.

This is actually something that JIRA itself is solving for us - all "older" 
files have greyed out name whereas only the "most up to date" variant is blue 
(and that is the one that should be used).

I've heard this concern a few times already though, so I guess that people are 
not aware of this "feature". Perhaps we should improve it to make it easier for 
everyone. Would you mind doing that as part of separate JIRA though?


> On Jan. 20, 2016, 2:30 a.m., Colin Ma wrote:
> > dev-support/upload-patch.py, line 273
> > <https://reviews.apache.org/r/42495/diff/1/?file=1201503#file1201503line273>
> >
> > It will be better to add a link to review board in JIRA.

The scripts already updates JIRA with the link to the review board (just on 
different place).


- Jarek


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


On Jan. 19, 2016, 11:13 a.m., Jarek Cecho wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42495/
> -------
> 
> (Updated Jan. 19, 2016, 11:13 a.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-2796
> https://issues.apache.org/jira/browse/SQOOP-2796
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> ---
> 
> Our process to submit a contribution to Sqoop has quite a few steps:
> 
> # Clone repo
> # Work on the fix/feature/...
> # Generate text patch
> # Upload it to review board
> # Don't forget to publish the review board
> # Upload the patch to JIRA
> # Switch the JIRA to "Patch available"
> 
> Forgetting to do any of those steps will lead to a quite unnecessary delays  
> with accepting the patch so I would like to automate steps 3-7 with simple 
> script that one can call from the computer and all that will happen 
> "magically" automatically.
> 
> 
> Diffs
> -
> 
>   dev-support/upload-patch.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42495/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jarek Cecho
> 
>

Review Request 42494: SQOOP-2792: Test JIRA, please ignore

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2792
https://issues.apache.org/jira/browse/SQOOP-2792


Repository: sqoop-sqoop2


Description
---

Testing JIRA, please ignore any updates on this one.


Diffs
-

  README.txt 8251755 
  dev-support/upload-patch.py PRE-CREATION 
  extra.file PRE-CREATION 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 42494: SQOOP-2792: Test JIRA, please ignore

[Jarek Cecho]

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

(Updated Jan. 19, 2016, 10:14 a.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2792
https://issues.apache.org/jira/browse/SQOOP-2792


Repository: sqoop-sqoop2


Description
---

Testing JIRA, please ignore any updates on this one.


Diffs (updated)
-

  README.txt 8251755 
  dev-support/upload-patch.py PRE-CREATION 
  extra.file PRE-CREATION 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 42494: SQOOP-2792: Test JIRA, please ignore

[Jarek Cecho]

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

(Updated Jan. 19, 2016, 10:20 a.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2792
https://issues.apache.org/jira/browse/SQOOP-2792


Repository: sqoop-sqoop2


Description
---

Testing JIRA, please ignore any updates on this one.


Diffs (updated)
-

  README.txt 8251755 
  dev-support/upload-patch.py PRE-CREATION 
  extra.file PRE-CREATION 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 42494: SQOOP-2792: Test JIRA, please ignore

[Jarek Cecho]

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

(Updated Jan. 19, 2016, 10:24 a.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2792
https://issues.apache.org/jira/browse/SQOOP-2792


Repository: sqoop-sqoop2


Description
---

Testing JIRA, please ignore any updates on this one.


Diffs (updated)
-

  README.txt 8251755 
  dev-support/upload-patch.py PRE-CREATION 
  extra.file PRE-CREATION 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 42494: SQOOP-2792: Test JIRA, please ignore

[Jarek Cecho]

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

(Updated Jan. 19, 2016, 10:34 a.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2792
https://issues.apache.org/jira/browse/SQOOP-2792


Repository: sqoop-sqoop2


Description
---

Testing JIRA, please ignore any updates on this one.


Diffs (updated)
-

  README.txt 8251755 
  dev-support/upload-patch.py PRE-CREATION 
  extra.file PRE-CREATION 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 42494: SQOOP-2792: Test JIRA, please ignore

[Jarek Cecho]

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

(Updated Jan. 19, 2016, 10:35 a.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2792
https://issues.apache.org/jira/browse/SQOOP-2792


Repository: sqoop-sqoop2


Description
---

Testing JIRA, please ignore any updates on this one.


Diffs (updated)
-

  README.txt 8251755 
  dev-support/upload-patch.py PRE-CREATION 
  extra.file PRE-CREATION 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 42494: SQOOP-2792: Test JIRA, please ignore

[Jarek Cecho]

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

(Updated Jan. 19, 2016, 10:36 a.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2792
https://issues.apache.org/jira/browse/SQOOP-2792


Repository: sqoop-sqoop2


Description
---

Testing JIRA, please ignore any updates on this one.


Diffs (updated)
-

  README.txt 8251755 
  dev-support/upload-patch.py PRE-CREATION 
  extra.file PRE-CREATION 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 42494: SQOOP-2792: Test JIRA, please ignore

[Jarek Cecho]

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

(Updated Jan. 19, 2016, 10:43 a.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2792
https://issues.apache.org/jira/browse/SQOOP-2792


Repository: sqoop-sqoop2


Description
---

Testing JIRA, please ignore any updates on this one.


Diffs (updated)
-

  README.txt 8251755 
  dev-support/upload-patch.py PRE-CREATION 
  extra.file PRE-CREATION 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 42494: SQOOP-2792: Test JIRA, please ignore

[Jarek Cecho]

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

(Updated Jan. 19, 2016, 10:44 a.m.)


Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2792
https://issues.apache.org/jira/browse/SQOOP-2792


Repository: sqoop-sqoop2


Description
---

Testing JIRA, please ignore any updates on this one.


Diffs (updated)
-

  README.txt 8251755 
  dev-support/upload-patch.py PRE-CREATION 
  extra.file PRE-CREATION 

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


Testing
---


Thanks,

Jarek Cecho

Review Request 42495: SQOOP-2796: Sqoop2: Add cmd tool to automate patch upload process

[Jarek Cecho]

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

Review request for Sqoop and Jarek Cecho.


Bugs: SQOOP-2796
https://issues.apache.org/jira/browse/SQOOP-2796


Repository: sqoop-sqoop2


Description
---

Our process to submit a contribution to Sqoop has quite a few steps:

# Clone repo
# Work on the fix/feature/...
# Generate text patch
# Upload it to review board
# Don't forget to publish the review board
# Upload the patch to JIRA
# Switch the JIRA to "Patch available"

Forgetting to do any of those steps will lead to a quite unnecessary delays  
with accepting the patch so I would like to automate steps 3-7 with simple 
script that one can call from the computer and all that will happen "magically" 
automatically.


Diffs
-

  dev-support/upload-patch.py PRE-CREATION 

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


Testing
---


Thanks,

Jarek Cecho

Re: Review Request 41898: SQOOP-2771 Sqoop2: Remove the notion of SubmissionBean

[Jarek Cecho]

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

(Updated Jan. 15, 2016, 5:49 p.m.)


Review request for Sqoop.


Changes
---

Rebased.


Bugs: SQOOP-2771
https://issues.apache.org/jira/browse/SQOOP-2771


Repository: sqoop-sqoop2


Description
---

Removed the class as suggested in JIRA.


Diffs (updated)
-

  
client/src/main/java/org/apache/sqoop/client/request/SqoopResourceRequests.java 
bb5242f 
  
client/src/main/java/org/apache/sqoop/client/request/SubmissionResourceRequest.java
 0317b93 
  common/src/main/java/org/apache/sqoop/json/SubmissionBean.java 4040688 
  common/src/main/java/org/apache/sqoop/json/SubmissionsBean.java 52e8efa 
  common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java 204c1de 
  server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java 
6cf3dbe 
  
test/src/test/java/org/apache/sqoop/integration/tools/RepositoryDumpLoadToolTest.java
 f046e25 
  tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryDumpTool.java 
49978fa 
  tools/src/main/java/org/apache/sqoop/tools/tool/RepositoryLoadTool.java 
08c914a 

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


Testing
---

Unit tests are passing. Integration tests will be verified by precommit hook.


Thanks,

Jarek Cecho