Re: Review Request 49976: SENTRY-1401 In V2, show role grant group groupname should not throw an exception if group doesnt exist in db.

2016-07-12 Thread Ke Jia
> On July 13, 2016, 5:32 a.m., Dapeng Sun wrote: > > Better to refine TestGrantUserToRole like SENTRY-320 did. Thanks dapeng comment. SENTRY-1402 will add the TestGrantUserToRole.java in v2. Then I will refine TestGrantUSerToRole like SENTRY-320 did. - Ke ---

Re: Review Request 49976: SENTRY-1401 In V2, show role grant group groupname should not throw an exception if group doesnt exist in db.

2016-07-12 Thread Dapeng Sun
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49976/#review142008 --- Better to refine TestGrantUserToRole like SENTRY-320 did. sentry

Review Request 49976: SENTRY-1401 In V2, show role grant group groupname should not throw an exception if group doesnt exist in db.

2016-07-12 Thread Ke Jia
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49976/ --- Review request for sentry and Dapeng Sun. Bugs: SENTRY-1401 https://issues.

Re: Review Request 49777: SENTRY-1321: Implement HMSFollower in Sentry service which reads the NotificationLog entries

2016-07-12 Thread Sravya Tirukkovalur
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49777/ --- (Updated July 13, 2016, 12:42 a.m.) Review request for sentry and Hao Hao. Re

Re: [2/4] sentry git commit: SENTRY-1329: Adapt SentryMetaStorePostEventListener to write HMS notification logs

2016-07-12 Thread Sravya Tirukkovalur
Sorry, pushed this change by mistake while pushing Sentry-1317. Fixed it now. Thanks! On Tue, Jul 12, 2016 at 1:06 PM, wrote: > SENTRY-1329: Adapt SentryMetaStorePostEventListener to write HMS > notification logs > > Also, > 1. Implementing the SentryJSONMessageFactory to add custom information

Re: Review Request 49397: SENTRY-1329: Adapt SentryMetaStorePostEventListener to write HMS notification logs

2016-07-12 Thread Sravya Tirukkovalur
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49397/ --- (Updated July 12, 2016, 8:02 p.m.) Review request for sentry, Anne Yu, Colin Mc

Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

2016-07-12 Thread Sravya Tirukkovalur
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49738/#review141940 --- Ship it! Ship It! - Sravya Tirukkovalur On July 11, 2016, 6:

Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

2016-07-12 Thread Sravya Tirukkovalur
> On July 12, 2016, 4:36 p.m., Sravya Tirukkovalur wrote: > > Thanks Colin! Code mostly looks good to me ( I have one unresolved > > comment). I think it is ok to add e2e testing in a follow on patch, is > > there a jira tracking it? Would you be able to add a unit test for Fencer > > as part

Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

2016-07-12 Thread Sravya Tirukkovalur
> On July 8, 2016, 8:17 p.m., Sravya Tirukkovalur wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java, > > line 201 > > > > > > Do we want to inst

Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

2016-07-12 Thread Colin McCabe
> On July 8, 2016, 8:17 p.m., Sravya Tirukkovalur wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java, > > line 114 > > > > > > Signature does not

Re: Review Request 49397: SENTRY-1329: Adapt SentryMetaStorePostEventListener to write HMS notification logs

2016-07-12 Thread Sravya Tirukkovalur
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49397/ --- (Updated July 12, 2016, 5:28 p.m.) Review request for sentry, Anne Yu, Colin Mc

Re: Review Request 49397: SENTRY-1329: Adapt SentryMetaStorePostEventListener to write HMS notification logs

2016-07-12 Thread Sravya Tirukkovalur
> On July 11, 2016, 8:33 p.m., Hao Hao wrote: > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestSentryListenerSentryDeserializer.java, > > line 339 > > > > > > Test for drop

Re: Review Request 49397: SENTRY-1329: Adapt SentryMetaStorePostEventListener to write HMS notification logs

2016-07-12 Thread Sravya Tirukkovalur
> On July 11, 2016, 8:33 p.m., Hao Hao wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageDeserializer.java, > > line 100 > > > > > > Why

Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

2016-07-12 Thread Sravya Tirukkovalur
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49738/#review141910 --- Thanks Colin! Code mostly looks good to me ( I have one unresolved

Re: Review Request 49738: SENTRY-1317: Implement fencing required for active/passive

2016-07-12 Thread Sravya Tirukkovalur
> On July 8, 2016, 8:17 p.m., Sravya Tirukkovalur wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/Fencer.java, > > line 201 > > > > > > Do we want to inst