Re: Review Request 68799: [PIG-3038] Support for Credentials for UDF, Loader and Storer

2018-09-26 Thread Rohini Palaniswamy 

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


Ship it!




Ship It!

- Rohini Palaniswamy


On Sept. 26, 2018, 4:23 p.m., Satish Saley wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68799/
> ---
> 
> (Updated Sept. 26, 2018, 4:23 p.m.)
> 
> 
> Review request for pig.
> 
> 
> Repository: pig-git
> 
> 
> Description
> ---
> 
> [PIG-3038] Support for Credentials for UDF,Loader and Storer
> 
> 
> Diffs
> -
> 
>   src/org/apache/pig/EvalFunc.java fd139a8b4 
>   src/org/apache/pig/LoadFunc.java 83e89a34c 
>   src/org/apache/pig/StoreFuncInterface.java c590084dc 
>   
> src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java
>  4d3ab5086 
>   
> src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POUserFunc.java
>  2c8dea608 
>   src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java 
> f292487f0 
>   
> src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/LoaderProcessor.java
>  7a12df784 
>   src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java 98040382f 
>   test/org/apache/pig/test/TestCredentials.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68799/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Satish Saley
> 
>

Re: Review Request 68799: [PIG-3038] Support for Credentials for UDF, Loader and Storer

2018-09-26 Thread Satish Saley 

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

(Updated Sept. 26, 2018, 9:23 a.m.)


Review request for pig.


Repository: pig-git


Description
---

[PIG-3038] Support for Credentials for UDF,Loader and Storer


Diffs (updated)
-

  src/org/apache/pig/EvalFunc.java fd139a8b4 
  src/org/apache/pig/LoadFunc.java 83e89a34c 
  src/org/apache/pig/StoreFuncInterface.java c590084dc 
  
src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java
 4d3ab5086 
  
src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POUserFunc.java
 2c8dea608 
  src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java 
f292487f0 
  
src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/LoaderProcessor.java
 7a12df784 
  src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java 98040382f 
  test/org/apache/pig/test/TestCredentials.java PRE-CREATION 


Diff: https://reviews.apache.org/r/68799/diff/5/

Changes: https://reviews.apache.org/r/68799/diff/4-5/


Testing
---


Thanks,

Satish Saley

Re: Review Request 68799: [PIG-3038] Support for Credentials for UDF, Loader and Storer

2018-09-26 Thread Rohini Palaniswamy 

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




test/org/apache/pig/test/TestCredentials.java
Lines 128 (patched)


Can you rename methods as testCredentialsEvalFunc, testCredentialsLoadFunc 
and testCredentialsStoreFunc?


- Rohini Palaniswamy


On Sept. 24, 2018, 7:39 p.m., Satish Saley wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68799/
> ---
> 
> (Updated Sept. 24, 2018, 7:39 p.m.)
> 
> 
> Review request for pig.
> 
> 
> Repository: pig-git
> 
> 
> Description
> ---
> 
> [PIG-3038] Support for Credentials for UDF,Loader and Storer
> 
> 
> Diffs
> -
> 
>   src/org/apache/pig/EvalFunc.java fd139a8b4 
>   src/org/apache/pig/LoadFunc.java 83e89a34c 
>   src/org/apache/pig/StoreFuncInterface.java c590084dc 
>   
> src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java
>  4d3ab5086 
>   
> src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POUserFunc.java
>  2c8dea608 
>   src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java 
> f292487f0 
>   
> src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/LoaderProcessor.java
>  7a12df784 
>   src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java 98040382f 
>   test/org/apache/pig/test/TestCredentials.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68799/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Satish Saley
> 
>

Re: Review Request 68799: [PIG-3038] Support for Credentials for UDF, Loader and Storer

2018-09-24 Thread Satish Saley 

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

(Updated Sept. 24, 2018, 12:39 p.m.)


Review request for pig.


Repository: pig-git


Description
---

[PIG-3038] Support for Credentials for UDF,Loader and Storer


Diffs (updated)
-

  src/org/apache/pig/EvalFunc.java fd139a8b4 
  src/org/apache/pig/LoadFunc.java 83e89a34c 
  src/org/apache/pig/StoreFuncInterface.java c590084dc 
  
src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java
 4d3ab5086 
  
src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POUserFunc.java
 2c8dea608 
  src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java 
f292487f0 
  
src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/LoaderProcessor.java
 7a12df784 
  src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java 98040382f 
  test/org/apache/pig/test/TestCredentials.java PRE-CREATION 


Diff: https://reviews.apache.org/r/68799/diff/4/

Changes: https://reviews.apache.org/r/68799/diff/3-4/


Testing
---


Thanks,

Satish Saley

Re: Review Request 68799: [PIG-3038] Support for Credentials for UDF, Loader and Storer

2018-09-21 Thread Satish Saley 

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

(Updated Sept. 21, 2018, 9:22 p.m.)


Review request for pig.


Repository: pig-git


Description
---

[PIG-3038] Support for Credentials for UDF,Loader and Storer


Diffs (updated)
-

  src/org/apache/pig/EvalFunc.java fd139a8b4 
  src/org/apache/pig/LoadFunc.java 83e89a34c 
  src/org/apache/pig/StoreFuncInterface.java c590084dc 
  
src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java
 4d3ab5086 
  
src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POUserFunc.java
 2c8dea608 
  src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java 
f292487f0 
  
src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/LoaderProcessor.java
 7a12df784 
  src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java 98040382f 
  test/org/apache/pig/test/TestCredentials.java PRE-CREATION 


Diff: https://reviews.apache.org/r/68799/diff/3/

Changes: https://reviews.apache.org/r/68799/diff/2-3/


Testing
---


Thanks,

Satish Saley

Re: Review Request 68799: [PIG-3038] Support for Credentials for UDF, Loader and Storer

2018-09-21 Thread Rohini Palaniswamy 

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




test/org/apache/pig/test/TestCredentialsEvalFunc.java
Lines 42 (patched)


You need to run it in cluster mode


- Rohini Palaniswamy


On Sept. 21, 2018, 5:47 p.m., Satish Saley wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68799/
> ---
> 
> (Updated Sept. 21, 2018, 5:47 p.m.)
> 
> 
> Review request for pig.
> 
> 
> Repository: pig-git
> 
> 
> Description
> ---
> 
> [PIG-3038] Support for Credentials for UDF,Loader and Storer
> 
> 
> Diffs
> -
> 
>   src/org/apache/pig/EvalFunc.java fd139a8b4 
>   src/org/apache/pig/LoadFunc.java 83e89a34c 
>   src/org/apache/pig/StoreFuncInterface.java c590084dc 
>   
> src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java
>  4d3ab5086 
>   
> src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POUserFunc.java
>  2c8dea608 
>   
> src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/plans/UdfCacheShipFilesVisitor.java
>  033fff7c2 
>   src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java 
> f292487f0 
>   src/org/apache/pig/backend/hadoop/executionengine/tez/TezJobCompiler.java 
> 6343c819a 
>   src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperPlan.java 
> 4f9b75b14 
>   
> src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezPOUserFuncVisitor.java
>  47d75855f 
>   
> src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/LoaderProcessor.java
>  7a12df784 
>   src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java 98040382f 
>   test/org/apache/pig/test/TestCredentialsEvalFunc.java PRE-CREATION 
>   test/org/apache/pig/test/TestLoaderStorerShipCacheFiles.java 793e1277d 
>   test/org/apache/pig/test/TestLoaderStorerShipCacheFilesMR.java 42b36e517 
>   test/org/apache/pig/test/utils/CredAvroStorage.java PRE-CREATION 
>   test/org/apache/pig/test/utils/CredentialsEvalFunc.java PRE-CREATION 
>   test/org/apache/pig/tez/TestLoaderStorerShipCacheFilesTez.java 4ba8abb09 
> 
> 
> Diff: https://reviews.apache.org/r/68799/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Satish Saley
> 
>

Re: Review Request 68799: [PIG-3038] Support for Credentials for UDF, Loader and Storer

2018-09-21 Thread Rohini Palaniswamy 

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




src/org/apache/pig/EvalFunc.java
Lines 388 (patched)


Allows adding secrets or custom credentials that can be used to talk to 
external systems. For eg: keys to decrypt encrypted data, database passwords, 
hcatalog/hbase delegation tokens, etc. This will be called once on the front 
end before the job is submitted. The added credentials can be accessed in the 
backend



src/org/apache/pig/EvalFunc.java
Lines 390 (patched)


to which delegation tokens and secrets can be added



src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java
Lines 562 (patched)


I think it would be cleaner to do similar thing for Tez as well in 
TezDAGBuilder instead of doing it in UDFShipCacheFilesVisitor. MR and Spark 
code also refer to that class.

Can remove these checks as they are redundant

if(userFuncs!=null && userFuncs.size()>0){
if(userFunc.getFunc() != null) {



src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/plans/UdfCacheShipFilesVisitor.java
Lines 40 (patched)


not initialized



src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/plans/UdfCacheShipFilesVisitor.java
Lines 44 (patched)


Initialize credentials from the plan. New constructor is not required



src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperPlan.java
Line 132 (original), 133 (patched)


can be done in the visitor itself



src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java
Lines 832 (patched)


Just do jobConf.setCredentials(credentials) here and get rid of variable ret



test/org/apache/pig/test/TestCredentialsEvalFunc.java
Lines 38 (patched)


Have a TestCredentials class for all credential tests and have all the test 
EvalFunc and LoadFunc classes within it.


- Rohini Palaniswamy


On Sept. 21, 2018, 5:47 p.m., Satish Saley wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68799/
> ---
> 
> (Updated Sept. 21, 2018, 5:47 p.m.)
> 
> 
> Review request for pig.
> 
> 
> Repository: pig-git
> 
> 
> Description
> ---
> 
> [PIG-3038] Support for Credentials for UDF,Loader and Storer
> 
> 
> Diffs
> -
> 
>   src/org/apache/pig/EvalFunc.java fd139a8b4 
>   src/org/apache/pig/LoadFunc.java 83e89a34c 
>   src/org/apache/pig/StoreFuncInterface.java c590084dc 
>   
> src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java
>  4d3ab5086 
>   
> src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POUserFunc.java
>  2c8dea608 
>   
> src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/plans/UdfCacheShipFilesVisitor.java
>  033fff7c2 
>   src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java 
> f292487f0 
>   src/org/apache/pig/backend/hadoop/executionengine/tez/TezJobCompiler.java 
> 6343c819a 
>   src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperPlan.java 
> 4f9b75b14 
>   
> src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezPOUserFuncVisitor.java
>  47d75855f 
>   
> src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/LoaderProcessor.java
>  7a12df784 
>   src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java 98040382f 
>   test/org/apache/pig/test/TestCredentialsEvalFunc.java PRE-CREATION 
>   test/org/apache/pig/test/TestLoaderStorerShipCacheFiles.java 793e1277d 
>   test/org/apache/pig/test/TestLoaderStorerShipCacheFilesMR.java 42b36e517 
>   test/org/apache/pig/test/utils/CredAvroStorage.java PRE-CREATION 
>   test/org/apache/pig/test/utils/CredentialsEvalFunc.java PRE-CREATION 
>   test/org/apache/pig/tez/TestLoaderStorerShipCacheFilesTez.java 4ba8abb09 
> 
> 
> Diff: https://reviews.apache.org/r/68799/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Satish Saley
> 
>

Re: Review Request 68799: [PIG-3038] Support for Credentials for UDF, Loader and Storer

2018-09-21 Thread Satish Saley 

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

(Updated Sept. 21, 2018, 10:47 a.m.)


Review request for pig.


Repository: pig-git


Description
---

[PIG-3038] Support for Credentials for UDF,Loader and Storer


Diffs (updated)
-

  src/org/apache/pig/EvalFunc.java fd139a8b4 
  src/org/apache/pig/LoadFunc.java 83e89a34c 
  src/org/apache/pig/StoreFuncInterface.java c590084dc 
  
src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java
 4d3ab5086 
  
src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POUserFunc.java
 2c8dea608 
  
src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/plans/UdfCacheShipFilesVisitor.java
 033fff7c2 
  src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java 
f292487f0 
  src/org/apache/pig/backend/hadoop/executionengine/tez/TezJobCompiler.java 
6343c819a 
  src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperPlan.java 
4f9b75b14 
  
src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezPOUserFuncVisitor.java
 47d75855f 
  
src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/LoaderProcessor.java
 7a12df784 
  src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java 98040382f 
  test/org/apache/pig/test/TestCredentialsEvalFunc.java PRE-CREATION 
  test/org/apache/pig/test/TestLoaderStorerShipCacheFiles.java 793e1277d 
  test/org/apache/pig/test/TestLoaderStorerShipCacheFilesMR.java 42b36e517 
  test/org/apache/pig/test/utils/CredAvroStorage.java PRE-CREATION 
  test/org/apache/pig/test/utils/CredentialsEvalFunc.java PRE-CREATION 
  test/org/apache/pig/tez/TestLoaderStorerShipCacheFilesTez.java 4ba8abb09 


Diff: https://reviews.apache.org/r/68799/diff/2/

Changes: https://reviews.apache.org/r/68799/diff/1-2/


Testing
---


Thanks,

Satish Saley

Review Request 68799: [PIG-3038] Support for Credentials for UDF, Loader and Storer

2018-09-21 Thread Satish Saley 

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

Review request for pig.


Repository: pig-git


Description
---

[PIG-3038] Support for Credentials for UDF,Loader and Storer


Diffs
-

  src/org/apache/pig/EvalFunc.java fd139a8b4 
  src/org/apache/pig/LoadFunc.java 83e89a34c 
  src/org/apache/pig/StoreFuncInterface.java c590084dc 
  src/org/apache/pig/StoreResources.java 284990b44 
  
src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java
 4d3ab5086 
  
src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POUserFunc.java
 2c8dea608 
  
src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/plans/UdfCacheShipFilesVisitor.java
 033fff7c2 
  src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java 
f292487f0 
  src/org/apache/pig/backend/hadoop/executionengine/tez/TezJobCompiler.java 
6343c819a 
  src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperPlan.java 
4f9b75b14 
  
src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezPOUserFuncVisitor.java
 47d75855f 
  
src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/LoaderProcessor.java
 7a12df784 
  src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java 98040382f 
  test/org/apache/pig/test/TestCredentialsEvalFunc.java PRE-CREATION 
  test/org/apache/pig/test/TestLoaderStorerShipCacheFiles.java 793e1277d 
  test/org/apache/pig/test/TestLoaderStorerShipCacheFilesMR.java 42b36e517 
  test/org/apache/pig/test/utils/CredAvroStorage.java PRE-CREATION 
  test/org/apache/pig/test/utils/CredentialsEvalFunc.java PRE-CREATION 
  test/org/apache/pig/tez/TestLoaderStorerShipCacheFilesTez.java 4ba8abb09 


Diff: https://reviews.apache.org/r/68799/diff/1/


Testing
---


Thanks,

Satish Saley