> On Nov. 16, 2017, 10 p.m., kalyan kumar kalvagadda wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolCommon.java > > Lines 218 (patched) > > <https://reviews.apache.org/r/63878/diff/1/?file=1894331#file1894331line218> > > > > grantPrivilege and revokePrivilege methods can throw exceptions. I=In > > the event one these API's throw an exception the dabtabase will be left in > > an inconsistent state and there is no way to reover.
The sentry client/server protocol does not support transactional (i.e. all_or_nothing) behavior. Given that privilege grant/revoke methods are idempotent, we just need to rerun the tool to fix any inconsistencies. I have added a --dry-run option as well, which will allow user to figure out the change in advance to mitigate the impact of failed migration. > On Nov. 16, 2017, 10 p.m., kalyan kumar kalvagadda wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolCommon.java > > Lines 236 (patched) > > <https://reviews.apache.org/r/63878/diff/1/?file=1894331#file1894331line236> > > > > If you implement Collection<String> > > transformPrivileges(Collection<TSentryPrivilege> privileges). You can > > avaoid over ahead of converting TSentryPrivilege to string and back to > > TSentryPrivilege. > > > > Ideally you should handle exceptions and roll back to a state the > > database was before migration was done. Same as above. - Hrishikesh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63878/#review191254 ----------------------------------------------------------- On Nov. 16, 2017, 2:31 p.m., Hrishikesh Gadre wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63878/ > ----------------------------------------------------------- > > (Updated Nov. 16, 2017, 2:31 p.m.) > > > Review request for sentry, kalyan kumar kalvagadda and Sergio Pena. > > > Bugs: SENTRY-1480 > https://issues.apache.org/jira/browse/SENTRY-1480 > > > Repository: sentry > > > Description > ------- > > SENTRY-1475 upgraded the Sentry/SOLR plugin to use the latest version of SOLR > (7.1.0) which provides pluggable authorization framework. With this, the way > admin privileges are defined is changed. This patch provides a command-line > tool to migrate existing privileges to this new model. It supports both (a) > file based Sentry and (b) Sentry service configuration. > > > Diffs > ----- > > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyFileConstants.java > 6b625ffb61159fa50a7d556762fd08f33e873c40 > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyFiles.java > ca2de7a165f0c8c37efe2b0c1ab3858addc15acf > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/StrictStringTokenizer.java > PRE-CREATION > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/Version.java > PRE-CREATION > > sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/PrivilegeUtils.java > 6628a2f3cbb338542a6f02a0387b6fd7a0e092dc > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolCommon.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/PermissionsMigrationToolSolr.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/tools/TestPermissionsMigrationToolSolr.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/63878/diff/1/ > > > Testing > ------- > > All unit tests are passing. > Added a unit test to verify this migration tool. > > > Thanks, > > Hrishikesh Gadre > >