Re: Review Request 65485: RANGER-1971 - Switch to use for-each loops

2018-02-05 Thread Colm O hEigeartaigh 


> On Feb. 2, 2018, 10:36 p.m., Zsombor Gegesy wrote:
> > agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestCacheMap.java
> > Line 110 (original), 109 (patched)
> > 
> >
> > If this wasn't a test class, I would suggest to switch the if with the 
> > for.

Agreed, I've fixed it.


> On Feb. 2, 2018, 10:36 p.m., Zsombor Gegesy wrote:
> > ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
> > Line 431 (original), 430 (patched)
> > 
> >
> > Instead of 
> > if (map.containsKey(x)) {
> >...   map.get(x)
> > }
> > 
> > value = map.get(x);
> > if (value != null) { 
> >... value
> > }

Also fixed.


- Colm


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


On Feb. 2, 2018, 3:58 p.m., Colm O hEigeartaigh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65485/
> ---
> 
> (Updated Feb. 2, 2018, 3:58 p.m.)
> 
> 
> Review request for ranger.
> 
> 
> Bugs: RANGER-1971
> https://issues.apache.org/jira/browse/RANGER-1971
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> We should switch to use for-each loops in the code, as it is more readable 
> and concise than using an index.
> 
> 
> Diffs
> -
> 
>   
> agents-audit/src/main/java/org/apache/ranger/audit/provider/hdfs/HdfsLogDestination.java
>  065e8b07 
>   
> agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditFileSpool.java 
> 9abd99f5 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/geo/GeolocationMetadata.java
>  d27a0308 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/geo/RangerGeolocationData.java
>  99d6027f 
>   
> agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestCacheMap.java
>  5f39b224 
>   
> credentialbuilder/src/main/java/org/apache/ranger/credentialapi/buildks.java 
> b8bdb6fa 
>   
> hbase-agent/src/main/java/org/apache/ranger/authorization/hbase/RangerAuthorizationCoprocessor.java
>  e30f7957 
>   
> hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuthorizer.java
>  fa84b138 
>   jisql/src/main/java/org/apache/util/sql/Jisql.java 53a6ca4f 
>   
> kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSMetricUtil.java 
> 1527681a 
>   
> plugin-solr/src/main/java/org/apache/ranger/services/solr/client/ServiceSolrClient.java
>  5875a298 
>   security-admin/src/main/java/org/apache/ranger/biz/AssetMgr.java a53d46af 
>   security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java 272dec40 
>   security-admin/src/main/java/org/apache/ranger/common/RESTErrorUtil.java 
> 967804ad 
>   security-admin/src/main/java/org/apache/ranger/common/StringUtil.java 
> 045e07c4 
>   
> security-admin/src/main/java/org/apache/ranger/patch/cliutil/MetricUtil.java 
> d1ab0d09 
>   security-admin/src/main/java/org/apache/ranger/service/UserService.java 
> 3fb279e9 
>   
> security-admin/src/main/java/org/apache/ranger/service/filter/RangerRESTAPIFilter.java
>  551f824b 
>   security-admin/src/main/java/org/apache/ranger/util/RangerRestUtil.java 
> fe7a53e5 
>   
> ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapUserGroupBuilder.java
>  8dc147ca 
>   
> ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
>  ade2ee71 
> 
> 
> Diff: https://reviews.apache.org/r/65485/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>

Re: Review Request 65485: RANGER-1971 - Switch to use for-each loops

2018-02-02 Thread Ramesh Mani 

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


Ship it!




Ship It!

- Ramesh Mani


On Feb. 2, 2018, 3:58 p.m., Colm O hEigeartaigh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65485/
> ---
> 
> (Updated Feb. 2, 2018, 3:58 p.m.)
> 
> 
> Review request for ranger.
> 
> 
> Bugs: RANGER-1971
> https://issues.apache.org/jira/browse/RANGER-1971
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> We should switch to use for-each loops in the code, as it is more readable 
> and concise than using an index.
> 
> 
> Diffs
> -
> 
>   
> agents-audit/src/main/java/org/apache/ranger/audit/provider/hdfs/HdfsLogDestination.java
>  065e8b07 
>   
> agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditFileSpool.java 
> 9abd99f5 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/geo/GeolocationMetadata.java
>  d27a0308 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/geo/RangerGeolocationData.java
>  99d6027f 
>   
> agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestCacheMap.java
>  5f39b224 
>   
> credentialbuilder/src/main/java/org/apache/ranger/credentialapi/buildks.java 
> b8bdb6fa 
>   
> hbase-agent/src/main/java/org/apache/ranger/authorization/hbase/RangerAuthorizationCoprocessor.java
>  e30f7957 
>   
> hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuthorizer.java
>  fa84b138 
>   jisql/src/main/java/org/apache/util/sql/Jisql.java 53a6ca4f 
>   
> kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSMetricUtil.java 
> 1527681a 
>   
> plugin-solr/src/main/java/org/apache/ranger/services/solr/client/ServiceSolrClient.java
>  5875a298 
>   security-admin/src/main/java/org/apache/ranger/biz/AssetMgr.java a53d46af 
>   security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java 272dec40 
>   security-admin/src/main/java/org/apache/ranger/common/RESTErrorUtil.java 
> 967804ad 
>   security-admin/src/main/java/org/apache/ranger/common/StringUtil.java 
> 045e07c4 
>   
> security-admin/src/main/java/org/apache/ranger/patch/cliutil/MetricUtil.java 
> d1ab0d09 
>   security-admin/src/main/java/org/apache/ranger/service/UserService.java 
> 3fb279e9 
>   
> security-admin/src/main/java/org/apache/ranger/service/filter/RangerRESTAPIFilter.java
>  551f824b 
>   security-admin/src/main/java/org/apache/ranger/util/RangerRestUtil.java 
> fe7a53e5 
>   
> ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapUserGroupBuilder.java
>  8dc147ca 
>   
> ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
>  ade2ee71 
> 
> 
> Diff: https://reviews.apache.org/r/65485/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>

Re: Review Request 65485: RANGER-1971 - Switch to use for-each loops

2018-02-02 Thread Zsombor Gegesy 

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


Ship it!




I think, this can be merged as is.


agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestCacheMap.java
Line 110 (original), 109 (patched)


If this wasn't a test class, I would suggest to switch the if with the for.



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


Instead of 
if (map.containsKey(x)) {
   ...   map.get(x)
}

value = map.get(x);
if (value != null) { 
   ... value
}


- Zsombor Gegesy


On febr. 2, 2018, 3:58 du, Colm O hEigeartaigh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65485/
> ---
> 
> (Updated febr. 2, 2018, 3:58 du)
> 
> 
> Review request for ranger.
> 
> 
> Bugs: RANGER-1971
> https://issues.apache.org/jira/browse/RANGER-1971
> 
> 
> Repository: ranger
> 
> 
> Description
> ---
> 
> We should switch to use for-each loops in the code, as it is more readable 
> and concise than using an index.
> 
> 
> Diffs
> -
> 
>   
> agents-audit/src/main/java/org/apache/ranger/audit/provider/hdfs/HdfsLogDestination.java
>  065e8b07 
>   
> agents-audit/src/main/java/org/apache/ranger/audit/queue/AuditFileSpool.java 
> 9abd99f5 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/geo/GeolocationMetadata.java
>  d27a0308 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/geo/RangerGeolocationData.java
>  99d6027f 
>   
> agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestCacheMap.java
>  5f39b224 
>   
> credentialbuilder/src/main/java/org/apache/ranger/credentialapi/buildks.java 
> b8bdb6fa 
>   
> hbase-agent/src/main/java/org/apache/ranger/authorization/hbase/RangerAuthorizationCoprocessor.java
>  e30f7957 
>   
> hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuthorizer.java
>  fa84b138 
>   jisql/src/main/java/org/apache/util/sql/Jisql.java 53a6ca4f 
>   
> kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSMetricUtil.java 
> 1527681a 
>   
> plugin-solr/src/main/java/org/apache/ranger/services/solr/client/ServiceSolrClient.java
>  5875a298 
>   security-admin/src/main/java/org/apache/ranger/biz/AssetMgr.java a53d46af 
>   security-admin/src/main/java/org/apache/ranger/biz/UserMgr.java 272dec40 
>   security-admin/src/main/java/org/apache/ranger/common/RESTErrorUtil.java 
> 967804ad 
>   security-admin/src/main/java/org/apache/ranger/common/StringUtil.java 
> 045e07c4 
>   
> security-admin/src/main/java/org/apache/ranger/patch/cliutil/MetricUtil.java 
> d1ab0d09 
>   security-admin/src/main/java/org/apache/ranger/service/UserService.java 
> 3fb279e9 
>   
> security-admin/src/main/java/org/apache/ranger/service/filter/RangerRESTAPIFilter.java
>  551f824b 
>   security-admin/src/main/java/org/apache/ranger/util/RangerRestUtil.java 
> fe7a53e5 
>   
> ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapUserGroupBuilder.java
>  8dc147ca 
>   
> ugsync/src/main/java/org/apache/ranger/unixusersync/process/PolicyMgrUserGroupBuilder.java
>  ade2ee71 
> 
> 
> Diff: https://reviews.apache.org/r/65485/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Colm O hEigeartaigh
> 
>