> On Aug. 16, 2018, 7:57 p.m., Sergio Pena wrote: > > sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java > > Lines 74-80 (patched) > > <https://reviews.apache.org/r/68367/diff/1/?file=2073209#file2073209line74> > > > > Why do we need to bring this thread local here? is there another way to > > get a list of the privileges without this variable? Why is it trhead local > > btw?
The same function can be called be multiple threads. Without thread local, the error experienced by thread1 will be returned to thread 2. It will be a mess. > On Aug. 16, 2018, 7:57 p.m., Sergio Pena wrote: > > sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java > > Lines 429 (patched) > > <https://reviews.apache.org/r/68367/diff/1/?file=2073209#file2073209line429> > > > > Is there an example of the message that will be printed? I think it > > would be useful to get some examples and ask a technical writer feedback > > about it. I feel we'll have unorganized information in the output. I can change it to just contain the first exception message. It will be the same as before. > On Aug. 16, 2018, 7:57 p.m., Sergio Pena wrote: > > sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java > > Lines 464 (patched) > > <https://reviews.apache.org/r/68367/diff/1/?file=2073210#file2073210line464> > > > > That hard-coded value looks like one of our server names. Can we use a > > fake name? will change it. It is still faked one as the original is "hdfs://rogue-4.gce.cloudera.com:8020/tmp" > On Aug. 16, 2018, 7:57 p.m., Sergio Pena wrote: > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java > > Lines 78-79 (patched) > > <https://reviews.apache.org/r/68367/diff/1/?file=2073211#file2073211line78> > > > > Is it necessary to expose this to the interface? Yes. So we can return the following privileges required after encounting the failure in access check, and avoid the overhead of calling privilege.implies() (which goes through each privileges and check if it implies the required privileges). - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68367/#review207428 ----------------------------------------------------------- On Aug. 15, 2018, 10:23 p.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68367/ > ----------------------------------------------------------- > > (Updated Aug. 15, 2018, 10:23 p.m.) > > > Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio > Pena. > > > Bugs: sentry-2354 > https://issues.apache.org/jira/browse/sentry-2354 > > > Repository: sentry > > > Description > ------- > > When multiple privileges are required on a Hive operation, return the > privilege that failed access check and the required privileges not checked > yet (they may fail the access check, or may not. We don't check those > privileges to avoid unnecessary overhead) > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java > 6a1556f > > sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java > 3bbf6fb > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java > aecfe5b > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/NoAuthorizationProvider.java > 61400ca > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java > 1e1aa63 > > > Diff: https://reviews.apache.org/r/68367/diff/1/ > > > Testing > ------- > > Add test case that verifies the behavior stated above. All test cases in > TestHiveAuthzBindings passed > > > Thanks, > > Na Li > >
