> On Nov. 3, 2014, 4:04 p.m., Lenni Kuff wrote:
> >

Fixed, thanks.


> On Nov. 3, 2014, 4:04 p.m., Lenni Kuff wrote:
> > sentry-core/sentry-core-common/src/test/java/org/apache/sentry/core/common/utils/TestPathUtils.java,
> >  line 131
> > <https://reviews.apache.org/r/27512/diff/1/?file=747211#file747211line131>
> >
> >     Curious, where is the IllegalStateException coming from? Seems like 
> > this should be an IllegalArgumentException.

Fixed, thanks.


> On Nov. 3, 2014, 4:04 p.m., Lenni Kuff wrote:
> > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java,
> >  line 124
> > <https://reviews.apache.org/r/27512/diff/1/?file=747210#file747210line124>
> >
> >     The name of this method is not correct because now you are passing a 
> > "isLocal" flag so it's not always a DFS.
> >     It also seems like the existing Hadoop "Path" class implements all this 
> > functionality already. Why can't we use that?

For the method name, fixed, thaks.
For the "Path" class in Hadoop, I check the "Path" class and think this class 
can initial the url from string and do the check. But for this feature, I think 
we can keep the current implement, thanks.


- Colin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27512/#review59578
-----------------------------------------------------------


On Nov. 4, 2014, 6:46 a.m., Colin Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27512/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2014, 6:46 a.m.)
> 
> 
> Review request for sentry and Prasad Mujumdar.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Currently the URI privilege validation assumes that the filesystem prefixes 
> can be 'file:' or 'hdfs:'. We should make this more generic to support other 
> prefixes available in the underlying filesystem.
> 
> 
> Diffs
> -----
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
>  f94ae7c 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PathUtils.java
>  1cdbdb8 
>   
> sentry-core/sentry-core-common/src/test/java/org/apache/sentry/core/common/utils/TestPathUtils.java
>  d30305b 
> 
> Diff: https://reviews.apache.org/r/27512/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colin Ma
> 
>

Reply via email to