> On Sept. 20, 2013, 11:26 p.m., Shreepadma Venugopalan wrote: > > sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java, > > line 79 > > <https://reviews.apache.org/r/14165/diff/1/?file=352569#file352569line79> > > > > I don't see sentry-deprecated-site.xml in this patch. Does this patch > > assume SENTRY-16 is available?
Yes, my mistake. I'll rebase now that SENTRY-16 has been committed. > On Sept. 20, 2013, 11:26 p.m., Shreepadma Venugopalan wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java, > > line 166 > > <https://reviews.apache.org/r/14165/diff/1/?file=352568#file352568line166> > > > > Shouldn't deprecatedConfigs be a map of the deprecated config's name to > > the one that' currently in use? It depends on how you want to use it, I guess. Here's my thinking: 1) the code should work if the deprecated properties are provided, as long as the new property names aren't also provided 2) the binding code will only call the new property names. That is, hive-binding will only call get(AUTHZ_PROVIDER) not get(AUTHZ_PROVIDER_DEPRECATED) 3) given #1 and #2, a call to get(AUTHZ_PROVIDER) needs to return the same value as get(AUTHZ_PROVIDER_DEPRECATED) if AUTHZ_PROVIDER is not specified 4) The most straightforward way of accomplishing #3 is to have a map from non-deprecated-name to deprecated-name, e.g. AUTHZ_PROVIDER -> AUTH_PROVIDER_DEPRECATED. Make sense? We can rename the collection if you think that improves things. - Gregory ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14165/#review26307 ----------------------------------------------------------- On Sept. 17, 2013, 6:27 p.m., Gregory Chanan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14165/ > ----------------------------------------------------------- > > (Updated Sept. 17, 2013, 6:27 p.m.) > > > Review request for sentry and Shreepadma Venugopalan. > > > Bugs: SENTRY-4 > https://issues.apache.org/jira/browse/SENTRY-4 > > > Repository: sentry > > > Description > ------- > > SENTRY-4: Rename Configuration properties that mention hive but are sentry > related > > As discussed in the JIRA, does the following: > 0) add new sentry.* properties, as discussed in SENTRY-14 > 1) remove deprecation code around hive.access.* properties > 2) add deprecation code around hive.sentry.* properties > 3) warn about deprecated configs only if they actually end up being used > > I'm a bit unsure of spacing (how many spaces is a tab? I used two in most > places -- perhaps something to add to the how to contribute page?) > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java > 5190ba6 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java > bfd58fa > > sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java > 20d4e8f > > sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzConf.java > de59546 > sentry-binding/sentry-binding-hive/src/test/resources/access-site.xml > 1936c21 > sentry-binding/sentry-binding-hive/src/test/resources/sentry-site.xml > beef40d > sentry-tests/src/test/resources/access-site.xml e4de05a > sentry-tests/src/test/resources/sentry-site.xml de0c9cf > > Diff: https://reviews.apache.org/r/14165/diff/ > > > Testing > ------- > > Ran the unit tests and they were successful: > mvn test -Pdownload-hadoop > > > Thanks, > > Gregory Chanan > >
