----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69875/#review212504 -----------------------------------------------------------
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java Lines 454 (patched) <https://reviews.apache.org/r/69875/#comment298304> What exactly is the purpose of this log? If the following "if" statement is false, then nothing will happen, but this line will be logged regardless, which makes it look like something did happen. I think it would be better to either combine this log message with the one inside the if statement, or to change it to something like, "Attempting to remove path..." and make sure we have a log message that shows whether or not it was successful. - Haley Reeve On Jan. 31, 2019, 4:46 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69875/ > ----------------------------------------------------------- > > (Updated Jan. 31, 2019, 4:46 p.m.) > > > Review request for sentry, Arjun Mishra, Haley Reeve, and Na Li. > > > Bugs: SENTRY-2205 > https://issues.apache.org/jira/browse/SENTRY-2205 > > > Repository: sentry > > > Description > ------- > > Logging will help us identify below > 1. Track the updates from Sentry > 2. What the path update is? > 3. What the perm update is? > 4. How the path change is effecting the exizting cache and what is the result > of the update? > 5. How the perm change is effecting the exizting cache and what is the result > of the update? > 6. Changes to the path tree stored as part of the cache. > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java > 97a04d9eb5a1e0c5feb97b038b8fc0edec9b0d38 > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/Updateable.java > bbf3f13e44ca3ee6ecb1d635abb9048942b26ac3 > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPaths.java > c967f4e2ee0a811e04362aef452b1cfddfd973f3 > > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java > 9dae03562aff82574dcda5339df415193655d5a2 > > sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/UpdateableAuthzPermissions.java > 5726c0e834e6e5ad47f47f3337d9317ee1366955 > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryHDFSServiceClientDefaultImpl.java > 9115697088cab8fd1732086963c7c3f982069380 > > > Diff: https://reviews.apache.org/r/69875/diff/1/ > > > Testing > ------- > > As there is no functiuonal change I have not added any tests. I made sure > that existing tests pass. > > > Thanks, > > kalyan kumar kalvagadda > >
