> 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
"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? - kalyan kumar ----------------------------------------------------------- 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 > >
