> On June 27, 2018, 12:50 p.m., kalyan kumar kalvagadda wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryHmsEvent.java
> > Lines 224-233 (patched)
> > <https://reviews.apache.org/r/67749/diff/2/?file=2046227#file2046227line224>
> >
> > As sentry doesn't changes to configurations at runtime, we doesn't have
> > to do this every time.
> > We can use constant that is initialized once and we can just use in
> > setAuthorizable method.
> >
> > we need not pass the HiveAuthzConf while construncting SentryHmsEvent.
>
> Na Li wrote:
> I have changed to code to get server name at once and use it in following
> calls.
>
> For class encapsulation and future extention, it is still better to
> provide HiveAuthzConf to SentryHmsEvent than for caller to get the
> configuration and provide to it as input.
Lina, serverName need be not stored again in SentryHmsEvent. Why don't you pass
serverName to setAuthorizable in the constructor directly?
Example:
public SentryHmsEvent(String inServerName, CreateDatabaseEvent event) {
this(event, EventType.CREATE_DATABASE);
setOwnerInfo(event.getDatabase());
setAuthorizable(event.getDatabase(), inServerName);
}
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67749/#review205423
-----------------------------------------------------------
On June 27, 2018, 7:13 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67749/
> -----------------------------------------------------------
>
> (Updated June 27, 2018, 7:13 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-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java
> f7d1b07
>
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/metastore/TestSentrySyncHMSNotificationsPostEventListener.java
> 8e79cac
>
> 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/4/
>
>
> Testing
> -------
>
> Manually trace into code and verfied that now the server receives request and
> client receiveds response correctly
>
>
> Thanks,
>
> Na Li
>
>
