[DISCUSS] Allowing Java 8 features in Sentry

2018-02-22 Thread Alexander Kolbasov
Hello Sentry hackers, We are now compiling Sentry with Java 8. I would like to propose allowing Java 8 features in Sentry code. As of now it is difficult to enforce strict Java 7 compatibility, so we may just as well embrace the reality and start using Java 8 which hopefully will improve the

Re: [DISCUSS] Allowing Java 8 features in Sentry

2018-02-22 Thread Na Li
+1 - Lina On Thu, Feb 22, 2018 at 12:19 PM, Alexander Kolbasov wrote: > Hello Sentry hackers, > > We are now compiling Sentry with Java 8. I would like to propose allowing > Java 8 features in Sentry code. As of now it is difficult to enforce strict > Java 7 compatibility,

Re: Review Request 65704: SENTRY-2143: Table renames should synchronize with Sentry

2018-02-22 Thread Na Li via Review Board
> On Feb. 22, 2018, 5:52 p.m., Sergio Pena wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java > > Lines 135-136 (patched) > >

[DISCUSS] Adjustments to the commit process

2018-02-22 Thread Alexander Kolbasov
Hello everyone, I would like to propose an adjustment to the commit process in Sentry project. The idea is to require that commit should not be done by the person providing the change but by some other committer. This committer's responsibility is to ensure that all code review concerns were

Re: Review Request 65704: SENTRY-2143: Table renames should synchronize with Sentry

2018-02-22 Thread Sergio Pena via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65704/#review198124 ---

Re: Review Request 65704: SENTRY-2143: Table renames should synchronize with Sentry

2018-02-22 Thread kalyan kumar kalvagadda via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65704/#review198129 ---

Re: [DISCUSS] Adjustments to the commit process

2018-02-22 Thread Stephen Moist
Sounds reasonable to me as long as they can get someone to do the commit in a reasonable timeframe. I wouldn’t want to have to wait days for it to get in after it has been properly reviewed. > On Feb 22, 2018, at 12:22 PM, Alexander Kolbasov wrote: > > Hello everyone, >

Re: Review Request 65704: SENTRY-2143: Table renames should synchronize with Sentry

2018-02-22 Thread Na Li via Review Board
> On Feb. 22, 2018, 5:52 p.m., Sergio Pena wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Lines 2412-2413 (patched) > > > > > > Is

Re: Review Request 65704: SENTRY-2143: Table renames should synchronize with Sentry

2018-02-22 Thread Steve Moist via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65704/#review198135 ---

Re: Review Request 65704: SENTRY-2143: Table renames should synchronize with Sentry

2018-02-22 Thread Na Li via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65704/ --- (Updated Feb. 22, 2018, 6:55 p.m.) Review request for sentry, Alexander

Re: [DISCUSS] Allowing Java 8 features in Sentry

2018-02-22 Thread Stephen Moist
Sounds good to me, do we have plans to migrate to the new date api? > On Feb 22, 2018, at 12:44 PM, Na Li wrote: > > +1 - Lina > > On Thu, Feb 22, 2018 at 12:19 PM, Alexander Kolbasov > wrote: > >> Hello Sentry hackers, >> >> We are now compiling

Re: Review Request 65704: SENTRY-2143: Table renames should synchronize with Sentry

2018-02-22 Thread Alexander Kolbasov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65704/#review198133 --- Does the test show the problem before the fix?

Re: [DISCUSS] Allowing Java 8 features in Sentry

2018-02-22 Thread Sergio Pena
It sounds good to me. +1 On Thu, Feb 22, 2018 at 1:30 PM, Stephen Moist wrote: > Sounds good to me, do we have plans to migrate to the new date api? > > > On Feb 22, 2018, at 12:44 PM, Na Li wrote: > > > > +1 - Lina > > > > On Thu, Feb 22, 2018 at

Re: [DISCUSS] Adjustments to the commit process

2018-02-22 Thread Sergio Pena
How is this committer going to be assigned? This might lead to some complications if the committer assigned leave for vacations afterward and the community is not notified. It will end up delaying the commits and the author (being a committer) won't be able to commit the patch due to this process.

Re: [DISCUSS] Adjustments to the commit process

2018-02-22 Thread Alexander Kolbasov
For assigning committers I think this may be a simple informal request - for example to one of the reviewers or to someone else to volunteer. It may delay commits a bit indeed, but I don't think it will be a problem. The problem I am trying to address is the quality of the review process. Suppose

Review Request 65768: SENTRY-2144: Table Rename should allow database name change

2018-02-22 Thread Na Li via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65768/ --- Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and

Re: Review Request 65704: SENTRY-2143: Table renames should synchronize with Sentry

2018-02-22 Thread Na Li via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65704/ --- (Updated Feb. 23, 2018, 7 a.m.) Review request for sentry, Alexander Kolbasov,

Re: Review Request 65704: SENTRY-2143: Table renames should synchronize with Sentry

2018-02-22 Thread Na Li via Review Board
> On Feb. 22, 2018, 7:43 p.m., Steve Moist wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java > > Lines 141 (patched) > > > >

Re: Review Request 65704: SENTRY-2143: Table renames should synchronize with Sentry

2018-02-22 Thread Na Li via Review Board
> On Feb. 22, 2018, 7:43 p.m., Steve Moist wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java > > Lines 157 (patched) > > > >

Re: Review Request 65704: SENTRY-2143: Table renames should synchronize with Sentry

2018-02-22 Thread Alexander Kolbasov
> On Feb. 22, 2018, 6:56 p.m., Alexander Kolbasov wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java > > Lines 146 (patched) > >

Re: Review Request 65704: SENTRY-2143: Table renames should synchronize with Sentry

2018-02-22 Thread Na Li via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65704/ --- (Updated Feb. 23, 2018, 6:56 a.m.) Review request for sentry, Alexander

Re: Review Request 65704: SENTRY-2143: Table renames should synchronize with Sentry

2018-02-22 Thread Na Li via Review Board
> On Feb. 21, 2018, 5:13 p.m., Sergio Pena wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java > > Lines 135 (patched) > > > >

Re: Review Request 65704: SENTRY-2143: Table renames should synchronize with Sentry

2018-02-22 Thread Na Li via Review Board
> On Feb. 22, 2018, 6:56 p.m., Alexander Kolbasov wrote: > > Does the test show the problem before the fix? The original test did not fail because the HMSFollower polling interval was configured to be 50 ms. I increased it to be 2 seconds, and the test failed with and without the fix. Then I

Re: Review Request 65704: SENTRY-2143: Table renames should synchronize with Sentry

2018-02-22 Thread Na Li via Review Board
> On Feb. 22, 2018, 6:56 p.m., Alexander Kolbasov wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java > > Line 96 (original), 98 (patched) > >

Re: Review Request 65704: SENTRY-2143: Table renames should synchronize with Sentry

2018-02-22 Thread Na Li via Review Board
> On Feb. 22, 2018, 6:56 p.m., Alexander Kolbasov wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java > > Lines 146 (patched) > >