Re: Review Request 73677: RANGER-3469: Off-By-One Error in XUser Syncing

2022-03-07 Thread Sailaja Polavarapu 

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




ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
Line 1023 (original), 1024 (patched)


I think uploadedCount should be initialized to 0 instead of -1
Same for lines 1118 & 1204


- Sailaja Polavarapu


On Dec. 10, 2021, 4 p.m., David Mollitor wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73677/
> ---
> 
> (Updated Dec. 10, 2021, 4 p.m.)
> 
> 
> Review request for ranger.
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> RANGER-3285: Off-By-One Error in XUser Syncing
> 
> 
> Diffs
> -
> 
>   
> ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
>  60445043f 
> 
> 
> Diff: https://reviews.apache.org/r/73677/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Mollitor
> 
>

Re: Review Request 73677: RANGER-3469: Off-By-One Error in XUser Syncing

2021-12-10 Thread David Mollitor 

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

(Updated Dec. 10, 2021, 4 p.m.)


Review request for ranger.


Repository: ranger


Description
---

RANGER-3285: Off-By-One Error in XUser Syncing


Diffs (updated)
-

  
ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
 60445043f 


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

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


Testing
---


Thanks,

David Mollitor

Re: Review Request 73677: RANGER-3469: Off-By-One Error in XUser Syncing

2021-12-08 Thread Sailaja Polavarapu 

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




ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
Line 932 (original), 932 (patched)


Also while going through the code, I noticed that similar check is missing 
for getGroups, getGroupUsers, and updateUserRoles. Can you please add?


- Sailaja Polavarapu


On Nov. 22, 2021, 9:40 p.m., David Mollitor wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73677/
> ---
> 
> (Updated Nov. 22, 2021, 9:40 p.m.)
> 
> 
> Review request for ranger.
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> RANGER-3285: Off-By-One Error in XUser Syncing
> 
> 
> Diffs
> -
> 
>   
> ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
>  60445043f 
> 
> 
> Diff: https://reviews.apache.org/r/73677/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Mollitor
> 
>

Re: Review Request 73677: RANGER-3469: Off-By-One Error in XUser Syncing

2021-12-08 Thread Sailaja Polavarapu 


> On Nov. 22, 2021, 10:35 p.m., Abhishek  Kumar wrote:
> > ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
> > Lines 919 (patched)
> > 
> >
> > Why is max required here ?
> 
> David Mollitor wrote:
> Hello, I am not sure, but this was the behavior of the original code and 
> I did not want to change behavior in this way for this particular issue.
> 
> David Mollitor wrote:
> Any additional questions or thoughts on this matter?

David,
 I agree that this behavior is from the original code. you can drop this 
comment. But please make similar changes for getGroups, getGroupUsers, and 
updateUserRoles where needed as commented by Abhishek.


- Sailaja


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


On Nov. 22, 2021, 9:40 p.m., David Mollitor wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73677/
> ---
> 
> (Updated Nov. 22, 2021, 9:40 p.m.)
> 
> 
> Review request for ranger.
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> RANGER-3285: Off-By-One Error in XUser Syncing
> 
> 
> Diffs
> -
> 
>   
> ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
>  60445043f 
> 
> 
> Diff: https://reviews.apache.org/r/73677/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Mollitor
> 
>

Re: Review Request 73677: RANGER-3469: Off-By-One Error in XUser Syncing

2021-12-03 Thread David Mollitor 


> On Nov. 22, 2021, 10:35 p.m., Abhishek  Kumar wrote:
> > ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
> > Lines 919 (patched)
> > 
> >
> > Why is max required here ?
> 
> David Mollitor wrote:
> Hello, I am not sure, but this was the behavior of the original code and 
> I did not want to change behavior in this way for this particular issue.

Any additional questions or thoughts on this matter?


- David


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


On Nov. 22, 2021, 9:40 p.m., David Mollitor wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73677/
> ---
> 
> (Updated Nov. 22, 2021, 9:40 p.m.)
> 
> 
> Review request for ranger.
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> RANGER-3285: Off-By-One Error in XUser Syncing
> 
> 
> Diffs
> -
> 
>   
> ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
>  60445043f 
> 
> 
> Diff: https://reviews.apache.org/r/73677/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Mollitor
> 
>

Re: Review Request 73677: RANGER-3469: Off-By-One Error in XUser Syncing

2021-11-22 Thread David Mollitor 


> On Nov. 22, 2021, 10:35 p.m., Abhishek  Kumar wrote:
> > ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
> > Lines 919 (patched)
> > 
> >
> > Why is max required here ?

Hello, I am not sure, but this was the behavior of the original code and I did 
not want to change behavior in this way for this particular issue.


- David


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


On Nov. 22, 2021, 9:40 p.m., David Mollitor wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73677/
> ---
> 
> (Updated Nov. 22, 2021, 9:40 p.m.)
> 
> 
> Review request for ranger.
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> RANGER-3285: Off-By-One Error in XUser Syncing
> 
> 
> Diffs
> -
> 
>   
> ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
>  60445043f 
> 
> 
> Diff: https://reviews.apache.org/r/73677/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Mollitor
> 
>

Re: Review Request 73677: RANGER-3469: Off-By-One Error in XUser Syncing

2021-11-22 Thread Abhishek Kumar 

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



Also, I believe a similar change would be required for getGroups, getGroupUsers 
and updateUserRoles as well.


ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
Lines 919 (patched)


Why is max required here ?


- Abhishek  Kumar


On Nov. 22, 2021, 9:40 p.m., David Mollitor wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73677/
> ---
> 
> (Updated Nov. 22, 2021, 9:40 p.m.)
> 
> 
> Review request for ranger.
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> RANGER-3285: Off-By-One Error in XUser Syncing
> 
> 
> Diffs
> -
> 
>   
> ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
>  60445043f 
> 
> 
> Diff: https://reviews.apache.org/r/73677/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Mollitor
> 
>

Re: Review Request 73677: RANGER-3469: Off-By-One Error in XUser Syncing

2021-11-22 Thread David Mollitor 


> On Nov. 16, 2021, 5:28 a.m., Abhishek  Kumar wrote:
> > The jira number tagged in the review does not appear to be associated with 
> > the code change. 
> > Please open a new jira and tag the review with it.

Hello.  My appologies.  I have no idea how I managed to do that.  I have 
updated the JIRA to reflect the correct value: 
https://issues.apache.org/jira/browse/RANGER-3469


- David


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


On Nov. 22, 2021, 9:40 p.m., David Mollitor wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73677/
> ---
> 
> (Updated Nov. 22, 2021, 9:40 p.m.)
> 
> 
> Review request for ranger.
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> RANGER-3285: Off-By-One Error in XUser Syncing
> 
> 
> Diffs
> -
> 
>   
> ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
>  60445043f 
> 
> 
> Diff: https://reviews.apache.org/r/73677/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Mollitor
> 
>

Re: Review Request 73677: RANGER-3469: Off-By-One Error in XUser Syncing

2021-11-22 Thread David Mollitor 

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

(Updated Nov. 22, 2021, 9:40 p.m.)


Review request for ranger.


Summary (updated)
-

RANGER-3469: Off-By-One Error in XUser Syncing


Repository: ranger


Description
---

RANGER-3285: Off-By-One Error in XUser Syncing


Diffs
-

  
ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
 60445043f 


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


Testing
---


Thanks,

David Mollitor