> On Nov. 5, 2015, 11:45 p.m., Sravya Tirukkovalur wrote:
> >
>
> Hao Hao wrote:
> In getUser / getGroup and some otehr APIs we are doing the same checking,
> if the condition is not good, then we should change all of them at the same
> time to be consistent?
>
> Sravya Tirukkovalur wrote:
> There is a difference between the two
>
> if (!authzInfo.isManaged(pathElements)
> || !authzInfo.doesBelongToAuthzObject(pathElements)) //Either
> not in prefix or not a hive object
>
>
>
> if (!authzInfo.isManaged(pathElements)) { //If not in prefix
> group = getDefaultProviderGroup(node, snapshotId);
> } else if (!authzInfo.doesBelongToAuthzObject(pathElements)) { //If
> in prefix and not a hive object
> group = getDefaultProviderGroup(node, snapshotId);
> }
>
> Hao Hao wrote:
> If a path is associated with hive object but not in prefix, the following
> statement will be evaluated to be true and writes to hdfs, which is what we
> desire, right? No op is only for paths inside the prefix + hive object.
>
> if (!authzInfo.isManaged(pathElements)
> || !authzInfo.doesBelongToAuthzObject(pathElements))
Going to have to side with Hao on this one. Hive objects which reside outside
of a prefix should be managed by the DefaultAuthorizationProvider and not
Sentry's implementation. Technically Hive objects which reside outside of a
prefix should not be found in UpdateableAuthzPath anyway. The initial check
just acts a short-circuit for the latter evaluation. That being said an or
evaluation serves the same purpose. It also improve readability.
- Ryan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39928/#review105329
-----------------------------------------------------------
On Nov. 6, 2015, 1:29 a.m., Hao Hao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39928/
> -----------------------------------------------------------
>
> (Updated Nov. 6, 2015, 1:29 a.m.)
>
>
> Review request for sentry, Anne Yu, Lenni Kuff, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Change-Id: I7c65bf182c44075f41de16943c5b7eb66e3dec0b
>
> SENTRY-994: Changed the Logger level and added more test case for
> removeACLFeature.
>
> Change-Id: I851344e088155e28c8978203759fe56884b29e41
>
>
> Diffs
> -----
>
>
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java
> 419ab68e0d03f995c55d229b762453468de47571
>
> sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryAuthorizationProvider.java
> fd5146f079d93687738a522f42beaa59031a4f82
>
> Diff: https://reviews.apache.org/r/39928/diff/
>
>
> Testing
> -------
>
> Added several new unit tests for setPermission/setUser/setGroup/removeAcl
> cases validation.
>
>
> Thanks,
>
> Hao Hao
>
>
