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


Hi Abe,
thank you very much for working on this one. Changes looks good to me, I just 
have few minor comments:

1) Would you mind rebasing on current head of sqoop2 branch? I'm having 
compilation exception because of recent changes.


client/src/main/java/org/apache/sqoop/client/utils/FormDisplayer.java
<https://reviews.apache.org/r/10048/#comment38520>

    I can see the same if-else condition in every "displayInput$TYPE" method. 
What about refactoring the code a bit and putting the if-else logic into 
calling method displayForm()?



client/src/main/java/org/apache/sqoop/client/utils/FormFiller.java
<https://reviews.apache.org/r/10048/#comment38521>

    I can see the same if-else on multiple places. What about refactoring it to 
standalone method?


Jarcec

- Jarek Cecho


On March 21, 2013, 12:32 a.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10048/
> -----------------------------------------------------------
> 
> (Updated March 21, 2013, 12:32 a.m.)
> 
> 
> Review request for Sqoop, Jarek Cecho and Kathleen Ting.
> 
> 
> Description
> -------
> 
> commit 64f2ddbf5654ea8bbcc47dc7cc4cabd6168c4fad
> Author: Abraham Elmahrek <abra...@elmahrek.com>
> Date:   Wed Mar 20 12:15:59 2013 -0700
> 
>     SQOOP-947 Introduce the concept of "sensitivity" input to all supported 
> metadata structures
> 
> :100644 100644 8b40a54... 523f498... M        
> common/src/main/java/org/apache/sqoop/json/util/FormSerialization.java
> :100644 100644 e88216d... 80b10c8... M        
> common/src/main/java/org/apache/sqoop/model/FormUtils.java
> :100644 100644 77598a8... 12705a6... M        
> common/src/main/java/org/apache/sqoop/model/MEnumInput.java
> :100644 100644 96397e8... ea3d753... M        
> common/src/main/java/org/apache/sqoop/model/MInput.java
> :100644 100644 d281d7e... d23ac31... M        
> common/src/main/java/org/apache/sqoop/model/MIntegerInput.java
> :100644 100644 b458022... 704c1f8... M        
> common/src/main/java/org/apache/sqoop/model/MMapInput.java
> :100644 100644 e96ec92... 16da76d... M        
> common/src/main/java/org/apache/sqoop/model/MStringInput.java
> :100644 100644 85c65de... 08dfa7b... M        
> common/src/test/java/org/apache/sqoop/model/TestFormUtils.java
> :100644 100644 1cde5cf... 4ea42b1... M        
> common/src/test/java/org/apache/sqoop/model/TestMAccountableEntity.java
> :100644 100644 a99b7dc... c3469ff... M        
> common/src/test/java/org/apache/sqoop/model/TestMConnection.java
> :100644 100644 1d86c13... edaedb1... M        
> common/src/test/java/org/apache/sqoop/model/TestMEnumInput.java
> :100644 100644 b7b1356... 0bd55d9... M        
> common/src/test/java/org/apache/sqoop/model/TestMForm.java
> :100644 100644 5f599eb... bd21fcb... M        
> common/src/test/java/org/apache/sqoop/model/TestMFormList.java
> :100644 100644 042158a... 1f38e6d... M        
> common/src/test/java/org/apache/sqoop/model/TestMIntegerInput.java
> :100644 100644 772f230... 1587147... M        
> common/src/test/java/org/apache/sqoop/model/TestMMapInput.java
> :100644 100644 fa6b9ad... f336bab... M        
> common/src/test/java/org/apache/sqoop/model/TestMNamedElement.java
> :100644 100644 2ba9709... 7822ba5... M        
> common/src/test/java/org/apache/sqoop/model/TestMStringInput.java
> :100644 100644 8d07521... 3fd5a95... M        
> common/src/test/java/org/apache/sqoop/model/TestMValidatedElement.java
> :100644 100644 8af86b7... 486635d... M        
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
> 
> 
> This addresses bug SQOOP-947.
>     https://issues.apache.org/jira/browse/SQOOP-947
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/sqoop/client/core/Constants.java 
> cc80df108e2b2944fc31c9fba17b3639df663eec 
>   client/src/main/java/org/apache/sqoop/client/utils/FormDisplayer.java 
> 0609ae103fd3b7bcd9b0a3d1a9063ccf542d9b5c 
>   client/src/main/java/org/apache/sqoop/client/utils/FormFiller.java 
> f11b9d29d3344c18f763fea3d15de794af325f08 
>   client/src/main/resources/client-resource.properties 
> 734333eebf147fc547d1d0664f00c63f72b81f70 
>   common/src/main/java/org/apache/sqoop/json/util/FormSerialization.java 
> 8b40a547e230793a922cb97dd7f7031cb3e94712 
>   common/src/main/java/org/apache/sqoop/model/FormUtils.java 
> e88216d6f11548b9d6b20ee5f76257c1f72c1128 
>   common/src/main/java/org/apache/sqoop/model/MEnumInput.java 
> 77598a820b50907c7d4410604e27cfd575d707cc 
>   common/src/main/java/org/apache/sqoop/model/MInput.java 
> 96397e820e9f1075a37f1fffda52d3fd14cd9990 
>   common/src/main/java/org/apache/sqoop/model/MIntegerInput.java 
> d281d7efda9fcc0d265ee46601fd9529d8f2fdc4 
>   common/src/main/java/org/apache/sqoop/model/MMapInput.java 
> b4580221374b47ad1f64e4483a36abc6d8aac711 
>   common/src/main/java/org/apache/sqoop/model/MStringInput.java 
> e96ec929a5bdcabe03b61692d62ea8857ad0bade 
>   common/src/test/java/org/apache/sqoop/json/TestConnectionBean.java 
> 1322dd3d64f072f9a93d9ef5bfccbfb9aff0556b 
>   common/src/test/java/org/apache/sqoop/model/TestFormUtils.java 
> 85c65de6dc6b2029307d66f5b7b54763c090edcb 
>   common/src/test/java/org/apache/sqoop/model/TestMAccountableEntity.java 
> 1cde5cf29bbfc1a9ffb08107c52f69aa0c109088 
>   common/src/test/java/org/apache/sqoop/model/TestMConnection.java 
> a99b7dc0009924ed4be6ada7fa8731fc4f6d9c0c 
>   common/src/test/java/org/apache/sqoop/model/TestMEnumInput.java 
> 1d86c132d4465773fb90e5ccc4d91fe4de40db07 
>   common/src/test/java/org/apache/sqoop/model/TestMForm.java 
> b7b1356edef9447cf80d9d07f3ecef7b2e5cb916 
>   common/src/test/java/org/apache/sqoop/model/TestMFormList.java 
> 5f599ebd93865c81b181044943ab44d69a8ddeab 
>   common/src/test/java/org/apache/sqoop/model/TestMIntegerInput.java 
> 042158a8eabc3d2a2a41e0a3de5dda4e01517b36 
>   common/src/test/java/org/apache/sqoop/model/TestMMapInput.java 
> 772f230b17f22a829ba12a62425073c1d53b6895 
>   common/src/test/java/org/apache/sqoop/model/TestMNamedElement.java 
> fa6b9ad805b02a1d67185091d3f65045af9e7993 
>   common/src/test/java/org/apache/sqoop/model/TestMStringInput.java 
> 2ba9709c014b8d4d7b59505234f36773ee85f195 
>   common/src/test/java/org/apache/sqoop/model/TestMValidatedElement.java 
> 8d0752194a3c7e37be4f3a316c851ea55348ee36 
>   
> repository/repository-derby/src/main/java/org/apache/sqoop/repository/derby/DerbyRepositoryHandler.java
>  8af86b78f14ace5b440b610603e7031ca5117177 
> 
> Diff: https://reviews.apache.org/r/10048/diff/
> 
> 
> Testing
> -------
> 
> Ran unit tests and integration tests.
> Also performed requests with tcpdump to see if response and request JSON has 
> "sensitive" key.
> Added test case to test that sensitivity exists.
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>

Reply via email to