> On June 26, 2018, 8:20 p.m., Sergio Pena wrote: > > sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift > > Lines 360-362 (original), 360-362 (patched) > > <https://reviews.apache.org/r/67749/diff/1/?file=2045274#file2045274line360> > > > > Why was it moved in different order? I think all the fields, exceptio > > the protocl_version, should be optional to allow future compatibility if we > > decide to deprecate some of the fields.
the optional fields must be after required fields. We always need the other fields to function correctly, so they are required fields. We need to balance the flexibility and avoid too many checks for optional fields - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67749/#review205392 ----------------------------------------------------------- On June 26, 2018, 8 p.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67749/ > ----------------------------------------------------------- > > (Updated June 26, 2018, 8 p.m.) > > > Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio > Pena. > > > Bugs: sentry-2280 > https://issues.apache.org/jira/browse/sentry-2280 > > > Repository: sentry > > > Description > ------- > > The required fields in TSentryHmsEventNotification and > TSentryHmsEventNotificationResponse are not filled, which caused the request > from client to sever to be null. And the response from server to client is > null. Change the data structure to make owner info optional, and fill other > required fields properly > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryHmsEvent.java > 42be3c3 > > sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/AccessConstants.java > a4fa226 > > sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryHmsEventNotification.java > 75b2799 > > sentry-service/sentry-service-api/src/main/resources/sentry_policy_service.thrift > 56aedcb > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java > 3927150 > > > Diff: https://reviews.apache.org/r/67749/diff/1/ > > > Testing > ------- > > Manually trace into code and verfied that now the server receives request and > client receiveds response correctly > > > Thanks, > > Na Li > >
