You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Brock Noland <br...@cloudera.com> on 2014/02/12 22:21:29 UTC
Review Request 18028: SENTRY-99 - Create infrastructure for notification
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18028/
-----------------------------------------------------------
Review request for sentry and Shreepadma Venugopalan.
Bugs: SENTRY-99
https://issues.apache.org/jira/browse/SENTRY-99
Repository: sentry
Description
-------
Implements the basic notification infrastructure.
Diffs
-----
pom.xml 31eca9e
sentry-provider/sentry-provider-db/pom.xml 6a301d8
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationContext.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 2671ffc
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 95d5005
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SequenceIdGenerator.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ProcessorFactory.java 0ba1ace
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationContext.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java PRE-CREATION
Diff: https://reviews.apache.org/r/18028/diff/
Testing
-------
New unit test pass.
Thanks,
Brock Noland
Re: Review Request 18028: SENTRY-99 - Create infrastructure for notification
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18028/
-----------------------------------------------------------
(Updated Feb. 26, 2014, 12:31 a.m.)
Review request for sentry and Shreepadma Venugopalan.
Changes
-------
Patch has been +1'ed but here is the final update.
Bugs: SENTRY-99
https://issues.apache.org/jira/browse/SENTRY-99
Repository: sentry
Description
-------
Implements the basic notification infrastructure.
Diffs (updated)
-----
pom.xml 423623a
sentry-provider/sentry-provider-db/pom.xml 5181988
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/SentryUserException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CommitContext.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryAlreadyExistsException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryNoSuchObjectException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java a0325da
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryConfigurationException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java a923424
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 95d5005
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ProcessorFactory.java 0ba1ace
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Status.java 5fba2a1
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java PRE-CREATION
Diff: https://reviews.apache.org/r/18028/diff/
Testing
-------
New unit test pass.
Thanks,
Brock Noland
Re: Review Request 18028: SENTRY-99 - Create infrastructure for notification
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
> On Feb. 25, 2014, 5:24 p.m., Brock Noland wrote:
> > Not sure if there is additional feedback or should we commit?
I can commit provided there's no additional feedback. Lenni?
- Shreepadma
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18028/#review35417
-----------------------------------------------------------
On Feb. 24, 2014, 6:17 p.m., Brock Noland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18028/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2014, 6:17 p.m.)
>
>
> Review request for sentry and Shreepadma Venugopalan.
>
>
> Bugs: SENTRY-99
> https://issues.apache.org/jira/browse/SENTRY-99
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Implements the basic notification infrastructure.
>
>
> Diffs
> -----
>
> pom.xml 423623a
> sentry-provider/sentry-provider-db/pom.xml 5181988
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/SentryUserException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CommitContext.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryAlreadyExistsException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryNoSuchObjectException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java a0325da
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryConfigurationException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java a923424
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 95d5005
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ProcessorFactory.java 0ba1ace
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Status.java 5fba2a1
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/18028/diff/
>
>
> Testing
> -------
>
> New unit test pass.
>
>
> Thanks,
>
> Brock Noland
>
>
Re: Review Request 18028: SENTRY-99 - Create infrastructure for notification
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18028/#review35417
-----------------------------------------------------------
Not sure if there is additional feedback or should we commit?
- Brock Noland
On Feb. 24, 2014, 6:17 p.m., Brock Noland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18028/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2014, 6:17 p.m.)
>
>
> Review request for sentry and Shreepadma Venugopalan.
>
>
> Bugs: SENTRY-99
> https://issues.apache.org/jira/browse/SENTRY-99
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Implements the basic notification infrastructure.
>
>
> Diffs
> -----
>
> pom.xml 423623a
> sentry-provider/sentry-provider-db/pom.xml 5181988
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/SentryUserException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CommitContext.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryAlreadyExistsException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryNoSuchObjectException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java a0325da
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryConfigurationException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java a923424
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 95d5005
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ProcessorFactory.java 0ba1ace
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Status.java 5fba2a1
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/18028/diff/
>
>
> Testing
> -------
>
> New unit test pass.
>
>
> Thanks,
>
> Brock Noland
>
>
Re: Review Request 18028: SENTRY-99 - Create infrastructure for notification
Posted by Brock Noland <br...@cloudera.com>.
> On Feb. 25, 2014, 9:13 p.m., Lenni Kuff wrote:
> >
Thank you! I will upload the patch with these changes so Shreepadma can commit.
- Brock
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18028/#review35460
-----------------------------------------------------------
On Feb. 24, 2014, 6:17 p.m., Brock Noland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18028/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2014, 6:17 p.m.)
>
>
> Review request for sentry and Shreepadma Venugopalan.
>
>
> Bugs: SENTRY-99
> https://issues.apache.org/jira/browse/SENTRY-99
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Implements the basic notification infrastructure.
>
>
> Diffs
> -----
>
> pom.xml 423623a
> sentry-provider/sentry-provider-db/pom.xml 5181988
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/SentryUserException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CommitContext.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryAlreadyExistsException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryNoSuchObjectException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java a0325da
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryConfigurationException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java a923424
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 95d5005
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ProcessorFactory.java 0ba1ace
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Status.java 5fba2a1
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/18028/diff/
>
>
> Testing
> -------
>
> New unit test pass.
>
>
> Thanks,
>
> Brock Noland
>
>
Re: Review Request 18028: SENTRY-99 - Create infrastructure for notification
Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18028/#review35460
-----------------------------------------------------------
Ship it!
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CommitContext.java
<https://reviews.apache.org/r/18028/#comment65990>
To enforce that this is a UUID, how about just accepting a UUID object as the parameter rather than a string?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/18028/#comment65992>
comment on use of sequence ID
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/18028/#comment65991>
might be a bit more clear to have a method called:
synchronized long incrementGetSequenceId() {
}
that would make it a bit more clear that the sequence ID should only be incremented in a single place.
- Lenni Kuff
On Feb. 24, 2014, 6:17 p.m., Brock Noland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18028/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2014, 6:17 p.m.)
>
>
> Review request for sentry and Shreepadma Venugopalan.
>
>
> Bugs: SENTRY-99
> https://issues.apache.org/jira/browse/SENTRY-99
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Implements the basic notification infrastructure.
>
>
> Diffs
> -----
>
> pom.xml 423623a
> sentry-provider/sentry-provider-db/pom.xml 5181988
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/SentryUserException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CommitContext.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryAlreadyExistsException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryNoSuchObjectException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java a0325da
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryConfigurationException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java a923424
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 95d5005
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ProcessorFactory.java 0ba1ace
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Status.java 5fba2a1
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/18028/diff/
>
>
> Testing
> -------
>
> New unit test pass.
>
>
> Thanks,
>
> Brock Noland
>
>
Re: Review Request 18028: SENTRY-99 - Create infrastructure for notification
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18028/
-----------------------------------------------------------
(Updated Feb. 24, 2014, 6:17 p.m.)
Review request for sentry and Shreepadma Venugopalan.
Changes
-------
Remove one trailing space.
Bugs: SENTRY-99
https://issues.apache.org/jira/browse/SENTRY-99
Repository: sentry
Description
-------
Implements the basic notification infrastructure.
Diffs (updated)
-----
pom.xml 423623a
sentry-provider/sentry-provider-db/pom.xml 5181988
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/SentryUserException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CommitContext.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryAlreadyExistsException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryNoSuchObjectException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java a0325da
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryConfigurationException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java a923424
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 95d5005
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ProcessorFactory.java 0ba1ace
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Status.java 5fba2a1
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java PRE-CREATION
Diff: https://reviews.apache.org/r/18028/diff/
Testing
-------
New unit test pass.
Thanks,
Brock Noland
Re: Review Request 18028: SENTRY-99 - Create infrastructure for notification
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18028/
-----------------------------------------------------------
(Updated Feb. 24, 2014, 6:16 p.m.)
Review request for sentry and Shreepadma Venugopalan.
Changes
-------
Updates based on review.
Bugs: SENTRY-99
https://issues.apache.org/jira/browse/SENTRY-99
Repository: sentry
Description
-------
Implements the basic notification infrastructure.
Diffs (updated)
-----
pom.xml 423623a
sentry-provider/sentry-provider-db/pom.xml 5181988
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/SentryUserException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CommitContext.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryAlreadyExistsException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryNoSuchObjectException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java a0325da
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryConfigurationException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java a923424
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 95d5005
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ProcessorFactory.java 0ba1ace
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Status.java 5fba2a1
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java PRE-CREATION
Diff: https://reviews.apache.org/r/18028/diff/
Testing
-------
New unit test pass.
Thanks,
Brock Noland
Re: Review Request 18028: SENTRY-99 - Create infrastructure for notification
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18028/#review35090
-----------------------------------------------------------
Ship it!
LGTM.
- Shreepadma Venugopalan
On Feb. 20, 2014, 9:38 p.m., Brock Noland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18028/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2014, 9:38 p.m.)
>
>
> Review request for sentry and Shreepadma Venugopalan.
>
>
> Bugs: SENTRY-99
> https://issues.apache.org/jira/browse/SENTRY-99
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Implements the basic notification infrastructure.
>
>
> Diffs
> -----
>
> pom.xml 423623a
> sentry-provider/sentry-provider-db/pom.xml 5181988
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CommitContext.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryAlreadyExistsException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryNoSuchObjectException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java a0325da
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryUserException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SequenceIdGenerator.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java a923424
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 95d5005
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ProcessorFactory.java 0ba1ace
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Status.java 5fba2a1
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationContext.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSequenceIdGenerator.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/18028/diff/
>
>
> Testing
> -------
>
> New unit test pass.
>
>
> Thanks,
>
> Brock Noland
>
>
Re: Review Request 18028: SENTRY-99 - Create infrastructure for notification
Posted by Brock Noland <br...@cloudera.com>.
> On Feb. 24, 2014, 5:11 p.m., Brock Noland wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SequenceIdGenerator.java, line 21
> > <https://reviews.apache.org/r/18028/diff/5/?file=499361#file499361line21>
> >
> > I don't see why it has to be a singleton as long the server creates only a single instance.
> >
> > We can actually remove the synchronized aspect of that method since synchronization is done at the caller.
This class is removed in the new patch.
> On Feb. 24, 2014, 5:11 p.m., Brock Noland wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java, line 99
> > <https://reviews.apache.org/r/18028/diff/5/?file=499365#file499365line99>
> >
> > I don't think it's a concern since in the scenario when it's thrown is non-recoverable. We want to bail and I am only using the catch block to add the notification handler to the error message.
Added SentryConfigurationException for this.
- Brock
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18028/#review35290
-----------------------------------------------------------
On Feb. 20, 2014, 9:38 p.m., Brock Noland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18028/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2014, 9:38 p.m.)
>
>
> Review request for sentry and Shreepadma Venugopalan.
>
>
> Bugs: SENTRY-99
> https://issues.apache.org/jira/browse/SENTRY-99
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Implements the basic notification infrastructure.
>
>
> Diffs
> -----
>
> pom.xml 423623a
> sentry-provider/sentry-provider-db/pom.xml 5181988
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CommitContext.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryAlreadyExistsException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryNoSuchObjectException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java a0325da
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryUserException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SequenceIdGenerator.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java a923424
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 95d5005
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ProcessorFactory.java 0ba1ace
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Status.java 5fba2a1
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationContext.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSequenceIdGenerator.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/18028/diff/
>
>
> Testing
> -------
>
> New unit test pass.
>
>
> Thanks,
>
> Brock Noland
>
>
Re: Review Request 18028: SENTRY-99 - Create infrastructure for notification
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18028/#review35290
-----------------------------------------------------------
Thank you Lenni for the feedback! I'll be posting a new patch shortly.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CommitContext.java
<https://reviews.apache.org/r/18028/#comment65750>
Will do.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/18028/#comment65751>
Yes this is wrong. It should if (error) rollback();
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/18028/#comment65753>
I think we want to perform the commit and generation of the sequence id in a mutex. Otherwise you would have a (small) race between commit and generation of the seq id.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SequenceIdGenerator.java
<https://reviews.apache.org/r/18028/#comment65756>
I don't see why it has to be a singleton as long the server creates only a single instance.
We can actually remove the synchronized aspect of that method since synchronization is done at the caller.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java
<https://reviews.apache.org/r/18028/#comment65757>
Yeah I thought about that as well. At this point let's keep it simple. If this becomes a bottleneck we can start calling these from a thread pool. I don't think the semantics change.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
<https://reviews.apache.org/r/18028/#comment65758>
I don't think it's a concern since in the scenario when it's thrown is non-recoverable. We want to bail and I am only using the catch block to add the notification handler to the error message.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
<https://reviews.apache.org/r/18028/#comment65759>
Follow-on work is required here as alterSentryRoleDeleteGroups is a stubb.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
<https://reviews.apache.org/r/18028/#comment65749>
It's actually not implemented as the method below the TODO is not correct for list_sentry_roles_by_group.
- Brock Noland
On Feb. 20, 2014, 9:38 p.m., Brock Noland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18028/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2014, 9:38 p.m.)
>
>
> Review request for sentry and Shreepadma Venugopalan.
>
>
> Bugs: SENTRY-99
> https://issues.apache.org/jira/browse/SENTRY-99
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Implements the basic notification infrastructure.
>
>
> Diffs
> -----
>
> pom.xml 423623a
> sentry-provider/sentry-provider-db/pom.xml 5181988
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CommitContext.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryAlreadyExistsException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryNoSuchObjectException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java a0325da
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryUserException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SequenceIdGenerator.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java a923424
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 95d5005
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ProcessorFactory.java 0ba1ace
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Status.java 5fba2a1
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationContext.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSequenceIdGenerator.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/18028/diff/
>
>
> Testing
> -------
>
> New unit test pass.
>
>
> Thanks,
>
> Brock Noland
>
>
Re: Review Request 18028: SENTRY-99 - Create infrastructure for notification
Posted by Lenni Kuff <ls...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18028/#review35287
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CommitContext.java
<https://reviews.apache.org/r/18028/#comment65736>
I don't think this comment really describes the purpose of this class.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CommitContext.java
<https://reviews.apache.org/r/18028/#comment65735>
What if two servers start at the same time? I know it's rare, but perhaps a GUID would be more unique than a timestamp.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/18028/#comment65737>
Curious - It seems counter intuitive that if there is no error the transaction is rolled back. Couldn't this also lead to cases where the transaction was opened and never rolled back (ex - line 108 throws an exception).
Is this what you want? Perhaps a comment would be useful.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/18028/#comment65742>
"Synchronized due to sequence id generation"
Shouldn't the ID generator be thread safe?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SequenceIdGenerator.java
<https://reviews.apache.org/r/18028/#comment65739>
Should this be a singleton class? I'm not sure why this class knows anything about the server start time, it seems like those two things are independent. Perhaps just name the parameter "initialValue"?
Also, would it be better to use an AtomicLong instead of synchronizing?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java
<https://reviews.apache.org/r/18028/#comment65743>
... will be passed a CommitContext.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java
<https://reviews.apache.org/r/18028/#comment65744>
Comment?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java
<https://reviews.apache.org/r/18028/#comment65748>
Maybe not something to worry about now, but if you have a number of subscribers this could become a bottle neck because you must serially go through all handlers and (probably) make a possibly long running RPC. I'm not sure what impact that has on the rest of the system (might be useful to mention that in the comment on the class).
One option would be to use a thread pool and invoke these asynchronously.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
<https://reviews.apache.org/r/18028/#comment65745>
Do you want createHandler to throw unchecked exceptions (IllegalStateException)
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
<https://reviews.apache.org/r/18028/#comment65747>
Remove TODO?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
<https://reviews.apache.org/r/18028/#comment65746>
Remove TODO?
- Lenni Kuff
On Feb. 20, 2014, 9:38 p.m., Brock Noland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18028/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2014, 9:38 p.m.)
>
>
> Review request for sentry and Shreepadma Venugopalan.
>
>
> Bugs: SENTRY-99
> https://issues.apache.org/jira/browse/SENTRY-99
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Implements the basic notification infrastructure.
>
>
> Diffs
> -----
>
> pom.xml 423623a
> sentry-provider/sentry-provider-db/pom.xml 5181988
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CommitContext.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryAlreadyExistsException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryNoSuchObjectException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java a0325da
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryUserException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SequenceIdGenerator.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java a923424
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 95d5005
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ProcessorFactory.java 0ba1ace
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Status.java 5fba2a1
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationContext.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSequenceIdGenerator.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/18028/diff/
>
>
> Testing
> -------
>
> New unit test pass.
>
>
> Thanks,
>
> Brock Noland
>
>
Re: Review Request 18028: SENTRY-99 - Create infrastructure for notification
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18028/
-----------------------------------------------------------
(Updated Feb. 20, 2014, 9:38 p.m.)
Review request for sentry and Shreepadma Venugopalan.
Changes
-------
Addressed feedback.
Bugs: SENTRY-99
https://issues.apache.org/jira/browse/SENTRY-99
Repository: sentry
Description
-------
Implements the basic notification infrastructure.
Diffs (updated)
-----
pom.xml 423623a
sentry-provider/sentry-provider-db/pom.xml 5181988
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CommitContext.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryAlreadyExistsException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryNoSuchObjectException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java a0325da
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryUserException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SequenceIdGenerator.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java a923424
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 95d5005
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ProcessorFactory.java 0ba1ace
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Status.java 5fba2a1
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationContext.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSequenceIdGenerator.java PRE-CREATION
Diff: https://reviews.apache.org/r/18028/diff/
Testing
-------
New unit test pass.
Thanks,
Brock Noland
Re: Review Request 18028: SENTRY-99 - Create infrastructure for notification
Posted by Brock Noland <br...@cloudera.com>.
> On Feb. 20, 2014, 8:28 p.m., Shreepadma Venugopalan wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, line 156
> > <https://reviews.apache.org/r/18028/diff/4/?file=499055#file499055line156>
> >
> > We don't throw this exception today. Is this for a follow up JIRA?
Correct, I was basically stubbing out the methods required since this jira isn't about the backend api.
> On Feb. 20, 2014, 8:28 p.m., Shreepadma Venugopalan wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java, line 121
> > <https://reviews.apache.org/r/18028/diff/4/?file=499061#file499061line121>
> >
> > This should be part of transaction commit, else we can't guarantee the sequence numbers reflect the commit order.
Good point, fixed in next rev of the patch.
- Brock
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18028/#review35056
-----------------------------------------------------------
On Feb. 20, 2014, 6:58 p.m., Brock Noland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18028/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2014, 6:58 p.m.)
>
>
> Review request for sentry and Shreepadma Venugopalan.
>
>
> Bugs: SENTRY-99
> https://issues.apache.org/jira/browse/SENTRY-99
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Implements the basic notification infrastructure.
>
>
> Diffs
> -----
>
> pom.xml 423623a
> sentry-provider/sentry-provider-db/pom.xml 5181988
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryAlreadyExistsException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryNoSuchObjectException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java a0325da
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryUserException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationContext.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java a923424
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 95d5005
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SequenceIdGenerator.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ProcessorFactory.java 0ba1ace
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Status.java 5fba2a1
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationContext.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSequenceIdGenerator.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/18028/diff/
>
>
> Testing
> -------
>
> New unit test pass.
>
>
> Thanks,
>
> Brock Noland
>
>
Re: Review Request 18028: SENTRY-99 - Create infrastructure for notification
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18028/#review35056
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
<https://reviews.apache.org/r/18028/#comment65444>
We don't throw this exception today. Is this for a follow up JIRA?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
<https://reviews.apache.org/r/18028/#comment65448>
This should be part of transaction commit, else we can't guarantee the sequence numbers reflect the commit order.
- Shreepadma Venugopalan
On Feb. 20, 2014, 6:58 p.m., Brock Noland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18028/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2014, 6:58 p.m.)
>
>
> Review request for sentry and Shreepadma Venugopalan.
>
>
> Bugs: SENTRY-99
> https://issues.apache.org/jira/browse/SENTRY-99
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Implements the basic notification infrastructure.
>
>
> Diffs
> -----
>
> pom.xml 423623a
> sentry-provider/sentry-provider-db/pom.xml 5181988
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryAlreadyExistsException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryNoSuchObjectException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java a0325da
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryUserException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationContext.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java a923424
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 95d5005
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SequenceIdGenerator.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ProcessorFactory.java 0ba1ace
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Status.java 5fba2a1
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationContext.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSequenceIdGenerator.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/18028/diff/
>
>
> Testing
> -------
>
> New unit test pass.
>
>
> Thanks,
>
> Brock Noland
>
>
Re: Review Request 18028: SENTRY-99 - Create infrastructure for notification
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18028/
-----------------------------------------------------------
(Updated Feb. 20, 2014, 6:58 p.m.)
Review request for sentry and Shreepadma Venugopalan.
Changes
-------
Rebased patch on trunk.
Bugs: SENTRY-99
https://issues.apache.org/jira/browse/SENTRY-99
Repository: sentry
Description
-------
Implements the basic notification infrastructure.
Diffs (updated)
-----
pom.xml 423623a
sentry-provider/sentry-provider-db/pom.xml 5181988
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryAlreadyExistsException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryNoSuchObjectException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java a0325da
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryUserException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationContext.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java a923424
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 95d5005
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SequenceIdGenerator.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ProcessorFactory.java 0ba1ace
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/Status.java 5fba2a1
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationContext.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSequenceIdGenerator.java PRE-CREATION
Diff: https://reviews.apache.org/r/18028/diff/
Testing
-------
New unit test pass.
Thanks,
Brock Noland
Re: Review Request 18028: SENTRY-99 - Create infrastructure for notification
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18028/
-----------------------------------------------------------
(Updated Feb. 12, 2014, 9:25 p.m.)
Review request for sentry and Shreepadma Venugopalan.
Changes
-------
Finally remove ws
Bugs: SENTRY-99
https://issues.apache.org/jira/browse/SENTRY-99
Repository: sentry
Description
-------
Implements the basic notification infrastructure.
Diffs (updated)
-----
pom.xml 31eca9e
sentry-provider/sentry-provider-db/pom.xml 6a301d8
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationContext.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 2671ffc
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 95d5005
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SequenceIdGenerator.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ProcessorFactory.java 0ba1ace
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationContext.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java PRE-CREATION
Diff: https://reviews.apache.org/r/18028/diff/
Testing
-------
New unit test pass.
Thanks,
Brock Noland
Re: Review Request 18028: SENTRY-99 - Create infrastructure for notification
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18028/
-----------------------------------------------------------
(Updated Feb. 12, 2014, 9:23 p.m.)
Review request for sentry and Shreepadma Venugopalan.
Changes
-------
Remove ws
Bugs: SENTRY-99
https://issues.apache.org/jira/browse/SENTRY-99
Repository: sentry
Description
-------
Implements the basic notification infrastructure.
Diffs (updated)
-----
pom.xml 31eca9e
sentry-provider/sentry-provider-db/pom.xml 6a301d8
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationContext.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandler.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/NotificationHandlerInvoker.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java 2671ffc
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessorFactory.java 95d5005
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SequenceIdGenerator.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ProcessorFactory.java 0ba1ace
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationContext.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestNotificationHandlerInvoker.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryPolicyStoreProcessor.java PRE-CREATION
Diff: https://reviews.apache.org/r/18028/diff/
Testing
-------
New unit test pass.
Thanks,
Brock Noland