----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59655/#review176500 -----------------------------------------------------------
All my comments are about improving the code. Since you are fixing the actual problem, I'll leave it up to you to decide whether you want to separate the fix from the code improvement. If you do decide to do code improvement separately, please change just what's needed to fix the problem, but if you do decide to add improvements, let's make this function better. - Alexander Kolbasov On May 30, 2017, 11:17 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59655/ > ----------------------------------------------------------- > > (Updated May 30, 2017, 11:17 p.m.) > > > Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, > Sergio Pena, Vamsee Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1788 > https://issues.apache.org/jira/browse/SENTRY-1788 > > > Repository: sentry > > > Description > ------- > > Root Cause: Application was using JDO object even after the associated > database entry was deleted. > Fix: Made code change so that JDO object is detached so that deletion of data > in the database would not invalidate the object used by the application. > parent object which is an JDO object used in > SentryStore.dropOrRenamePrivilegeForAllRoles after associated data in the > database is deleted. Method alterSentryRoleRevokePrivilegeCore which called > would internally delete the data from database.As part of this exercise > > Summary of the code changes > 1. Fix as described above > 2. Code optimization. dropOrRenamePrivilegeForAllRoles method handles both > drop and rename of privileges. There is certain logic which constructs > privilege graph which is executed for both dropping and renaming of the > privilege. This logic needs to be executed only for renaming of privileges > as the privilege graph constructed is used only when the privilege is renamed. > > > Diffs > ----- > > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 958a46c > > > Diff: https://reviews.apache.org/r/59655/diff/1/ > > > Testing > ------- > > Made sure the unit tests are passing. > > > Thanks, > > kalyan kumar kalvagadda > >
