> 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. > > Hrishikesh Gadre wrote: > Same as above.
sorry missed the original question. The problem here is that file_based sentry provides privilege as a String, while the Sentry service provides it as a TSentryPrivilege object. In order to avoid code duplication (eg in PermissionsMigrationToolSolr::transformPrivileges), I am using String version of it. Even with your proposal, we would have to translate String privilege to TSentryPrivilege in case of file based Sentry. Hence for simplicity, I think we should be OK with this function signature. - 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 > >
