|
I'd guess the massive confusion at this point would be that we're using decimal rather than octal, but yeah, changing at this point cold-turkey would probably just confuse a our established users rather than our new users. My concern over using a lead 0 is that li/unix only uses octal for chmod, whether or not a leading 0 is specified. I'd like to end up with something that matches that behavior.
In that case, the best thing is probably to deprecate the current key and introduce a new one with octal/symbolic semantics. That's the approach I'll plan on using. Too bad this is one of those things where i think it is acceptable to break existing users, given proper release notes and enough warning. really. if we are willing to break api's then breaking something as relatively minor as permissions in the conf file does not seem like that big of deal.
perhaps this will be the jira that finally provides 'upgrade' notes instead of just 'release' notes, like most real world operating systems. Done with most of the work. I very much would like to use
Patch:
Still need to:
Moved to Common, where the affected files actually reside
Finished patch. Rather than wait for 6105, handcoded check for prior value. When 6105 is committed, will update to use its functionality.
Compared to previous patch, provided backwards compatibility, updated documentation and split one test into three. Of note, HDFS needs to be updated due to this patch and will open a new JIRA for that now. Passes all unit tests. Test-patch: [exec] +1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 3 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. submitting patch, although already ran unit tests and test-patch.
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12418574/COMMON-6234.patch  against trunk revision 810756. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/17/testReport/ This message is automatically generated. Committed the change. Thanks Jakob.
Integrated in Hadoop-Common-trunk-Commit #22 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/22/
). Add new option dfs.umaskmode to set umask in configuration to use octal or symbolic instead of decimal. Contributed by Jakob Homan. Integrated in Hadoop-Hdfs-trunk-Commit #23 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/23/
). Updated hadoop-core and test jars to propagate new option dfs.umaskmode in configuration. Contributed by Jakob Homan. Integrated in Hdfs-Patch-h2.grid.sp2.yahoo.net #5 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/5/
). Updated hadoop-core and test jars to propagate new option dfs.umaskmode in configuration. Contributed by Jakob Homan. Integrated in Hadoop-Common-trunk #90 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/90/
). Add new option dfs.umaskmode to set umask in configuration to use octal or symbolic instead of decimal. Contributed by Jakob Homan. Integrated in Hdfs-Patch-h5.grid.sp2.yahoo.net #21 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/21/
)
Editorial pass over all release notes prior to publication of 0.21.
attaching a patch for branch 0.20. This patch has some differences from the 0.21 patch. The changes related to sticky bits had to be removed since 0.20 does not have sticky bit feature.
Patch for 20 looks good. Essentially it is octal/symbolic umask settings in a universe where sticky bits don't exist.
Comments:
These are minor and otherwise +1. For the 0.20 patch, we should test whether it has the problem described in HDFS-760.
New patch that addresses Jakob's comments.
For 0.20 patch, the problem described in HDFS-760 is not relevant. On trunk we use a different backward compatibility mechanism to map deprecated configuration key to new configuration key, using DeprecatedKeyInfo. That patch has its own mechanism to provide backward compatibility.
updated patch looks good. +1.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Bug
Resolved
Major
Permission configuration files should use octal and symbolic
Symbolic is ok, since it isn't ambiguous. To use straight octal we either need a flag like a leading o or use a new attribute and handle the old attribute in decimal.
My other concern in this area is against supporting octal via a leading 0 on all integer attributes. That will lead to massive confusion, in my opinion.