Re: Review Request 54697: SENTRY-1513: Changed the way the split works when parsing a path and corrected spelling in PathsUpdate

2016-12-13 Thread Alexander Kolbasov
we call split twice where we can split just once and save the result. The first time we split for the if statement, the second - to return the result. - Alexander Kolbasov On Dec. 13, 2016, 9:55 a.m., Jan Hentschel wrote: > > ---

Re: Review Request 54525: SENTRY-1541: toSentryPrivilege() should not copy fields that are not set in the source

2016-12-13 Thread Alexander Kolbasov
removed as well? 2) This relates to SENTRY-1540 since now isMultiActionsSupported() becomes a real code and this changes the existing semantics some. Is it Ok or not? - Alexander Kolbasov On Dec. 12, 2016, 7:57 p.m., Vamsee Yarlagadda wrote

Re: Review Request 54697: SENTRY-1513: Changed the way the split works when parsing a path and corrected spelling in PathsUpdate

2016-12-13 Thread Alexander Kolbasov
> On Dec. 13, 2016, 5:58 p.m., Alexander Kolbasov wrote: > > The change is fine, but the original JIRA was about the fact that we call > > split twice where we can split just once and save the result. The first > > time we split for the if statement, the second - to return

Re: Getting started

2016-12-13 Thread Alexander Kolbasov
Hi Roberta! > On Dec 13, 2016, at 11:04 AM, Roberta Marton wrote: > > I work on Apache Trafodion and have been asked to evaluate integration of > Trafodion with Sentry. > My background includes supporting privileges for Trafodion and am very > familiar with the basics of users, roles, privileg

Re: Review Request 54697: SENTRY-1513: Changed the way the split works when parsing a path and corrected spelling in PathsUpdate

2016-12-14 Thread Alexander Kolbasov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54697/#review159178 --- Ship it! Ship It! - Alexander Kolbasov On Dec. 13, 2016, 9

[DISCUSS] Sentry interactive shell

2016-12-14 Thread Alexander Kolbasov
Inspired by SentryShell I wrote a prototype of interactive Sentry shell where you have an open session and can issue CRUD commands for roles/groups/privileges. I there any interest in making this integrated into Sentry code base?

Re: Review Request 54409: SENTRY-1476: SentryStore is subject to JDQL injection

2016-12-14 Thread Alexander Kolbasov
- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54409/#review159226 --- On Dec. 6, 2016, 5:30 a.m., Alexander Kolbasov wrote: > >

Re: Review Request 54454: SENTRY-1548 Setting GrantOption to UNSET upsets Sentry

2016-12-14 Thread Alexander Kolbasov
> On Dec. 7, 2016, 12:56 a.m., Alexander Kolbasov wrote: > > sentry-service/sentry-service-common/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryGrantOption.java, > > line 16 > > <https://reviews.apache.org/r/54454/diff/1/?file=1

Re: Review Request 54338: SENTRY-1515: Cleanup exception handling in SentryStore

2016-12-14 Thread Alexander Kolbasov
nly place where we use try-with-resource to close timers. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54338/#review159245 ------

Re: Review Request 54454: SENTRY-1548 Setting GrantOption to UNSET upsets Sentry

2016-12-14 Thread Alexander Kolbasov
4> It is a bit confusing for these two functions to share the same name. Also, given that this can be simply written as return privilege.getGrantOption() != TSentryGrantOption.UNSET do we need a function for that at all? - Alexander Kolbasov On Dec. 14, 2

Re: Review Request 54338: SENTRY-1515: Cleanup exception handling in SentryStore

2016-12-14 Thread Alexander Kolbasov
-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java e8d0e2505c5b178cae3d27e7d85caa652f630a2d Diff: https://reviews.apache.org/r/54338/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 54445: SENTRY-1547 - It is possible to create a privilege with all empty fields

2016-12-14 Thread Alexander Kolbasov
automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54445/ > --- > > (Updated Dec. 14, 2016, 10:23 p.m.) > > > Review request for sentry, Alexander Kolbasov, Colin Ma, Hao Hao, Vamsee > Yarlagadda, and Vadim Spector. > > > Repositor

Re: Review Request 54445: SENTRY-1547 - It is possible to create a privilege with all empty fields

2016-12-14 Thread Alexander Kolbasov
changes in SentryPolicyStoreProcessor but they are not in this diff. - Alexander Kolbasov On Dec. 14, 2016, 10:23 p.m., kalyan kumar kalvagadda wrote: > > --- > This is an automatically generated e-mail. To reply, visit

Re: [DISCUSS] Sentry interactive shell

2016-12-14 Thread Alexander Kolbasov
nni Thanks for your input! - Alex. > > On Wed, Dec 14, 2016 at 5:19 PM, Alexander Kolbasov > wrote: > >> Inspired by SentryShell I wrote a prototype of interactive Sentry shell >> where you have an open session and can issue CRUD commands for >> roles/groups/privileges. I there any interest in making this integrated >> into Sentry code base? >>

Review Request 54808: SENTRY-1526 Sentry processed stays alive after being killed

2016-12-15 Thread Alexander Kolbasov
://reviews.apache.org/r/54808/diff/ Testing --- Now process successfully dies after hitting ^C. Thanks, Alexander Kolbasov

Re: Review Request 54525: SENTRY-1541: toSentryPrivilege() should not copy fields that are not set in the source

2016-12-16 Thread Alexander Kolbasov
/54525/#comment230497> Same comment - can we just copy fields and null fields will automatically become null fields in the result. - Alexander Kolbasov On Dec. 16, 2016, 7:51 p.m., Vamsee Yarlagadda wrote: > > --- > T

Re: Review Request 54808: SENTRY-1526 Sentry processed stays alive after being killed

2016-12-16 Thread Alexander Kolbasov
: https://reviews.apache.org/r/54808/#review159478 --- On Dec. 16, 2016, 7:23 a.m., Alexander Kolbasov wrote: > > --- > This is an automatically generated e-

Re: Review Request 54819: SENTRY-1526: Sentry processed stays alive after being killed (Fixing Mismerge) (Alexander Kolbasov via Vamsee Yarlagadda)

2016-12-16 Thread Alexander Kolbasov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54819/#review159491 --- Ship it! Ship It! - Alexander Kolbasov On Dec. 16, 2016, 8

Re: Review Request 52795: SENTRY-1499: Add feature flag for using NotificationLog

2016-12-16 Thread Alexander Kolbasov
is there a need to undo any work? sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java (line 114) <https://reviews.apache.org/r/52795/#comment230560> What is ment by drop here? The method is about creating a table. -

Review Request 55080: SENTRY-1428: Only leader should follow HMS updates

2016-12-29 Thread Alexander Kolbasov
sending special signal that drops leadership status. I also tested shutting down ZK service itself and then restarting it. Thanks, Alexander Kolbasov

Review Request 55092: SENTRY-1581: Provide Log4J metrics reporter

2016-12-29 Thread Alexander Kolbasov
--- Thanks, Alexander Kolbasov

Review Request 55094: SENTRY-1532: Sentry Web UI isn't working

2016-12-29 Thread Alexander Kolbasov
entry-service-server/pom.xml 97bd326b0305303194a965a60b6afd0feefbbe0f Diff: https://reviews.apache.org/r/55094/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 54525: SENTRY-1541/SENTRY-1582: Additional comments to clarify the intent of string manipulation methods in SentryStore.java

2017-01-03 Thread Alexander Kolbasov
e null. sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 1753) <https://reviews.apache.org/r/54525/#comment231571> @return True if the input string represents a NULL string - when it is null, empty or equals @NULL_C

Re: Review Request 54525: SENTRY-1541/SENTRY-1582: Additional comments to clarify the intent of string manipulation methods in SentryStore.java

2017-01-03 Thread Alexander Kolbasov
va/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 1742) <https://reviews.apache.org/r/54525/#comment231575> Please use HTML formatting. - Alexander Kolbasov On Jan. 3, 2017, 8:02 p.m., Vamsee Yarl

Re: Review Request 54525: SENTRY-1541/SENTRY-1582: Additional comments to clarify the intent of string manipulation methods in SentryStore.java

2017-01-03 Thread Alexander Kolbasov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54525/#review160430 --- Ship it! Ship It! - Alexander Kolbasov On Jan. 3, 2017, 8

[DISCUSS] Problem with existing Sentry e2e tests

2017-01-03 Thread Alexander Kolbasov
I think we have a problem with the way e2e tests are currently structured. They run all components in a single JVM - a setup that is never used in practice. As a result, this creates a lot of potential for breaking things due to various library version issues. I've seen a few cases where reasonable

Re: Review Request 55094: SENTRY-1532: Sentry Web UI isn't working

2017-01-03 Thread Alexander Kolbasov
sting --- Thanks, Alexander Kolbasov

Review Request 55190: SENTRY-1583: Refactor ZK/Curator code

2017-01-04 Thread Alexander Kolbasov
active server - new sserver is elected as a leader - Restarting ZK service - a leader is elected once ZK service is back. Thanks, Alexander Kolbasov

Re: Review Request 55094: SENTRY-1532: Sentry Web UI isn't working

2017-01-04 Thread Alexander Kolbasov
------- On Jan. 4, 2017, 12:31 a.m., Alexander Kolbasov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55094/ > -

Re: Review Request 55094: SENTRY-1532: Sentry Web UI isn't working

2017-01-04 Thread Alexander Kolbasov
rking Diffs (updated) - sentry-service/sentry-service-server/pom.xml 97bd326b0305303194a965a60b6afd0feefbbe0f sentry-tests/sentry-tests-solr/pom.xml a60b4eed4dde6c182b061681305013b3bd548cc3 Diff: https://reviews.apache.org/r/55094/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 54338: SENTRY-1515: Cleanup exception handling in SentryStore

2017-01-04 Thread Alexander Kolbasov
makes exceptions more explicit. - Alexander Kolbasov On Dec. 14, 2016, 11:57 p.m., Alexander Kolbasov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 54409: SENTRY-1476: SentryStore is subject to JDQL injection

2017-01-04 Thread Alexander Kolbasov
/TestSentryStore.java 64df6a5655cf2c121cb44f2274369fbe9d70ec83 Diff: https://reviews.apache.org/r/54409/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 55192: SENTRY-1586: [unit test] Race condition between metastore server/client could cause connection refused errors

2017-01-04 Thread Alexander Kolbasov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55192/#review160551 --- Ship it! Ship It! - Alexander Kolbasov On Jan. 5, 2017, 1

Re: Review Request 53920: SENTRY-1534: Oracle supports serializable instead of repeatable-read

2017-01-05 Thread Alexander Kolbasov
/persistent/SentryStore.java f83d72160e1c2d3d1ec1b870ea4b4d5dda1a Diff: https://reviews.apache.org/r/53920/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 54338: SENTRY-1515: Cleanup exception handling in SentryStore

2017-01-05 Thread Alexander Kolbasov
/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java e8d0e2505c5b178cae3d27e7d85caa652f630a2d Diff: https://reviews.apache.org/r/54338/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 53920: SENTRY-1534: Oracle supports serializable instead of repeatable-read

2017-01-05 Thread Alexander Kolbasov
ected by Oracle and you get an error. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53920/#review160634 ------- O

Re: Review Request 53920: SENTRY-1534: Oracle supports serializable instead of repeatable-read

2017-01-05 Thread Alexander Kolbasov
------ On Jan. 5, 2017, 9:04 a.m., Alexander Kolbasov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53920/ >

Re: Review Request 54454: SENTRY-1548 Setting GrantOption to UNSET upsets Sentry

2017-01-05 Thread Alexander Kolbasov
org/r/54454/#comment231811> Do we allow any action as long as it is non-empty? Other code makes certain assumption about valid set of actions. - Alexander Kolbasov On Jan. 5, 2017, 4:50 p.m., kalyan kumar kalvagadda wrote: > > -

Re: Review Request 55190: SENTRY-1583: Refactor ZK/Curator code

2017-01-05 Thread Alexander Kolbasov
is an official limit, but I split long lines. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55190/#review160624 -

Re: Review Request 55190: SENTRY-1583: Refactor ZK/Curator code

2017-01-05 Thread Alexander Kolbasov
SIGUSR2 to processes which changes leadership status (verified via log messages and emtrics) - Killing active server - new sserver is elected as a leader - Restarting ZK service - a leader is elected once ZK service is back. Thanks, Alexander Kolbasov

Re: Review Request 55190: SENTRY-1583: Refactor ZK/Curator code

2017-01-05 Thread Alexander Kolbasov
ava > c796fabc21005479fa6afc687cf1df7af76538a9 > > Diff: https://reviews.apache.org/r/55190/diff/ > > > Testing > --- > > Performed manual testing with two instances of Sentry running. Tried the > following actions: > > - Sending SIGUSR2 to processes which changes leadership status (verified via > log messages and emtrics) > - Killing active server - new sserver is elected as a leader > - Restarting ZK service - a leader is elected once ZK service is back. > > > Thanks, > > Alexander Kolbasov > >

Re: Review Request 55190: SENTRY-1583: Refactor ZK/Curator code

2017-01-05 Thread Alexander Kolbasov
SIGUSR2 to processes which changes leadership status (verified via log messages and emtrics) - Killing active server - new sserver is elected as a leader - Restarting ZK service - a leader is elected once ZK service is back. Thanks, Alexander Kolbasov

Re: Review Request 52795: SENTRY-1499: Add feature flag for using NotificationLog

2017-01-06 Thread Alexander Kolbasov
che.org/r/52795/#comment231957> Seems like other constants are prefixed with 'sentry' - it may be better to follow the convention. - Alexander Kolbasov On Dec. 20, 2016, 12:21 a.m., Hao Hao wrote: > > --- > Thi

Re: Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

2017-01-06 Thread Alexander Kolbasov
e just use changeId as the hashcode? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPermChange.java (line 29) <https://reviews.apache.org/r/54729/#comment230430> Add - Alexander Kolbasov On Dec. 14, 2016, 2:35 a.m., Hao Hao w

Re: Review Request 55190: SENTRY-1583: Refactor ZK/Curator code

2017-01-06 Thread Alexander Kolbasov
generated e-mail. To reply, visit: https://reviews.apache.org/r/55190/#review160806 ------- On Jan. 6, 2017, 6:06 a.m., Alexander Kolbasov wrote: > > --- > This is an automatically

Re: Review Request 55094: SENTRY-1532: Sentry Web UI isn't working

2017-01-09 Thread Alexander Kolbasov
- Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55094/#review160918 --- On Jan. 5, 2017, 12:06 a.m., Alexander Kolbasov wrote: > > -

Re: Review Request 54729: SENTRY-1538: Create schema for storing HMS path change and Sentry permission change.

2017-01-10 Thread Alexander Kolbasov
b/service/model/MSentryPermChange.java (line 29) <https://reviews.apache.org/r/54729/#comment232352> Nit: add - Alexander Kolbasov On Jan. 10, 2017, 1:57 a.m., Hao Hao wrote: > > --- > This is an automatically

Re: Review Request 55094: SENTRY-1532: Sentry Web UI isn't working

2017-01-10 Thread Alexander Kolbasov
x27;t working Diffs (updated) - sentry-service/sentry-service-server/pom.xml 97bd326b0305303194a965a60b6afd0feefbbe0f sentry-tests/sentry-tests-solr/pom.xml a60b4eed4dde6c182b061681305013b3bd548cc3 Diff: https://reviews.apache.org/r/55094/diff/ Testing --- Thanks, Alex

Re: Review Request 53038: SENTRY-1507

2017-01-11 Thread Alexander Kolbasov
-server/pom.xml 97bd326b0305303194a965a60b6afd0feefbbe0f Diff: https://reviews.apache.org/r/53038/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 55246: SENTRY-1536: Refactor SentryStore transaction management to allow for extra transactions for a single permission update

2017-01-11 Thread Alexander Kolbasov
dateMap is always null, what is the idea here? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java (line 269) <https://reviews.apache.org/r/55246/#comment23

Re: Review Request 55094: SENTRY-1532: Sentry Web UI isn't working

2017-01-12 Thread Alexander Kolbasov
e-mail. To reply, visit: https://reviews.apache.org/r/55094/#review161459 --- On Jan. 11, 2017, 1:35 a.m., Alexander Kolbasov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.or

Re: Review Request 55246: SENTRY-1536: Refactor SentryStore transaction management to allow for extra transactions for a single permission update

2017-01-12 Thread Alexander Kolbasov
passing the update map or specific update to SentryStore, pass a TransactionBlock that should be executed as part of a SentrySTore transaction. This is a more generic approach. What do you think? - Alexander Kolbasov On Jan. 10, 2017, 2:31 a.m., Hao Hao wrote

Review Request 55594: SENTRY-1594 TransactionBlock should become generic

2017-01-16 Thread Alexander Kolbasov
85a5326ba52fc532f3d933f7e62a352f4d4080df Diff: https://reviews.apache.org/r/55594/diff/ Testing --- Thanks, Alexander Kolbasov

Review Request 55595: SENTRY-1428: Only leader should follow HMS updates

2017-01-16 Thread Alexander Kolbasov
--- Thanks, Alexander Kolbasov

Re: Review Request 54798: SENTRY-1546 Generic Policy provides bad error messages for Sentry exceptions

2017-01-17 Thread Alexander Kolbasov
che.org/r/54798/#comment233223> Should we do something similar for SentryNoSuchObjectException? - Alexander Kolbasov On Jan. 17, 2017, 7:34 p.m., kalyan kumar kalvagadda wrote: > > --- > This is an automatically generated e-

Re: Review Request 55735: SENTRY-1596: Hive tests failing for sentry-ha-redesign branch

2017-01-19 Thread Alexander Kolbasov
or off? - Alexander Kolbasov On Jan. 19, 2017, 11:28 p.m., Hao Hao wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55735/ >

Re: Review Request 55595: SENTRY-1428: Only leader should follow HMS updates

2017-01-19 Thread Alexander Kolbasov
visit: https://reviews.apache.org/r/55595/#review162362 --- On Jan. 17, 2017, 1:43 a.m., Alexander Kolbasov wrote: > > --- > This is an automatically generat

Re: Review Request 55595: SENTRY-1428: Only leader should follow HMS updates

2017-01-19 Thread Alexander Kolbasov
://reviews.apache.org/r/55595/diff/ Testing --- Thanks, Alexander Kolbasov

Review Request 55741: SENTRY-1599 CloseablePersistenceManager is no longer needed

2017-01-19 Thread Alexander Kolbasov
/provider/db/service/persistent/TransactionManager.java ee13f9f4d7e9ee0fd523d1144750222f0979c293 Diff: https://reviews.apache.org/r/55741/diff/ Testing --- Thanks, Alexander Kolbasov

Review Request 55744: SENTRY-1559 Remove fencing support

2017-01-19 Thread Alexander Kolbasov
77aaac5fd14ac7d3b721261b898120bf15cc0daa sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestFencer.java 7c080fab53d902eb6ce909139cf6f0a9c288ba54 Diff: https://reviews.apache.org/r/55744/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 54454: SENTRY-1548 Setting GrantOption to UNSET upsets Sentry

2017-01-21 Thread Alexander Kolbasov
- On Jan. 17, 2017, 7:28 p.m., kalyan kumar kalvagadda wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54454/ >

Re: Review Request 54454: SENTRY-1548 Setting GrantOption to UNSET upsets Sentry

2017-01-21 Thread Alexander Kolbasov
.java (line 1110) <https://reviews.apache.org/r/54454/#comment233900> It is usuallu better to use isEmpty instead of length() - Alexander Kolbasov On Jan. 17, 2017, 7:28 p.m., kalyan kumar kalvagadda wrote: > >

Re: Review Request 54947: SENTRY-1556 Simplify privilege cleaning

2017-01-22 Thread Alexander Kolbasov
ava (line 1930) <https://reviews.apache.org/r/54947/#comment233920> Do we still need this comment? sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 1940) <https://reviews.apache.o

Re: Review Request 55246: SENTRY-1536: Refactor SentryStore transaction management to allow for extra transactions for a single permission update

2017-01-22 Thread Alexander Kolbasov
/main/java/org/apache/sentry/provider/db/service/persistent/UpdateTransactionBlock.java (line 30) <https://reviews.apache.org/r/55246/#comment233945> Will this work for both path update and perm update? - Alexander Kolbasov On Jan. 19, 2017, 2:05 a.m., Hao Hao wrote: > >

Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications

2017-01-30 Thread Alexander Kolbasov
can be converted to foreach loop sentry-binding/sentry-binding-hive-follower/src/main/java/org/apache/sentry/binding/metastore/messaging/json/SentryJSONMessageFactory.java (line 134) <https://reviews.apache.org/r/55999/#comment235057> This can be converte

Re: Review Request 55246: SENTRY-1536: Refactor SentryStore transaction management to allow for extra transactions for a single permission update

2017-01-30 Thread Alexander Kolbasov
; I don't think we need hashCode and equals for DeltaTransactionBlock - Alexander Kolbasov On Jan. 27, 2017, 7:41 a.m., Hao Hao wrote: > > --- > This is an automatically gener

Re: Review Request 55246: SENTRY-1536: Refactor SentryStore transaction management to allow for extra transactions for a single permission update

2017-01-30 Thread Alexander Kolbasov
tained description of the class. For example: "Implementation of TransactionBlock that persists PathsUpdate or PermissionsUpdate." Please also comment on the assumptions and what exceptions are thrown if these assumptions are wrong. - Alexander Kolbasov On Jan. 27, 2017, 7:

Re: Review Request 56134: SENTRY-1593 and SENTRY-1592 Implementing client failover for Generic policy clients and namenode clients

2017-02-01 Thread Alexander Kolbasov
omment235374> Are you still using this constant? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java (line 262) <https://reviews.apache.org/r/56134/#comment235375> Are you still using this constant? - Alexander Kolbasov On Fe

Re: Review Request 56191: SENTRY-1619: Fix the secure HMS connection code in HMSFollower

2017-02-01 Thread Alexander Kolbasov
191/#comment235410> I trhink we should sleep here for a while before retrying. Also we shouldn't continue in this case - we should either retry forever or fail completely. - Alexander Kolbasov On Feb. 1, 2017, 1

Re: Review Request 55246: SENTRY-1536: Refactor SentryStore transaction management to allow for extra transactions for a single permission update

2017-02-01 Thread Alexander Kolbasov
th two TBs). sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java (line 396) <https://reviews.apache.org/r/55246/#comment235440> This is strange - you are only using the last transaction block - is it intentiona

Re: Review Request 56191: SENTRY-1619: Fix the secure HMS connection code in HMSFollower

2017-02-02 Thread Alexander Kolbasov
> On Feb. 1, 2017, 11:17 p.m., Alexander Kolbasov wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java, > > line 182 > > <https://reviews.apache.org/r/56191/diff/1/?file=1621825#file1621825line182> > >

Re: Review Request 56191: SENTRY-1619: Fix the secure HMS connection code in HMSFollower

2017-02-02 Thread Alexander Kolbasov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56191/#review164066 --- Ship it! Ship It! - Alexander Kolbasov On Feb. 2, 2017, 6

Re: Review Request 55246: SENTRY-1536: Refactor SentryStore transaction management to allow for extra transactions for a single permission update

2017-02-02 Thread Alexander Kolbasov
5633> This seems to be a copy and paste from SentryPlugin.getAuthzOb() - is it possible to just call that one? - Alexander Kolbasov On Feb. 3, 2017, 12:24 a.m., Hao Hao wrote: > > --- > This is an automatically gen

Re: Review Request 56271: SENTRY-1621: HMSFollower to retry connecting to HMS upon connection loss

2017-02-02 Thread Alexander Kolbasov
-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java (line 262) <https://reviews.apache.org/r/56271/#comment235649> can client.close() throw any exception? If it does, you'll miss resetting of kerberos context. - Alexander Kolbasov On Feb. 3, 2017, 4:42 a

Re: Review Request 55246: SENTRY-1536: Refactor SentryStore transaction management to allow for extra transactions for a single permission update

2017-02-02 Thread Alexander Kolbasov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55246/#review164091 --- Ship it! Ship It! - Alexander Kolbasov On Feb. 3, 2017, 5

Re: Review Request 55999: SENTRY-1602 Code cleanup for Sentry JSON message factory for hive notifications

2017-02-03 Thread Alexander Kolbasov
tiple places. sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/AbstractMetastoreTestWithStaticConfiguration.java <https://reviews.apache.org/r/55999/#comment235640> This looks like a stray change - Alexander Kolbasov On Jan. 31, 2017, 11:27 p.m., Nac

Re: Review Request 55744: SENTRY-1559 Remove fencing support

2017-02-04 Thread Alexander Kolbasov
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestFencer.java 7c080fab53d902eb6ce909139cf6f0a9c288ba54 Diff: https://reviews.apache.org/r/55744/diff/ Testing --- Thanks, Alexander Kolbasov

Review Request 56356: SENTRY-1615 SentryStore should not allocate empty objects that are immediately returned

2017-02-06 Thread Alexander Kolbasov
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegesAtFunctionScope.java cebad987c89c7950e28c700c3fe398d409280163 Diff: https://reviews.apache.org/r/56356/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 56356: SENTRY-1615 SentryStore should not allocate empty objects that are immediately returned

2017-02-06 Thread Alexander Kolbasov
/apache/sentry/provider/db/service/persistent/SentryStore.java 321c094366caa2b4a56758781e5fc5a2fc9218d0 Diff: https://reviews.apache.org/r/56356/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 56356: SENTRY-1615 SentryStore should not allocate empty objects that are immediately returned

2017-02-06 Thread Alexander Kolbasov
/org/apache/sentry/provider/db/service/persistent/SentryStore.java 321c094366caa2b4a56758781e5fc5a2fc9218d0 Diff: https://reviews.apache.org/r/56356/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 56356: SENTRY-1615 SentryStore should not allocate empty objects that are immediately returned

2017-02-06 Thread Alexander Kolbasov
------ This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56356/#review164439 --- On Feb. 7, 2017, 12:47 a.m., Alexander Kolbasov wrote: > >

Re: Review Request 56356: SENTRY-1615 SentryStore should not allocate empty objects that are immediately returned

2017-02-06 Thread Alexander Kolbasov
/sentry/provider/db/service/persistent/SentryStore.java 321c094366caa2b4a56758781e5fc5a2fc9218d0 Diff: https://reviews.apache.org/r/56356/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Sentry roadmap

2017-02-06 Thread Alexander Kolbasov
> On Feb 5, 2017, at 4:24 AM, Bruno Quinart wrote: > > Hi dev Hi Bruno, > > Is there a roadmap to a new version of Sentry available? Sentry HA is a major new feature, but there is no particular reason preventing a new release of Sentry cut before that if there is a critical mass of features.

Re: Review Request 55706: SENTRY-1566: Make full Perm/Path snapshot available for NN plugin

2017-02-07 Thread Alexander Kolbasov
n/java/org/apache/sentry/hdfs/PermImageRetriever.java (line 56) <https://reviews.apache.org/r/55706/#comment236222> Why do we construct it with curSeqNum but later in line 75 change it to seqNum? - Alexander Kolbasov On Feb. 3, 2017, 7:53 a.m., Hao Hao wrote: > > -

Re: Review Request 56134: SENTRY-1593 Implementing client failover for Generic policy clients and namenode clients

2017-02-07 Thread Alexander Kolbasov
on/src/main/java/org/apache/sentry/core/common/SentryServiceClient.java (line 29) <https://reviews.apache.org/r/56134/#comment236226> Better to just extend AutoCloseable. - Alexander Kolbasov On Feb. 6, 2017, 8:23 p.m., kalyan kumar kalvagadda wrote: > > ---

Re: Review Request 56134: SENTRY-1593 Implementing client failover for Generic policy clients and namenode clients

2017-02-07 Thread Alexander Kolbasov
vice/thrift/SentryGenericServiceClient.java (line 29) <https://reviews.apache.org/r/56134/#comment236228> We have a single implementor - do we actually need to define this as an interface? Can we just define the class we want (SentryGenericServiceClientDefaultImpl)? - Alexander Kolbasov On F

Review Request 56411: SENTRY-1624 DefaultSentryValidator doesn't correctly construct SentryOnFailureHookContextImpl

2017-02-07 Thread Alexander Kolbasov
/main/java/org/apache/sentry/binding/hive/v2/authorizer/DefaultSentryValidator.java c9da3ab0a901095acc0d78ce2ed0da0f0038ee56 Diff: https://reviews.apache.org/r/56411/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 56403: SENTRY-1387 Add HDFS sync tests for drop partition for external/implicit locations

2017-02-07 Thread Alexander Kolbasov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56403/#review164612 --- Ship it! Ship It! - Alexander Kolbasov On Feb. 7, 2017, 8

Re: Review Request 56000: SENTRY-1604 Sentry JSON message factory: Need more information in alter partition event

2017-02-07 Thread Alexander Kolbasov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56000/#review164613 --- Ship it! Ship It! - Alexander Kolbasov On Feb. 4, 2017, 5

Re: Review Request 56411: SENTRY-1624 DefaultSentryValidator doesn't correctly construct SentryOnFailureHookContextImpl

2017-02-07 Thread Alexander Kolbasov
This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56411/#review164577 ------- On Feb. 8, 2017, 12:53 a.m., Alexander Kolbasov wrote: > > -

Re: Review Request 56411: SENTRY-1624 DefaultSentryValidator doesn't correctly construct SentryOnFailureHookContextImpl

2017-02-07 Thread Alexander Kolbasov
sting --- Thanks, Alexander Kolbasov

[DISCUSS] Extending Sentry protocol

2017-02-07 Thread Alexander Kolbasov
Hello everyone, I would like to extend the Hive-to-Sentry API with a new call needed to synchronize between Hive operations and async Hive notification processing by Sentry. I would like to do it without incrementing the version number - do you think that it is reasonable, or *any* API changes req

Review Request 56454: SENTRY-1609: DelegateSentryStore is subject to JDQL injection

2017-02-08 Thread Alexander Kolbasov
/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 56356: SENTRY-1615 SentryStore should not allocate empty objects that are immediately returned

2017-02-08 Thread Alexander Kolbasov
r code that shows that this is possible? - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56356/#review164562 --- On Feb. 7, 20

Review Request 56481: SENTRY-1625: PrivilegeOperatePersistence can use QueryParamBuilder

2017-02-08 Thread Alexander Kolbasov
-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 17a4a937221891a72ee44db92976cfa5cab40bc4 Diff: https://reviews.apache.org/r/56481/diff/ Testing --- Thanks, Alexander Kolbasov

Re: Review Request 56458: SENTRY-1614 Removed code having PATHS column as primary key. Primary key of this long is not accepted by all databases

2017-02-09 Thread Alexander Kolbasov
ache/sentry/provider/db/tools/TestSentrySchemaToolWithmysql.java (line 66) <https://reviews.apache.org/r/56458/#comment236722> What is this IP address? - Alexander Kolbasov On Feb. 8, 2017, 7:16 p.m., kalyan kumar kalvagadda wrote: > > -

Re: Review Request 54532: SENTRY-378: Changed usages of SentryAccessDeniedException to SentryConfigurationException when retrieving Sentry version information in SentryStore

2017-02-16 Thread Alexander Kolbasov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54532/#review165906 --- Ship it! Ship It! - Alexander Kolbasov On Feb. 16, 2017, 5

Re: Review Request 56720: SENTRY-1611. Periodically purge MSentryPerm/PathChange table.

2017-02-16 Thread Alexander Kolbasov
it is weird that we are creating SentryStore in HMSFollower - something is wrong here. - Alexander Kolbasov On Feb. 16, 2017, 11:37 p.m., Lei Xu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: >

Re: Review Request 56481: SENTRY-1625: PrivilegeOperatePersistence can use QueryParamBuilder

2017-02-17 Thread Alexander Kolbasov
e. By purely looking at the logic, it seems > > what it is doing it just construct the QueryParamBuilder from the > > privilege. No more privileges are popluated from the given privilege. That's the original name. Agree that the name is weird. - Alexander --

Re: Review Request 56720: SENTRY-1611. Periodically purge MSentryPerm/PathChange table.

2017-02-17 Thread Alexander Kolbasov
ere and cast the result of execute. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java (line 124) <https://reviews.apache.org/r/56720/#comment237907> Who is consuming this? - Alexander Kolbasov

<    1   2   3   4   5   6   7   8   9   10   >