You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Colm O hEigeartaigh <co...@apache.org> on 2018/02/02 15:58:34 UTC
Review Request 65485: RANGER-1971 - Switch to use for-each loops
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65485/
-----------------------------------------------------------
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
Posted by Colm O hEigeartaigh <co...@apache.org>.
> 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)
> > <https://reviews.apache.org/r/65485/diff/1/?file=1952299#file1952299line110>
> >
> > 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)
> > <https://reviews.apache.org/r/65485/diff/1/?file=1952315#file1952315line431>
> >
> > 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
Posted by Zsombor Gegesy <gz...@gmail.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/65485/#comment276585>
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)
<https://reviews.apache.org/r/65485/#comment276586>
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
>
>
Re: Review Request 65485: RANGER-1971 - Switch to use for-each loops
Posted by Ramesh Mani <rm...@hortonworks.com>.
-----------------------------------------------------------
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
>
>