> On Nov. 28, 2017, 10:16 p.m., Vadim Spector wrote: > > 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/diff/1-2/?file=1882251#file1882251line2518> > > > > 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.
No matter what we decide, should probably document those assuimptions in the test code, if only in couple of sentences. - Vadim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63596/#review192068 ----------------------------------------------------------- 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 > >
