[ https://issues.apache.org/jira/browse/HADOOP-7575?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13105809#comment-13105809 ]
jirapos...@reviews.apache.org commented on HADOOP-7575: ------------------------------------------------------- ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1921/#review1919 ----------------------------------------------------------- I didn't really spend any time on the Test code. Sorry :( hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LocalDirAllocator.java <https://reviews.apache.org/r/1921/#comment4404> No biggie, but could you please import the specific class you are using? I know the others imported * too, but its a bad practice ( http://stackoverflow.com/questions/147454/why-is-using-a-wild-card-with-a-java-import-statement-bad ) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LocalDirAllocator.java <https://reviews.apache.org/r/1921/#comment4403> Does localDirs[i] need to be modified as well? i.e. is there somewhere else in the code where it might trip up? hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LocalDirAllocator.java <https://reviews.apache.org/r/1921/#comment4402> Do you really need to get the URI? Doesn't passing the Path to the File constructor here have the same effect? hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalDirAllocator.java <https://reviews.apache.org/r/1921/#comment4405> Can we please move these definitions at top with the others since its not being used in this method alone but in later methods too? Its just my opinion. - Ravi On 2011-09-15 20:10:18, Jonathan Eagles wrote: bq. bq. ----------------------------------------------------------- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1921/ bq. ----------------------------------------------------------- bq. bq. (Updated 2011-09-15 20:10:18) bq. bq. bq. Review request for Tom Graves, Jeffrey Naisbitt, Robert Evans, and Ravi Prakash. bq. bq. bq. Summary bq. ------- bq. bq. Contexts with configuration path strings using fully qualified paths (e.g. file:///tmp instead of /tmp) mistakenly creates a directory named 'file:' and sub-directories in the current local file system working directory. bq. bq. bq. This addresses bug HADOOP-7575. bq. http://issues.apache.org/jira/browse/HADOOP-7575 bq. bq. bq. Diffs bq. ----- bq. bq. hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/LocalDirAllocator.java 71c8235 bq. hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalDirAllocator.java 1e22a73 bq. bq. Diff: https://reviews.apache.org/r/1921/diff bq. bq. bq. Testing bq. ------- bq. bq. unit tests included as part of the patch bq. manual tests that verifies qualified paths don't accidentally create a directory named file: bq. bq. bq. Thanks, bq. bq. Jonathan bq. bq. > Support fully qualified paths as part of LocalDirAllocator > ---------------------------------------------------------- > > Key: HADOOP-7575 > URL: https://issues.apache.org/jira/browse/HADOOP-7575 > Project: Hadoop Common > Issue Type: Bug > Components: fs > Affects Versions: 0.23.0 > Reporter: Jonathan Eagles > Assignee: Jonathan Eagles > Priority: Minor > Fix For: 0.23.0 > > Attachments: HADOOP-7575-trunk-v1.patch, HADOOP-7575-trunk-v3.patch, > HADOOP-7575-trunk-v4.patch, HADOOP-7575-trunk-v5.patch, > HADOOP-7575-trunk-v8.patch > > > Contexts with configuration path strings using fully qualified paths (e.g. > file:///tmp instead of /tmp) mistakenly creates a directory named 'file:' and > sub-directories in the current local file system working directory. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira