Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer

2017-04-19 Thread Vamsee Yarlagadda
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58221/#review172434 --- LGTM otherwise.

Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer

2017-04-19 Thread Na Li
> On April 19, 2017, 9 p.m., Alexander Kolbasov wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > > Lines 321 (patched) > > > > > > I think you may

Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer

2017-04-19 Thread Na Li
> On April 19, 2017, 9:08 p.m., Alexander Kolbasov wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > > Lines 274 (patched) > > > > > > It seems that

Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer

2017-04-19 Thread Alexander Kolbasov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58221/#review172410 ---

Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer

2017-04-19 Thread Alexander Kolbasov
> On April 14, 2017, 7:04 p.m., kalyan kumar kalvagadda wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > > Lines 221 (patched) > > > > > >

Re: Clarification for MPath changes for SENTRY-1587

2017-04-19 Thread Na Li
Sasha, For now, the path entry in DB is owned by each Authz object. The DB change Kalyan made does not guarantee that with the same path value, there is only one instance of such path entry in DB in https://issues.apache.org/jira/browse/SENTRY-1629 (He will address this later). Therefore, there

Re: Review Request 58412: SENTRY-1587: Refactor SentryStore transaction to persist a single path transcation bundled with corresponding delta path change

2017-04-19 Thread Na Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58412/#review172250 ---

Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer

2017-04-19 Thread Na Li
> On April 14, 2017, 7:04 p.m., kalyan kumar kalvagadda wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > > Line 375 (original), 474 (patched) > > > > > >

Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer

2017-04-19 Thread Vamsee Yarlagadda
> On April 18, 2017, 9:15 p.m., Vamsee Yarlagadda wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java > > Lines 275 (patched) > > > > > > I know it

Re: Clarification for MPath changes for SENTRY-1587

2017-04-19 Thread Hao Hao
Hi Alex, I changed MPath class to include pathID to have correct MPath equality comparison. Since the original equals() definition only compares 'path' which for different MPath object it may have the same value. For pathID (primary key) assignment I change it to be auto-increment by specifying

Clarification for MPath changes for SENTRY-1587

2017-04-19 Thread Alexander Kolbasov
Hao, Can you clarify the changes you propose for MPath class and related package.jdo changes for SENTRY-1587? You suggest changing the identity type from "database" to "application", but the pathId is not initialized in constructor and not assigned anywhere. What is your intention here? Also

Re: [DISCUSS] Merging Sentry HA branch with master

2017-04-19 Thread Alexander Kolbasov
Sergio, thank you for clarifying the Hive version story. Do we have any explicit policy about compatibility with other components and Hive specifically? Do we need to support older versions of Hive or supporting the current released Hive 1.x version is sufficient? At least it seems that we

Re: Review Request 58481: SENTRY-1674 HMSFollower shouldn't print the same value of notification ID multiple times

2017-04-19 Thread Alexander Kolbasov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58481/#review172380 ---

Review Request 58536: SENTRY-1678 Add test class to test refactored refactored config code

2017-04-19 Thread kalyan kumar kalvagadda
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58536/ --- Review request for sentry, Alexander Kolbasov, Hao Hao, Na Li, Sergio Pena,

Re: [DISCUSS] Merging Sentry HA branch with master

2017-04-19 Thread Kalyan Kumar Kalvagadda
I agree with Sergio but I'm not sure if current plan is to release HA functionality. If we plan to make it a release, we need to make sure sentry HA works with one of the versions of Hive where 1. Notification log implementation has the functionality that sentry needs. 2. Supports

Re: Review Request 58221: SENTRY-1649 move HMS follower to runServer

2017-04-19 Thread Na Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58221/ --- (Updated April 19, 2017, 2:23 p.m.) Review request for sentry, Alexander