Re: Review Request 71217: RANGER-2535 : Good coding practices for storing and retrieving data history in ranger

2019-08-21 Thread Pradeep Agrawal

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


Ship it!




Ship It!

- Pradeep Agrawal


On Aug. 21, 2019, 7:10 a.m., Nikhil P wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71217/
> ---
> 
> (Updated Aug. 21, 2019, 7:10 a.m.)
> 
> 
> Review request for ranger, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, 
> Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, and 
> Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2535
> https://issues.apache.org/jira/browse/RANGER-2535
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> Good coding practices for storing and retrieving data history in ranger
> 
> 
> Diffs
> -
> 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java 
> 2af5845e2 
>   security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 
> 113e727c4 
>   security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 
> e7b317265 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerDataHistService.java
>  7bd0681b1 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelHelper.java
>  PRE-CREATION 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelsService.java
>  1a1b56e4c 
>   security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java 
> 2be371892 
> 
> 
> Diff: https://reviews.apache.org/r/71217/diff/3/
> 
> 
> Testing
> ---
> 
> 1.Tested If data is getting stored and retrieved properly in data history 
> table i.e. x_data_hist.
> 
> 
> Thanks,
> 
> Nikhil P
> 
>



Re: Review Request 71217: RANGER-2535 : Good coding practices for storing and retrieving data history in ranger

2019-08-21 Thread Nikhil P

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

(Updated Aug. 21, 2019, 12:40 p.m.)


Review request for ranger, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, 
Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, and 
Velmurugan Periasamy.


Bugs: RANGER-2535
https://issues.apache.org/jira/browse/RANGER-2535


Repository: ranger


Description
---

Good coding practices for storing and retrieving data history in ranger


Diffs (updated)
-

  agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java 
2af5845e2 
  security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 
113e727c4 
  security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 
e7b317265 
  
security-admin/src/main/java/org/apache/ranger/service/RangerDataHistService.java
 7bd0681b1 
  
security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelHelper.java
 PRE-CREATION 
  
security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelsService.java
 1a1b56e4c 
  security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java 
2be371892 


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

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


Testing
---

1.Tested If data is getting stored and retrieved properly in data history table 
i.e. x_data_hist.


Thanks,

Nikhil P



Re: Review Request 71217: RANGER-2535 : Good coding practices for storing and retrieving data history in ranger

2019-08-20 Thread Nikhil P


> On Aug. 16, 2019, 9:50 p.m., Pradeep Agrawal wrote:
> > security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
> > Line 2054 (original), 2058 (patched)
> > 
> >
> > There is already a deletePolicy(RangerPolicy policy, RangerService 
> > service) method there. Possibly you can use that by passing the service 
> > object value as null.
> 
> Nikhil P wrote:
> we are not adding a new method but changing existing method in order to 
> avoid extra DB call for get policy, so I feel its better to get rid of the 
> method with Id param which gives additional DB call to get policy which is 
> already present with the caller method of it.Also there would be no backward 
> compatability issue as this is not the REST layer method.
> 
> Pradeep Agrawal wrote:
> Won't a call to deletePolicy(RangerPolicy policy, RangerService service) 
> method will serve the purpose here ? if yes, then either you can keep this 
> method as is "deletePolicy(Long policyId)" or remove the implementation of 
> this method completly(to avoid pmd issues).
> 
> Nikhil P wrote:
> This delete method with policy & service parameter is used generally for 
> bulk delete,i.e policy loop at the caller method,so this method does not 
> throw an error if one of the policy is null while deleting. it seems to 
> ignore that policy and proceed to next, hence this method wont throw 
> exception to the user.
> whereas the current method with only policy parameter gets used for 
> single policy delete and it seems to check and throw an exception if that 
> policy is null, as this method has the REST level caller which deletes single 
> policy in one REST call.
> 
> do we plan to keep single policy delete method in ServiceDBStore for both 
> buld and single policy delete, If yes then we might need to think about 
> throwing an exception if policy does not exists/null.But I suppose we dont 
> throw an exception in case of bulk delete.
> 
> Pradeep Agrawal wrote:
> In case of bulk delete also error should be thrown. Its not being checked 
> in the deletePolicy(RangerPolicy policy, RangerService service) method as 
> bulk delete call always send valid RangerPolicy object. You can add the throw 
> statement in deletePolicy(RangerPolicy policy, RangerService service) as well.
> 
> Yes, there should be single method.

Bulk delete call does not necessarily always send valid RangerPolicy object.for 
instance: call from deleteExactMatchPolicyForResource() method where 
RangerPolicy object could be null as well. 
Throwing an exception in such case would break the flow and execution will stop 
there.while we need all valid policies provided to be deleted, those wont be 
deleted if we throw an error(for single invalid policy) from bulk delete API.


- Nikhil


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


On Aug. 19, 2019, 3:21 p.m., Nikhil P wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71217/
> ---
> 
> (Updated Aug. 19, 2019, 3:21 p.m.)
> 
> 
> Review request for ranger, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, 
> Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, and 
> Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2535
> https://issues.apache.org/jira/browse/RANGER-2535
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> Good coding practices for storing and retrieving data history in ranger
> 
> 
> Diffs
> -
> 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java 
> 2af5845e2 
>   security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 
> 113e727c4 
>   security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 
> e7b317265 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerDataHistService.java
>  7bd0681b1 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelHelper.java
>  PRE-CREATION 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelsService.java
>  1a1b56e4c 
>   security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java 
> 2be371892 
> 
> 
> Diff: https://reviews.apache.org/r/71217/diff/2/
> 
> 
> Testing
> ---
> 
> 1.Tested If data is getting stored and retrieved properly in data history 
> table i.e. x_data_hist.
> 
> 
> Thanks,
> 
> Nikhil P
> 
>



Re: Review Request 71217: RANGER-2535 : Good coding practices for storing and retrieving data history in ranger

2019-08-20 Thread Pradeep Agrawal


> On Aug. 16, 2019, 4:20 p.m., Pradeep Agrawal wrote:
> > security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
> > Line 2054 (original), 2058 (patched)
> > 
> >
> > There is already a deletePolicy(RangerPolicy policy, RangerService 
> > service) method there. Possibly you can use that by passing the service 
> > object value as null.
> 
> Nikhil P wrote:
> we are not adding a new method but changing existing method in order to 
> avoid extra DB call for get policy, so I feel its better to get rid of the 
> method with Id param which gives additional DB call to get policy which is 
> already present with the caller method of it.Also there would be no backward 
> compatability issue as this is not the REST layer method.
> 
> Pradeep Agrawal wrote:
> Won't a call to deletePolicy(RangerPolicy policy, RangerService service) 
> method will serve the purpose here ? if yes, then either you can keep this 
> method as is "deletePolicy(Long policyId)" or remove the implementation of 
> this method completly(to avoid pmd issues).
> 
> Nikhil P wrote:
> This delete method with policy & service parameter is used generally for 
> bulk delete,i.e policy loop at the caller method,so this method does not 
> throw an error if one of the policy is null while deleting. it seems to 
> ignore that policy and proceed to next, hence this method wont throw 
> exception to the user.
> whereas the current method with only policy parameter gets used for 
> single policy delete and it seems to check and throw an exception if that 
> policy is null, as this method has the REST level caller which deletes single 
> policy in one REST call.
> 
> do we plan to keep single policy delete method in ServiceDBStore for both 
> buld and single policy delete, If yes then we might need to think about 
> throwing an exception if policy does not exists/null.But I suppose we dont 
> throw an exception in case of bulk delete.

In case of bulk delete also error should be thrown. Its not being checked in 
the deletePolicy(RangerPolicy policy, RangerService service) method as bulk 
delete call always send valid RangerPolicy object. You can add the throw 
statement in deletePolicy(RangerPolicy policy, RangerService service) as well.

Yes, there should be single method.


- Pradeep


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


On Aug. 19, 2019, 9:51 a.m., Nikhil P wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71217/
> ---
> 
> (Updated Aug. 19, 2019, 9:51 a.m.)
> 
> 
> Review request for ranger, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, 
> Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, and 
> Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2535
> https://issues.apache.org/jira/browse/RANGER-2535
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> Good coding practices for storing and retrieving data history in ranger
> 
> 
> Diffs
> -
> 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java 
> 2af5845e2 
>   security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 
> 113e727c4 
>   security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 
> e7b317265 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerDataHistService.java
>  7bd0681b1 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelHelper.java
>  PRE-CREATION 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelsService.java
>  1a1b56e4c 
>   security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java 
> 2be371892 
> 
> 
> Diff: https://reviews.apache.org/r/71217/diff/2/
> 
> 
> Testing
> ---
> 
> 1.Tested If data is getting stored and retrieved properly in data history 
> table i.e. x_data_hist.
> 
> 
> Thanks,
> 
> Nikhil P
> 
>



Re: Review Request 71217: RANGER-2535 : Good coding practices for storing and retrieving data history in ranger

2019-08-20 Thread Pradeep Agrawal

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




security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
Line 2062 (original), 2064 (patched)


if policy is null then call to policy.getId() will throw null pointer 
exception. please correct the same in line 2060 also.


- Pradeep Agrawal


On Aug. 19, 2019, 9:51 a.m., Nikhil P wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71217/
> ---
> 
> (Updated Aug. 19, 2019, 9:51 a.m.)
> 
> 
> Review request for ranger, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, 
> Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, and 
> Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2535
> https://issues.apache.org/jira/browse/RANGER-2535
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> Good coding practices for storing and retrieving data history in ranger
> 
> 
> Diffs
> -
> 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java 
> 2af5845e2 
>   security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 
> 113e727c4 
>   security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 
> e7b317265 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerDataHistService.java
>  7bd0681b1 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelHelper.java
>  PRE-CREATION 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelsService.java
>  1a1b56e4c 
>   security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java 
> 2be371892 
> 
> 
> Diff: https://reviews.apache.org/r/71217/diff/2/
> 
> 
> Testing
> ---
> 
> 1.Tested If data is getting stored and retrieved properly in data history 
> table i.e. x_data_hist.
> 
> 
> Thanks,
> 
> Nikhil P
> 
>



Re: Review Request 71217: RANGER-2535 : Good coding practices for storing and retrieving data history in ranger

2019-08-20 Thread Nikhil P


> On Aug. 16, 2019, 9:50 p.m., Pradeep Agrawal wrote:
> > security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
> > Line 2054 (original), 2058 (patched)
> > 
> >
> > There is already a deletePolicy(RangerPolicy policy, RangerService 
> > service) method there. Possibly you can use that by passing the service 
> > object value as null.
> 
> Nikhil P wrote:
> we are not adding a new method but changing existing method in order to 
> avoid extra DB call for get policy, so I feel its better to get rid of the 
> method with Id param which gives additional DB call to get policy which is 
> already present with the caller method of it.Also there would be no backward 
> compatability issue as this is not the REST layer method.
> 
> Pradeep Agrawal wrote:
> Won't a call to deletePolicy(RangerPolicy policy, RangerService service) 
> method will serve the purpose here ? if yes, then either you can keep this 
> method as is "deletePolicy(Long policyId)" or remove the implementation of 
> this method completly(to avoid pmd issues).

This delete method with policy & service parameter is used generally for bulk 
delete,i.e policy loop at the caller method,so this method does not throw an 
error if one of the policy is null while deleting. it seems to ignore that 
policy and proceed to next, hence this method wont throw exception to the user.
whereas the current method with only policy parameter gets used for single 
policy delete and it seems to check and throw an exception if that policy is 
null, as this method has the REST level caller which deletes single policy in 
one REST call.

do we plan to keep single policy delete method in ServiceDBStore for both buld 
and single policy delete, If yes then we might need to think about throwing an 
exception if policy does not exists/null.But I suppose we dont throw an 
exception in case of bulk delete.


- Nikhil


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


On Aug. 19, 2019, 3:21 p.m., Nikhil P wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71217/
> ---
> 
> (Updated Aug. 19, 2019, 3:21 p.m.)
> 
> 
> Review request for ranger, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, 
> Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, and 
> Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2535
> https://issues.apache.org/jira/browse/RANGER-2535
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> Good coding practices for storing and retrieving data history in ranger
> 
> 
> Diffs
> -
> 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java 
> 2af5845e2 
>   security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 
> 113e727c4 
>   security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 
> e7b317265 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerDataHistService.java
>  7bd0681b1 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelHelper.java
>  PRE-CREATION 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelsService.java
>  1a1b56e4c 
>   security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java 
> 2be371892 
> 
> 
> Diff: https://reviews.apache.org/r/71217/diff/2/
> 
> 
> Testing
> ---
> 
> 1.Tested If data is getting stored and retrieved properly in data history 
> table i.e. x_data_hist.
> 
> 
> Thanks,
> 
> Nikhil P
> 
>



Re: Review Request 71217: RANGER-2535 : Good coding practices for storing and retrieving data history in ranger

2019-08-20 Thread Pradeep Agrawal


> On Aug. 16, 2019, 4:20 p.m., Pradeep Agrawal wrote:
> > security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
> > Line 2054 (original), 2058 (patched)
> > 
> >
> > There is already a deletePolicy(RangerPolicy policy, RangerService 
> > service) method there. Possibly you can use that by passing the service 
> > object value as null.
> 
> Nikhil P wrote:
> we are not adding a new method but changing existing method in order to 
> avoid extra DB call for get policy, so I feel its better to get rid of the 
> method with Id param which gives additional DB call to get policy which is 
> already present with the caller method of it.Also there would be no backward 
> compatability issue as this is not the REST layer method.

Won't a call to deletePolicy(RangerPolicy policy, RangerService service) method 
will serve the purpose here ? if yes, then either you can keep this method as 
is "deletePolicy(Long policyId)" or remove the implementation of this method 
completly(to avoid pmd issues).


- Pradeep


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


On Aug. 19, 2019, 9:51 a.m., Nikhil P wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71217/
> ---
> 
> (Updated Aug. 19, 2019, 9:51 a.m.)
> 
> 
> Review request for ranger, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, 
> Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, and 
> Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2535
> https://issues.apache.org/jira/browse/RANGER-2535
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> Good coding practices for storing and retrieving data history in ranger
> 
> 
> Diffs
> -
> 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java 
> 2af5845e2 
>   security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 
> 113e727c4 
>   security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 
> e7b317265 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerDataHistService.java
>  7bd0681b1 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelHelper.java
>  PRE-CREATION 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelsService.java
>  1a1b56e4c 
>   security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java 
> 2be371892 
> 
> 
> Diff: https://reviews.apache.org/r/71217/diff/2/
> 
> 
> Testing
> ---
> 
> 1.Tested If data is getting stored and retrieved properly in data history 
> table i.e. x_data_hist.
> 
> 
> Thanks,
> 
> Nikhil P
> 
>



Re: Review Request 71217: RANGER-2535 : Good coding practices for storing and retrieving data history in ranger

2019-08-19 Thread Nikhil P


> On Aug. 16, 2019, 9:50 p.m., Pradeep Agrawal wrote:
> > security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
> > Line 2054 (original), 2058 (patched)
> > 
> >
> > There is already a deletePolicy(RangerPolicy policy, RangerService 
> > service) method there. Possibly you can use that by passing the service 
> > object value as null.

we are not adding a new method but changing existing method in order to avoid 
extra DB call for get policy, so I feel its better to get rid of the method 
with Id param which gives additional DB call to get policy which is already 
present with the caller method of it.Also there would be no backward 
compatability issue as this is not the REST layer method.


- Nikhil


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


On Aug. 19, 2019, 3:21 p.m., Nikhil P wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71217/
> ---
> 
> (Updated Aug. 19, 2019, 3:21 p.m.)
> 
> 
> Review request for ranger, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, 
> Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, and 
> Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2535
> https://issues.apache.org/jira/browse/RANGER-2535
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> Good coding practices for storing and retrieving data history in ranger
> 
> 
> Diffs
> -
> 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java 
> 2af5845e2 
>   security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 
> 113e727c4 
>   security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 
> e7b317265 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerDataHistService.java
>  7bd0681b1 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelHelper.java
>  PRE-CREATION 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelsService.java
>  1a1b56e4c 
>   security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java 
> 2be371892 
> 
> 
> Diff: https://reviews.apache.org/r/71217/diff/2/
> 
> 
> Testing
> ---
> 
> 1.Tested If data is getting stored and retrieved properly in data history 
> table i.e. x_data_hist.
> 
> 
> Thanks,
> 
> Nikhil P
> 
>



Re: Review Request 71217: RANGER-2535 : Good coding practices for storing and retrieving data history in ranger

2019-08-19 Thread Nikhil P

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

(Updated Aug. 19, 2019, 3:21 p.m.)


Review request for ranger, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, 
Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, and 
Velmurugan Periasamy.


Bugs: RANGER-2535
https://issues.apache.org/jira/browse/RANGER-2535


Repository: ranger


Description
---

Good coding practices for storing and retrieving data history in ranger


Diffs (updated)
-

  agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java 
2af5845e2 
  security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 
113e727c4 
  security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 
e7b317265 
  
security-admin/src/main/java/org/apache/ranger/service/RangerDataHistService.java
 7bd0681b1 
  
security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelHelper.java
 PRE-CREATION 
  
security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelsService.java
 1a1b56e4c 
  security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java 
2be371892 


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

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


Testing
---

1.Tested If data is getting stored and retrieved properly in data history table 
i.e. x_data_hist.


Thanks,

Nikhil P



Re: Review Request 71217: RANGER-2535 : Good coding practices for storing and retrieving data history in ranger

2019-08-16 Thread Pradeep Agrawal

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




security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
Line 2054 (original), 2058 (patched)


There is already a deletePolicy(RangerPolicy policy, RangerService service) 
method there. Possibly you can use that by passing the service object value as 
null.



security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelHelper.java
Lines 48 (patched)


should be RangerPolicyLabelHelper.createNewOrGetLabel rather 
RangerPolicyLabelService.createNewOrGetLabel



security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelHelper.java
Lines 62 (patched)


change RangerPolicyLabelService to RangerPolicyLabelHelper


- Pradeep Agrawal


On Aug. 14, 2019, 2:39 p.m., Nikhil P wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71217/
> ---
> 
> (Updated Aug. 14, 2019, 2:39 p.m.)
> 
> 
> Review request for ranger, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, 
> Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, and 
> Velmurugan Periasamy.
> 
> 
> Bugs: RANGER-2535
> https://issues.apache.org/jira/browse/RANGER-2535
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> Good coding practices for storing and retrieving data history in ranger
> 
> 
> Diffs
> -
> 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java 
> 2af5845e2 
>   security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 
> 113e727c4 
>   security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 
> af74daf26 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerDataHistService.java
>  7bd0681b1 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelHelper.java
>  PRE-CREATION 
>   
> security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelsService.java
>  1a1b56e4c 
>   security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java 
> 2be371892 
> 
> 
> Diff: https://reviews.apache.org/r/71217/diff/1/
> 
> 
> Testing
> ---
> 
> 1.Tested If data is getting stored and retrieved properly in data history 
> table i.e. x_data_hist.
> 
> 
> Thanks,
> 
> Nikhil P
> 
>



Review Request 71217: RANGER-2535 : Good coding practices for storing and retrieving data history in ranger

2019-08-14 Thread Nikhil P

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

Review request for ranger, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, 
Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, and 
Velmurugan Periasamy.


Bugs: RANGER-2535
https://issues.apache.org/jira/browse/RANGER-2535


Repository: ranger


Description
---

Good coding practices for storing and retrieving data history in ranger


Diffs
-

  agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java 
2af5845e2 
  security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java 
113e727c4 
  security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java 
af74daf26 
  
security-admin/src/main/java/org/apache/ranger/service/RangerDataHistService.java
 7bd0681b1 
  
security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelHelper.java
 PRE-CREATION 
  
security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelsService.java
 1a1b56e4c 
  security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java 
2be371892 


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


Testing
---

1.Tested If data is getting stored and retrieved properly in data history table 
i.e. x_data_hist.


Thanks,

Nikhil P