> 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. > > Hrishikesh Gadre wrote: > 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.
Yes, you are right. Functionally they are duplicate but one would work in privileges in string format and the other one on privileges in TSentryPrivilege. That is fine. we can live with that for now. I will open a jira for that optimization and target it for setry 2.1.1. - kalyan kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63878/#review191254 ----------------------------------------------------------- On Nov. 21, 2017, 7:08 p.m., Hrishikesh Gadre wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63878/ > ----------------------------------------------------------- > > (Updated Nov. 21, 2017, 7:08 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/2/ > > > Testing > ------- > > All unit tests are passing. > Added a unit test to verify this migration tool. > > > Thanks, > > Hrishikesh Gadre > >
