> On Aug. 17, 2018, 5:24 p.m., Na Li wrote: > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializerState.java > > Lines 28 (patched) > > <https://reviews.apache.org/r/67949/diff/1/?file=2061160#file2061160line28> > > > > It is enum, but only one state. Kind of weird. > > > > Should it has other states, such as "INIT", "SUCCEED", "FAILED" > > > > Or it is just a normal class, no need to be enum > > kalyan kumar kalvagadda wrote: > "INIT", "SUCCEED", "FAILED" reflect status not current state. > Reason i did not add addtional states as we need not track any other > state at this point in time. When we may need to track more states in future > we can extend the enum to have more states. What do you say?
I am OK with this. - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67949/#review207514 ----------------------------------------------------------- On July 17, 2018, 10:53 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67949/ > ----------------------------------------------------------- > > (Updated July 17, 2018, 10:53 p.m.) > > > Review request for sentry, Arjun Mishra, Na Li, and Sergio Pena. > > > Bugs: SENTRY-2307 > https://issues.apache.org/jira/browse/SENTRY-2307 > > > Repository: sentry > > > Description > ------- > > Sentry tries to synchronize the HMS operations to make sure that sentry > server process the notifications for create/drop/alter table and databases > before it receives permission grants/revokes on them. Blocking HMS threads to > make sentry processes appropriate notifications makes sense but doing the > same when full snapshot is taken doesn’t serve any purpose as sentry server > doesn’t update sentry permissions based on HMS full snapshot. We can document > that there sentry could have some stale permissions when there are any DDL > operations performed at the time of snapshot creation. > > > Diffs > ----- > > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java > 2efc8cf9c1380a063c54d6bf4ef83e9d0fa8ebc9 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializerState.java > PRE-CREATION > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java > 12266cb25ea70753e07d27b53de777f347f68844 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryStateBank.java > 2c05d49d7f1b7e43318dcfeba22f4f8f4fa3c724 > > sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java > 3475624e35cb011df01cc0fa633c01141a882337 > > sentry-service/sentry-service-server/src/test/java/org/apache/sentry/service/thrift/TestSentryHMSClient.java > 38668ca88f155e0fa2f3beb4db32b18e953e58b6 > > > Diff: https://reviews.apache.org/r/67949/diff/1/ > > > Testing > ------- > > Added tests to verify the changes added. > > > Thanks, > > kalyan kumar kalvagadda > >