Review Request 63856: SENTRY-2048: Bump Hive version to 2.3.2

2017-11-15 Thread Sergio Pena via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63856/ --- Review request for sentry, Alexander Kolbasov, Colm O hEigeartaigh, and kalyan k

Re: Review Request 63775: SENTRY-1543 dropOrRenamePrivilegeForAllRoles() has confusing code

2017-11-15 Thread Steve Moist via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63775/ --- (Updated Nov. 15, 2017, 9:47 p.m.) Review request for sentry. Summary (update

Re: Shading of conflicting dependencies

2017-11-15 Thread Brian Towles
@Moist It can be found here https://github.com/apache/hbase/blob/master/hbase-shaded/hbase-shaded-check-invariants/src/test/resources/ensure-jars-have-correct-contents.sh On Wed, Nov 15, 2017 at 3:38 PM Brian Towles wrote: > HBase does have a ensure-jars-have-correct-contents.sh check that they

Re: Shading of conflicting dependencies

2017-11-15 Thread Brian Towles
HBase does have a ensure-jars-have-correct-contents.sh check that they run which we could do something similar to. On Wed, Nov 15, 2017 at 3:30 PM Stephen Moist wrote: > Is there any way to add a check to the maven build to make sure that the > classes being imported are the shaded ones? > > > >

Re: Shading of conflicting dependencies

2017-11-15 Thread Stephen Moist
Is there any way to add a check to the maven build to make sure that the classes being imported are the shaded ones? > On Nov 15, 2017, at 3:02 PM, Brian Towles wrote: > > I can see that. I can look into that as well. It would make sense for the > "Sentry as a Library" you talked about as we

Re: Review Request 63402: SENTRY-2021 MR session ACLs in Hive binding does not handle all types of ACLs

2017-11-15 Thread Wilfred Spiegelenburg via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63402/ --- (Updated Nov. 15, 2017, 9:13 p.m.) Review request for sentry. Bugs: SENTRY-20

Re: Review Request 63402: SENTRY-2021 MR session ACLs in Hive binding does not handle all types of ACLs

2017-11-15 Thread Wilfred Spiegelenburg via Review Board
> On Nov. 14, 2017, 8:19 p.m., Sergio Pena wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java > > Lines 116-117 (original), 116-122 (patched) > > >

Re: Shading of conflicting dependencies

2017-11-15 Thread Brian Towles
I can see that. I can look into that as well. It would make sense for the "Sentry as a Library" you talked about as well On Wed, Nov 15, 2017 at 2:56 PM Alexander Kolbasov wrote: > Brian, There are cases where shading is useful for server as well - mostly > for e2e tests which run server, HDFS

Re: Review Request 63402: SENTRY-2021 MR session ACLs in Hive binding does not handle all types of ACLs

2017-11-15 Thread Wilfred Spiegelenburg via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63402/ --- (Updated Nov. 15, 2017, 9:02 p.m.) Review request for sentry. Changes ---

Re: Shading of conflicting dependencies

2017-11-15 Thread Alexander Kolbasov
Brian, There are cases where shading is useful for server as well - mostly for e2e tests which run server, HDFS, Hive all in the same JVM. On Wed, Nov 15, 2017 at 12:47 PM, Brian Towles wrote: > So there would be two different levels of shading. > > The first would be the known issues shared rep

Re: Shading of conflicting dependencies

2017-11-15 Thread Brian Towles
So there would be two different levels of shading. The first would be the known issues shared repackaged dependencies like Guava. This would only be for libraries that we absolutely know will cause collisions with implementing services and implementing third party libraries that need to have the

Re: Shading of conflicting dependencies

2017-11-15 Thread Stephen Moist
+1 Agree with Kalyan’s concern. To me it seems simpler to shade all jars rather than known issue jars. One thing though is what if there are bug fixes in our jars that are not included in other components, it may cause subtle error that are hard to track. I’m thinking along the lines of the

Re: Review Request 63775: SENTRY-1543 Rename methods to be clearer

2017-11-15 Thread Sergio Pena via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63775/#review191083 --- The subject of the review board must match the JIRA subject. You c

Re: Shading of conflicting dependencies

2017-11-15 Thread Kalyan Kumar Kalvagadda
+1 It's good approach but I have a question/concern. Is the proposal to shade is for some specific jars OR to shade all the third party jars? If proposal is shade all the third-party jars then if would impact the run time memory usage as all the classes from the third-party jars would be loaded r

Re: Shading of conflicting dependencies

2017-11-15 Thread Na Li
+1 This gives sentry more flexibility without impacting other components using sentry. On Wed, Nov 15, 2017 at 12:04 PM, Sergio Pena wrote: > +1 > > I like the idea. It's hard to upgrade our libraries to newer ones when > other components break due to Sentry being a plugin. I was once thought o

Re: Review Request 63668: SENTRY-2038 - Some ShellCommand improvements

2017-11-15 Thread Sergio Pena via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63668/#review191082 --- Ship it! Ship It! - Sergio Pena On Nov. 15, 2017, 11:04 a.m.

Re: Shading of conflicting dependencies

2017-11-15 Thread Sergio Pena
+1 I like the idea. It's hard to upgrade our libraries to newer ones when other components break due to Sentry being a plugin. I was once thought of using different jars versions per plugin (i.e. guava11 on hdfs, guava14 on hive and the server, etc), but that is too much to do and not good. I like

Re: Shading of conflicting dependencies

2017-11-15 Thread Alexander Kolbasov
Agreed, this would be a very useful thing to do. I remember spending a lot of time trying to make Sentry work with DataNucleus 4 - the problem was that e2e tests combine Sentry with Hive in the same JVM and this created a conflict on the DataNucleus libraries and test failures. Looking at the HBas

Shading of conflicting dependencies

2017-11-15 Thread Brian Towles
Howdy all, An issue that keeps coming up seems to be the conflict of dependency versions between Sentry and the components it is plugging into. A current example of this impact is Google Guava with hive2 using v14 and Impala using v11 while Sentry needs to have at least v14 in order to fix some bu

Re: Review Request 63668: SENTRY-2038 - Some ShellCommand improvements

2017-11-15 Thread Colm O hEigeartaigh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63668/ --- (Updated Nov. 15, 2017, 11:04 a.m.) Review request for sentry. Changes --