----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63596/#review192068 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java Line 2514 (original), 2513 (patched) <https://reviews.apache.org/r/63596/#comment270077> So, what are our final assumptions about leading and trailing hashes in hdfs paths at the point of adding them to Sentry store? Do we assume they only have leading slashes? Can they be multiple slashes or single-only? What about trailing slashes - none, single-only, multiple ok? I start thinking that sentry store should be taking hdfs paths with "bad" slashes and canoinicalize them internally. I like the idea of a contract between Sentry components in regards to hdfs paths; we keep talking about, but the truth is contracts can be inadvertantly broken by code changes. Canonicalizing paths seems like a cheap operation, so I think that Sentry store should be fine accepting non-canonicalized paths - in fact, we should do just that and test the resuts by passing non-canonicalized paths and make sure canonicalization happened. Would like others to weigh in. - Vadim Spector On Nov. 27, 2017, 6:03 p.m., Arjun Mishra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63596/ > ----------------------------------------------------------- > > (Updated Nov. 27, 2017, 6:03 p.m.) > > > Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, Na > Li, Sergio Pena, and Vadim Spector. > > > Repository: sentry > > > Description > ------- > > The old retrieveFullPathsImage() method in SentryStore is no longer used by > actual code (retrieveFullPathsImageUpdate(final String[] prefixes) is used > instead. It was preserved because it is used by test which now doesn't make > much sense. > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestImageRetriever.java > b355630e7 > > sentry-hdfs/sentry-hdfs-service/src/test/java/org/apache/sentry/hdfs/TestSentryHDFSServiceProcessor.java > f09d1b228 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > f32a745ed > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 24b11f724 > > > Diff: https://reviews.apache.org/r/63596/diff/2/ > > > Testing > ------- > > mvn -f sentry-provider/sentry-provider-db/pom.xml test -Dtest=TestSentryStore > mvn -f sentry-hdfs/sentry-hdfs-service/pom.xml test > -Dtest=TestSentryHDFSServiceProcessor > mvn -f sentry-hdfs/sentry-hdfs-service/pom.xml test > -Dtest=TestImageRetriever.java > > > Thanks, > > Arjun Mishra > >
