You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Sailaja Polavarapu <sp...@hortonworks.com> on 2017/01/27 19:35:22 UTC
Review Request 56035: RANGER-1334: Good coding practices in Ranger
Usersync
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56035/
-----------------------------------------------------------
Review request for ranger, Ankita Sinha, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
Bugs: RANGER-1334
https://issues.apache.org/jira/browse/RANGER-1334
Repository: ranger
Description
-------
Fixed few issues found in the recent changes in the usersync code.
Diffs
-----
ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapDeltaUserGroupBuilder.java 0779918
ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapPolicyMgrUserGroupBuilder.java 0b909d1
ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java 25c0824
Diff: https://reviews.apache.org/r/56035/diff/
Testing
-------
Confirmed all the unit tests ran successfully.
Thanks,
Sailaja Polavarapu
Re: Review Request 56035: RANGER-1334: Good coding practices in Ranger
Usersync
Posted by dilli dorai <di...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56035/#review163365
-----------------------------------------------------------
ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapDeltaUserGroupBuilder.java (line 699)
<https://reviews.apache.org/r/56035/#comment234775>
Is it intentional that "LdapDeltaUserGroupBuilder.getGroups() failed with exception: " is written only at log level ERROR, but stackTrace is printed irrespective of log level?
- dilli dorai
On Jan. 27, 2017, 7:35 p.m., Sailaja Polavarapu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56035/
> -----------------------------------------------------------
>
> (Updated Jan. 27, 2017, 7:35 p.m.)
>
>
> Review request for ranger, Ankita Sinha, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-1334
> https://issues.apache.org/jira/browse/RANGER-1334
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Fixed few issues found in the recent changes in the usersync code.
>
>
> Diffs
> -----
>
> ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapDeltaUserGroupBuilder.java 0779918
> ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapPolicyMgrUserGroupBuilder.java 0b909d1
> ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java 25c0824
>
> Diff: https://reviews.apache.org/r/56035/diff/
>
>
> Testing
> -------
>
> Confirmed all the unit tests ran successfully.
>
>
> Thanks,
>
> Sailaja Polavarapu
>
>
Re: Review Request 56035: RANGER-1334: Good coding practices in Ranger
Usersync
Posted by Zsombor Gegesy <gz...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56035/#review163322
-----------------------------------------------------------
ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapDeltaUserGroupBuilder.java (line 699)
<https://reviews.apache.org/r/56035/#comment234740>
You should use the LOG, it's not a good practice to call directly the printStackTrace() which ends up in the consol
- Zsombor Gegesy
On Jan. 27, 2017, 7:35 p.m., Sailaja Polavarapu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56035/
> -----------------------------------------------------------
>
> (Updated Jan. 27, 2017, 7:35 p.m.)
>
>
> Review request for ranger, Ankita Sinha, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-1334
> https://issues.apache.org/jira/browse/RANGER-1334
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Fixed few issues found in the recent changes in the usersync code.
>
>
> Diffs
> -----
>
> ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapDeltaUserGroupBuilder.java 0779918
> ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapPolicyMgrUserGroupBuilder.java 0b909d1
> ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java 25c0824
>
> Diff: https://reviews.apache.org/r/56035/diff/
>
>
> Testing
> -------
>
> Confirmed all the unit tests ran successfully.
>
>
> Thanks,
>
> Sailaja Polavarapu
>
>
Re: Review Request 56035: RANGER-1334: Good coding practices in Ranger
Usersync
Posted by Velmurugan Periasamy <vp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56035/#review163868
-----------------------------------------------------------
Ship it!
Ship It!
- Velmurugan Periasamy
On Jan. 30, 2017, 10:19 p.m., Sailaja Polavarapu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56035/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2017, 10:19 p.m.)
>
>
> Review request for ranger, Ankita Sinha, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-1334
> https://issues.apache.org/jira/browse/RANGER-1334
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Fixed few issues found in the recent changes in the usersync code.
>
>
> Diffs
> -----
>
> ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapDeltaUserGroupBuilder.java 0779918
> ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapPolicyMgrUserGroupBuilder.java 0b909d1
> ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java 25c0824
>
> Diff: https://reviews.apache.org/r/56035/diff/
>
>
> Testing
> -------
>
> Confirmed all the unit tests ran successfully.
>
>
> Thanks,
>
> Sailaja Polavarapu
>
>
Re: Review Request 56035: RANGER-1334: Good coding practices in Ranger
Usersync
Posted by dilli dorai <di...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56035/#review163614
-----------------------------------------------------------
Ship it!
Ship It!
ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapDeltaUserGroupBuilder.java (line 698)
<https://reviews.apache.org/r/56035/#comment235080>
you could pass t as second parameter rather than uing the + to concatenate
ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapDeltaUserGroupBuilder.java (line 699)
<https://reviews.apache.org/r/56035/#comment235081>
It does not seem logical one log record in a cathc block is at error level and one is at info level.
If you decide to do that, would recomment that the record that writes too much information sould be at info level and less information should be at error level. The way it is coded here, you write the log that generates too much information at error level and the log that writes one line is enabled at info level.
Not opening issue.
- dilli dorai
On Jan. 30, 2017, 10:19 p.m., Sailaja Polavarapu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56035/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2017, 10:19 p.m.)
>
>
> Review request for ranger, Ankita Sinha, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-1334
> https://issues.apache.org/jira/browse/RANGER-1334
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Fixed few issues found in the recent changes in the usersync code.
>
>
> Diffs
> -----
>
> ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapDeltaUserGroupBuilder.java 0779918
> ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapPolicyMgrUserGroupBuilder.java 0b909d1
> ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java 25c0824
>
> Diff: https://reviews.apache.org/r/56035/diff/
>
>
> Testing
> -------
>
> Confirmed all the unit tests ran successfully.
>
>
> Thanks,
>
> Sailaja Polavarapu
>
>
Re: Review Request 56035: RANGER-1334: Good coding practices in Ranger
Usersync
Posted by Sailaja Polavarapu <sp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56035/
-----------------------------------------------------------
(Updated Jan. 30, 2017, 10:19 p.m.)
Review request for ranger, Ankita Sinha, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy.
Changes
-------
fixed the code based on the review comments.
Bugs: RANGER-1334
https://issues.apache.org/jira/browse/RANGER-1334
Repository: ranger
Description
-------
Fixed few issues found in the recent changes in the usersync code.
Diffs (updated)
-----
ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapDeltaUserGroupBuilder.java 0779918
ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapPolicyMgrUserGroupBuilder.java 0b909d1
ugsync/src/main/java/org/apache/ranger/unixusersync/config/UserGroupSyncConfig.java 25c0824
Diff: https://reviews.apache.org/r/56035/diff/
Testing
-------
Confirmed all the unit tests ran successfully.
Thanks,
Sailaja Polavarapu