-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69617/
-----------------------------------------------------------

Review request for sentry, Arjun Mishra, Anthony Young-Garner, kalyan kumar 
kalvagadda, Li Pi, Na Li, Steve Moist, and Sergio Pena.


Bugs: SENTRY-2469
    https://issues.apache.org/jira/browse/SENTRY-2469


Repository: sentry


Description
-------

i havefixed issues in RoleServlet.java. 

1. `RoleServlet` at L62 needs to use either `try` resource or put it in the 
`finally` block to avoid resource leak
2. `if (conf.getBoolean(ServerConfig.SENTRY_WEB_ENABLE, 
ServerConfig.SENTRY_WEB_ENABLE_DEFAULT))` may return an NPE if 
`conf.getBoolean` returns a `null` `Boolean` (not the primitive type)
3. `RoleServlet` at L45, use `Preconditions.checkNotNull` or 
`Objects.requreNonNull` instead of `assert`
4. `RoleServlet` at L55: `String json = new Gson().toJson(roleMap);`, it's 
better to reuse the `Gson` instance.


Diffs
-----

  
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/service/common/ServiceConstants.java
 4508361b 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/RolesServlet.java
 9ed2bf31 


Diff: https://reviews.apache.org/r/69617/diff/1/


Testing
-------

builded and tested it


Thanks,

hongtaq zhao

Reply via email to