[
https://issues.apache.org/jira/browse/HBASE-5526?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13224999#comment-13224999
]
[email protected] commented on HBASE-5526:
------------------------------------------------------
bq. On 2012-03-08 04:53:06, Lars Hofhansl wrote:
bq. > src/main/java/org/apache/hadoop/hbase/util/FSUtils.java, line 188
bq. > <https://reviews.apache.org/r/4217/diff/2/?file=89028#file89028line188>
bq. >
bq. > Ugh... Catching NPE? Better to do a null check for conf above.
I think is going to be a corner case, since if you enable that stuff, you are
expected to also be setting the umasking - was trying to avoid having the
extra branching. Can pull it if you still think necessary.
bq. On 2012-03-08 04:53:06, Lars Hofhansl wrote:
bq. > src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java, line 157
bq. > <https://reviews.apache.org/r/4217/diff/2/?file=89031#file89031line157>
bq. >
bq. > Maybe also have a test that verifies the actual HBase file
permissions.
bq. > I.e. write some data, then check permissions of HFile.
+1
bq. On 2012-03-08 04:53:06, Lars Hofhansl wrote:
bq. > src/main/java/org/apache/hadoop/hbase/util/FSUtils.java, line 152
bq. > <https://reviews.apache.org/r/4217/diff/2/?file=89028#file89028line152>
bq. >
bq. > Why the -- leading the comment?
Cruft from personal debugging. Going switch it over to debug as well.
bq. On 2012-03-08 04:53:06, Lars Hofhansl wrote:
bq. > src/main/java/org/apache/hadoop/hbase/util/FSUtils.java, line 127
bq. > <https://reviews.apache.org/r/4217/diff/2/?file=89028#file89028line127>
bq. >
bq. > Since we pass perms in, we do not need configuration, right?
bq. > Should overwrite be false for existing behavior?
+1
bq. On 2012-03-08 04:53:06, Lars Hofhansl wrote:
bq. > src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java, line
211
bq. > <https://reviews.apache.org/r/4217/diff/2/?file=89027#file89027line211>
bq. >
bq. > I know we went back and forth on this... But there is nothing in
.tableinfo that is secret to a viewer, just some table metadata. So we do not
really have to go through this huh-hah :)
bq. >
bq. > Also if we wanted to do this and we always have to create an
HBaseConfiguration anyway, why not do this down in writeHTD?
bq. > That way the conf would not need to be passed down.
For the first part, thought we had decided the other way around (in fact
remember you saying today I forgot to get it with v1 ;). But yeah, the full htd
is up in the hbase ui, so kinda pointless :)
"That way the conf would not need to be passed down."
Wanted to make it explicit from whence the conf came. But yeah, I guess that
also works and is cleaner wrt to api changes. Again, pointless since dropping
.tableinfo.
bq. On 2012-03-08 04:53:06, Lars Hofhansl wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 346
bq. > <https://reviews.apache.org/r/4217/diff/2/?file=89026#file89026line346>
bq. >
bq. > Leftover?
cruft from last time.
bq. On 2012-03-08 04:53:06, Lars Hofhansl wrote:
bq. > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 772
bq. > <https://reviews.apache.org/r/4217/diff/2/?file=89026#file89026line772>
bq. >
bq. > Should be HREGION_INFO_PERMISSION_KEY, right?
bq. > Or I guess HREGION_INFO_PERMISSION_KEY is a leftover?
The latter. The question here is whether we should be referencing that key
directly when doing getFilePermissions or if we should just have that method
assume we are getting it from that conf key? Did it this way to make it
explicit where the info is coming from - its not a bit change later if we
wanted to do multiple levels of 'if nothing set here, then this key'.
- Jesse
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4217/#review5702
-----------------------------------------------------------
On 2012-03-08 02:56:16, Jesse Yates wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/4217/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-03-08 02:56:16)
bq.
bq.
bq. Review request for hbase and Lars Hofhansl.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Currently many all the files created by the HBase user are just written
using the default file permissions granted by hdfs. However, to ensure only the
correct user/group views the files and directories, we need to be able to apply
a configurable umask to either directories or files.
bq.
bq. This ticket covers setting permissions for files written to dfs, as
opposed to things like pid and log files.
bq.
bq. The impetus for this was to allow the web-user to view the directory
structure of hbase, but not to actually see any of the actual data hbase is
storing.
bq.
bq.
bq. This addresses bug HBASE-5526.
bq. https://issues.apache.org/jira/browse/HBASE-5526
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. src/main/java/org/apache/hadoop/hbase/HConstants.java e60ce04
bq. src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileWriter.java
9e7e624
bq. src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 76ff422
bq. src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
62cf6ac
bq. src/main/java/org/apache/hadoop/hbase/util/FSUtils.java d2d7efe
bq. src/main/resources/hbase-default.xml 9277e0c
bq. src/test/java/org/apache/hadoop/hbase/util/TestFSTableDescriptors.java
0db4d42
bq. src/test/java/org/apache/hadoop/hbase/util/TestFSUtils.java e2611e6
bq.
bq. Diff: https://reviews.apache.org/r/4217/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. "mvn clean test -P localTests" passes
bq.
bq.
bq. Thanks,
bq.
bq. Jesse
bq.
bq.
> Configurable file and directory based umask
> -------------------------------------------
>
> Key: HBASE-5526
> URL: https://issues.apache.org/jira/browse/HBASE-5526
> Project: HBase
> Issue Type: New Feature
> Components: regionserver
> Reporter: Jesse Yates
> Assignee: Jesse Yates
> Fix For: 0.94.0
>
> Attachments: java_HBASE-5526-v2.patch, java_HBASE-5526-v3.patch,
> java_HBASE-5526.patch
>
>
> Currently many all the files created by the HBase user are just written using
> the default file permissions granted by hdfs. However, to ensure only the
> correct user/group views the files and directories, we need to be able to
> apply a configurable umask to either directories or files.
> This ticket covers setting permissions for files written to dfs, as opposed
> to things like pid and log files.
> The impetus for this was to allow the web-user to view the directory
> structure of hbase, but not to actually see any of the actual data hbase is
> storing.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
