You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "Daryn Sharp (JIRA)" <ji...@apache.org> on 2011/04/22 00:02:05 UTC
[jira] [Created] (HADOOP-7236) Refactor FsShell's mkdir
Refactor FsShell's mkdir
------------------------
Key: HADOOP-7236
URL: https://issues.apache.org/jira/browse/HADOOP-7236
Project: Hadoop Common
Issue Type: Improvement
Components: fs
Affects Versions: 0.23.0
Reporter: Daryn Sharp
Assignee: Daryn Sharp
Need to refactor tail to conform to new FsCommand class.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (HADOOP-7236) Refactor FsShell's mkdir
Posted by "Daryn Sharp (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-7236?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Daryn Sharp updated HADOOP-7236:
--------------------------------
Attachment: HADOOP-7236.patch
Split out mkdir command.
Fix usage of mkdir.
Move glob handling of PathData into PathData.java.
Minor changes to PathData to record actual string used to instantiate object since Path does not return what it was given which breaks tests.
> Refactor FsShell's mkdir
> ------------------------
>
> Key: HADOOP-7236
> URL: https://issues.apache.org/jira/browse/HADOOP-7236
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs
> Affects Versions: 0.23.0
> Reporter: Daryn Sharp
> Assignee: Daryn Sharp
> Attachments: HADOOP-7236.patch
>
>
> Need to refactor tail to conform to new FsCommand class.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HADOOP-7236) Refactor FsShell's mkdir
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-7236?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13029284#comment-13029284 ]
Hudson commented on HADOOP-7236:
--------------------------------
Integrated in Hadoop-Common-trunk #679 (See [https://builds.apache.org/hudson/job/Hadoop-Common-trunk/679/])
HADOOP-7236. Refactor the mkdir command to conform to new FsCommand class. Contributed by Daryn Sharp
> Refactor FsShell's mkdir
> ------------------------
>
> Key: HADOOP-7236
> URL: https://issues.apache.org/jira/browse/HADOOP-7236
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs
> Affects Versions: 0.23.0
> Reporter: Daryn Sharp
> Assignee: Daryn Sharp
> Fix For: 0.23.0
>
> Attachments: HADOOP-7236-2.patch, HADOOP-7236-3.patch, HADOOP-7236.patch
>
>
> Need to refactor tail to conform to new FsCommand class.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HADOOP-7236) Refactor FsShell's mkdir
Posted by "John George (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-7236?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13027018#comment-13027018 ]
John George commented on HADOOP-7236:
-------------------------------------
Overall the code looks good. Minor comments -
The mkdirs() in PathData.java file: I understand that it was added to help move FsShell to FileContext in an easier way. Since it looks like it might be out of place being in PathData.java, would it make sense to instead move it to Mkdirs.java. I understand that "fs" was moved to PathData to avoid the callers from having to deal/know about the "fs", but to me it looks better than having to call mkdirs() from PathData.java.
In the following code:
{quote}
+ stats.length == 1
+ &&
+ stats[0].getPath().equals(fs.makeQualified(globPath)))
+ {
+ // if the fq path is identical to the pattern passed, use the pattern
+ // to initialize the string value
+ items = new PathData[]{ new PathData(fs, pattern, stats[0]) };
{quote}
What happens in cases where stats.length == 1, but the globPath is just a "pattern" and not identical to the path? Would you run in to the same issue? Can you possibly use stats[0].getPath().toUri().getPath() (not pretty either) to obtain the path? Am not sure if it returns what you want it to return, but just throwing it out there....
Very minor comment: In some places "String thePath" is used while in some other places "String thePathString" and "Path thePath". Could thePathString be used for String and thePath be used for Path?
> Refactor FsShell's mkdir
> ------------------------
>
> Key: HADOOP-7236
> URL: https://issues.apache.org/jira/browse/HADOOP-7236
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs
> Affects Versions: 0.23.0
> Reporter: Daryn Sharp
> Assignee: Daryn Sharp
> Attachments: HADOOP-7236.patch
>
>
> Need to refactor tail to conform to new FsCommand class.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HADOOP-7236) Refactor FsShell's mkdir
Posted by "Matt Foley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-7236?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13028977#comment-13028977 ]
Matt Foley commented on HADOOP-7236:
------------------------------------
+1 Thanks.
> Refactor FsShell's mkdir
> ------------------------
>
> Key: HADOOP-7236
> URL: https://issues.apache.org/jira/browse/HADOOP-7236
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs
> Affects Versions: 0.23.0
> Reporter: Daryn Sharp
> Assignee: Daryn Sharp
> Attachments: HADOOP-7236-2.patch, HADOOP-7236-3.patch, HADOOP-7236.patch
>
>
> Need to refactor tail to conform to new FsCommand class.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HADOOP-7236) Refactor FsShell's mkdir
Posted by "Daryn Sharp (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-7236?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13027050#comment-13027050 ]
Daryn Sharp commented on HADOOP-7236:
-------------------------------------
bq. The mkdirs() in PathData.java file: I understand that it was added to help move FsShell to FileContext in an easier way. Since it looks like it might be out of place being in PathData.java, would it make sense to instead move it to Mkdirs.java. I understand that "fs" was moved to PathData to avoid the callers from having to deal/know about the "fs", but to me it looks better than having to call mkdirs() from PathData.java.
Yes, one of my end goals is greatly simplifying the switch to FileContext. I'm trying to make the FileSystem, FileContext, and/or anything else, opaque to the user of a path. This will afford us the ability to transparently use whatever is needed w/o modifying the callers when something changes.
If it was moved to a Mkdirs.java, that would set the precedent for every fs operation being a separate class which I don't think is very desirable. Having a mkdirs(PathData) would require that method to reach into the guts to get the fs or context which defeats the purpose of "hiding" it.
Regarding that horrible chunk of code that processes paths w/o an authority... The comment before it starts with "this is very nasty". It's entirely for backwards compatibility. If I were to do it for all paths, then other tests break. :( I want the initial redesign to be fully backwards compatible, then we can work on eliminating the gross inconsistencies in the code.
Sure, I'll make the references to path/string more consistent.
Thanks for the comments!
> Refactor FsShell's mkdir
> ------------------------
>
> Key: HADOOP-7236
> URL: https://issues.apache.org/jira/browse/HADOOP-7236
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs
> Affects Versions: 0.23.0
> Reporter: Daryn Sharp
> Assignee: Daryn Sharp
> Attachments: HADOOP-7236.patch
>
>
> Need to refactor tail to conform to new FsCommand class.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HADOOP-7236) Refactor FsShell's mkdir
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-7236?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13028815#comment-13028815 ]
Hadoop QA commented on HADOOP-7236:
-----------------------------------
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12478175/HADOOP-7236-3.patch
against trunk revision 1099284.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 7 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 (version 1.3.9) 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.
+1 system test framework. The patch passed system test framework compile.
Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/396//testReport/
Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/396//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/396//console
This message is automatically generated.
> Refactor FsShell's mkdir
> ------------------------
>
> Key: HADOOP-7236
> URL: https://issues.apache.org/jira/browse/HADOOP-7236
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs
> Affects Versions: 0.23.0
> Reporter: Daryn Sharp
> Assignee: Daryn Sharp
> Attachments: HADOOP-7236-2.patch, HADOOP-7236-3.patch, HADOOP-7236.patch
>
>
> Need to refactor tail to conform to new FsCommand class.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HADOOP-7236) Refactor FsShell's mkdir
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-7236?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13026364#comment-13026364 ]
Hadoop QA commented on HADOOP-7236:
-----------------------------------
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12477664/HADOOP-7236.patch
against trunk revision 1097322.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 7 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 (version 1.3.9) 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.
+1 system test framework. The patch passed system test framework compile.
Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/383//testReport/
Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/383//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/383//console
This message is automatically generated.
> Refactor FsShell's mkdir
> ------------------------
>
> Key: HADOOP-7236
> URL: https://issues.apache.org/jira/browse/HADOOP-7236
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs
> Affects Versions: 0.23.0
> Reporter: Daryn Sharp
> Assignee: Daryn Sharp
> Attachments: HADOOP-7236.patch
>
>
> Need to refactor tail to conform to new FsCommand class.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HADOOP-7236) Refactor FsShell's mkdir
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-7236?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13028994#comment-13028994 ]
Hudson commented on HADOOP-7236:
--------------------------------
Integrated in Hadoop-Common-trunk-Commit #571 (See [https://builds.apache.org/hudson/job/Hadoop-Common-trunk-Commit/571/])
HADOOP-7236. Refactor the mkdir command to conform to new FsCommand class. Contributed by Daryn Sharp
> Refactor FsShell's mkdir
> ------------------------
>
> Key: HADOOP-7236
> URL: https://issues.apache.org/jira/browse/HADOOP-7236
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs
> Affects Versions: 0.23.0
> Reporter: Daryn Sharp
> Assignee: Daryn Sharp
> Fix For: 0.23.0
>
> Attachments: HADOOP-7236-2.patch, HADOOP-7236-3.patch, HADOOP-7236.patch
>
>
> Need to refactor tail to conform to new FsCommand class.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (HADOOP-7236) Refactor FsShell's mkdir
Posted by "Daryn Sharp (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-7236?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Daryn Sharp updated HADOOP-7236:
--------------------------------
Attachment: HADOOP-7236-3.patch
I agree, it felt a little awkward. My motivation was to simplify the move to FileContext or the next new and improved api. That said, you won me over, I'm fine with switching it back.
> Refactor FsShell's mkdir
> ------------------------
>
> Key: HADOOP-7236
> URL: https://issues.apache.org/jira/browse/HADOOP-7236
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs
> Affects Versions: 0.23.0
> Reporter: Daryn Sharp
> Assignee: Daryn Sharp
> Attachments: HADOOP-7236-2.patch, HADOOP-7236-3.patch, HADOOP-7236.patch
>
>
> Need to refactor tail to conform to new FsCommand class.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HADOOP-7236) Refactor FsShell's mkdir
Posted by "John George (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-7236?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13027133#comment-13027133 ]
John George commented on HADOOP-7236:
-------------------------------------
+1. looks good.
> Refactor FsShell's mkdir
> ------------------------
>
> Key: HADOOP-7236
> URL: https://issues.apache.org/jira/browse/HADOOP-7236
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs
> Affects Versions: 0.23.0
> Reporter: Daryn Sharp
> Assignee: Daryn Sharp
> Attachments: HADOOP-7236-2.patch, HADOOP-7236.patch
>
>
> Need to refactor tail to conform to new FsCommand class.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HADOOP-7236) Refactor FsShell's mkdir
Posted by "Matt Foley (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-7236?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13028794#comment-13028794 ]
Matt Foley commented on HADOOP-7236:
------------------------------------
It looks generally good, and this refactoring is very good for the FsShell.
However, I think John George was on the right track when he questioned adding mkdirs(), etc., methods to the PathData class. We seem to already be creating FileContext as a mirror of all FS methods, we shouldn't do the same with PathData. To me it seems obvious that the invocation of mkdirs(), rightly removed from FSCommand, should be located in the Mkdir class.
Daryl and I talked about various ways to try to "simplify the switch to FileContext". We concluded (or at least I did -- sorry if I'm putting words in your mouth :-) ) that we were going through contortions to avoid changing twenty single lines in twenty different files at some point in the future; i.e., converting the invocations of FileSystem methods to FileContext methods in each of the implementations of the various FSCommands. While it would be nice to avoid that future editing, it isn't worth complicating the design, in my opinion. And isn't it kind of the point of all this that each command (or related subset of commands) can become a separate subclass of FSCommand? Thanks.
> Refactor FsShell's mkdir
> ------------------------
>
> Key: HADOOP-7236
> URL: https://issues.apache.org/jira/browse/HADOOP-7236
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs
> Affects Versions: 0.23.0
> Reporter: Daryn Sharp
> Assignee: Daryn Sharp
> Attachments: HADOOP-7236-2.patch, HADOOP-7236.patch
>
>
> Need to refactor tail to conform to new FsCommand class.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (HADOOP-7236) Refactor FsShell's mkdir
Posted by "Daryn Sharp (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-7236?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Daryn Sharp updated HADOOP-7236:
--------------------------------
Attachment: HADOOP-7236-2.patch
Rename all the "theXXX" to "XXX". Consistently use "path" and "pathString".
> Refactor FsShell's mkdir
> ------------------------
>
> Key: HADOOP-7236
> URL: https://issues.apache.org/jira/browse/HADOOP-7236
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs
> Affects Versions: 0.23.0
> Reporter: Daryn Sharp
> Assignee: Daryn Sharp
> Attachments: HADOOP-7236-2.patch, HADOOP-7236.patch
>
>
> Need to refactor tail to conform to new FsCommand class.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (HADOOP-7236) Refactor FsShell's mkdir
Posted by "Daryn Sharp (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-7236?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Daryn Sharp updated HADOOP-7236:
--------------------------------
Status: Patch Available (was: Open)
> Refactor FsShell's mkdir
> ------------------------
>
> Key: HADOOP-7236
> URL: https://issues.apache.org/jira/browse/HADOOP-7236
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs
> Affects Versions: 0.23.0
> Reporter: Daryn Sharp
> Assignee: Daryn Sharp
> Attachments: HADOOP-7236.patch
>
>
> Need to refactor tail to conform to new FsCommand class.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HADOOP-7236) Refactor FsShell's mkdir
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-7236?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13027065#comment-13027065 ]
Hadoop QA commented on HADOOP-7236:
-----------------------------------
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12477809/HADOOP-7236-2.patch
against trunk revision 1097322.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 7 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 (version 1.3.9) 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.
+1 system test framework. The patch passed system test framework compile.
Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/387//testReport/
Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/387//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/387//console
This message is automatically generated.
> Refactor FsShell's mkdir
> ------------------------
>
> Key: HADOOP-7236
> URL: https://issues.apache.org/jira/browse/HADOOP-7236
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs
> Affects Versions: 0.23.0
> Reporter: Daryn Sharp
> Assignee: Daryn Sharp
> Attachments: HADOOP-7236-2.patch, HADOOP-7236.patch
>
>
> Need to refactor tail to conform to new FsCommand class.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (HADOOP-7236) Refactor FsShell's mkdir
Posted by "Tsz Wo (Nicholas), SZE (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-7236?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Tsz Wo (Nicholas), SZE updated HADOOP-7236:
-------------------------------------------
Resolution: Fixed
Fix Version/s: 0.23.0
Hadoop Flags: [Reviewed]
Status: Resolved (was: Patch Available)
I have committed this. Thanks, Daryn!
Also thanks John and Matt for reviewing it.
> Refactor FsShell's mkdir
> ------------------------
>
> Key: HADOOP-7236
> URL: https://issues.apache.org/jira/browse/HADOOP-7236
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs
> Affects Versions: 0.23.0
> Reporter: Daryn Sharp
> Assignee: Daryn Sharp
> Fix For: 0.23.0
>
> Attachments: HADOOP-7236-2.patch, HADOOP-7236-3.patch, HADOOP-7236.patch
>
>
> Need to refactor tail to conform to new FsCommand class.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira