> On Feb. 22, 2018, 6:56 p.m., Alexander Kolbasov wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java
> > Lines 146 (patched)
> > <https://reviews.apache.org/r/65704/diff/3/?file=1963273#file1963273line146>
> >
> >     Please remove all this logging. This whole thing can be simplified to
> >     
> >     `return !newDbName.equalsIgnoreCase(oldDbName) || 
> > !newTableName.equalsIgnoreCase(oldTableName);`
> >     
> >     You don't need this if statement at all.

Sergio and Kalyan want to check if the values to be null before the 
comparision. They also want to log in warning level in case one of them are 
null.


> On Feb. 22, 2018, 6:56 p.m., Alexander Kolbasov wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 2412-2413 (patched)
> > <https://reviews.apache.org/r/65704/diff/3/?file=1963274#file1963274line2412>
> >
> >     What does this have to do with the JIRA that you are fixing? Looks like 
> > some kinnd of separate issue.

I have created SENTRY-2144 to track this bug, will remove from this fix.


- Na


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


On Feb. 22, 2018, 6:55 p.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65704/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2018, 6:55 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and 
> Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> wait for HMS sync at alter table, which including table rename and changing 
> table columns
> 
> 
> Diffs
> -----
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java
>  24d7763 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  7a31a01 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java
>  5fe6625 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  9b0aeb2 
> 
> 
> Diff: https://reviews.apache.org/r/65704/diff/4/
> 
> 
> Testing
> -------
> 
> unit tests succeeded
> 
> 
> Thanks,
> 
> Na Li
> 
>

Reply via email to