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




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolCommon.java
Line 242 (original), 241 (patched)
<https://reviews.apache.org/r/66019/#comment279272>

    The code would be more readable if the variable "p" is named as 
"originalPrivilege" or something like that.
    
    "x" as "migratedPrivilege" or something like that.
    
    Since this is not part of the fix, it is up to you to change it or not.


- Na Li


On March 10, 2018, 6:20 a.m., Hrishikesh Gadre wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66019/
> -----------------------------------------------------------
> 
> (Updated March 10, 2018, 6:20 a.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda and Sergio Pena.
> 
> 
> Bugs: SENTRY-2178
>     https://issues.apache.org/jira/browse/SENTRY-2178
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> The root cause of this issue is that the equals method in TSentryPrivilege 
> object performs case sensitive comparison of component string. When we 
> convert TSentryPrivilege object from string representation, the component 
> name is set as SOLR. On the other hand TSentryPrivilege object prepared from 
> database state has the component name as solr. Due to this the original 
> permission can not be found in the converted permissions resulting in 
> incorrect revocation.
> 
> To fix this problem, the migration tool performs case insensitive string 
> comparison to figure out if the original permission is already included in 
> the migrated permissions and hence must not be revoked.
> 
> 
> Diffs
> -----
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolCommon.java
>  8c2ec329f32f28207757372083afe0dd93af1290 
> 
> 
> Diff: https://reviews.apache.org/r/66019/diff/1/
> 
> 
> Testing
> -------
> 
> Unit tests passing
> Tested an upgrade from Sentry 1.5 to Sentry 2.0 on a real cluster by using 
> this migration tool.
> 
> 
> Thanks,
> 
> Hrishikesh Gadre
> 
>

Reply via email to