[ 
https://issues.apache.org/jira/browse/HDFS-456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12746078#action_12746078
 ] 

Suresh Srinivas commented on HDFS-456:
--------------------------------------

Comments:
# {{FSImage.getDirectories()}} in this method, the path for storage directories 
is interpreted only as File. Could this also have URI?
# FSImage.java - when creating new URI please catch specific exception 
{{URISyntaxExcepion}} instead of blanket {{Exception}}
# FSImage.java - has System.out.println - please remove it
# FSImage.java - when trying to process {{name}} as file, on exception, should 
the error say {{"Error while processing element as URI or File: " + name}}
# Interpreting a path as URI, and on failure as file is repeated in multiple 
places. Move this into a util, instead of duplicating the code. Please add unit 
tests to test this method on various platforms to ensure different 
configuration (that is files, windows files, URIs) are handled. I would like to 
review the unit tests to see if we are catching all the conditions. These tests 
need to pass both on Windows and Linux.
# In some of the tests, {{replace(' ', '+');}} has been removed. Not sure if 
this is intentional?
# Please use two spaces instead of tab


> Problems with dfs.name.edits.dirs as URI
> ----------------------------------------
>
>                 Key: HDFS-456
>                 URL: https://issues.apache.org/jira/browse/HDFS-456
>             Project: Hadoop HDFS
>          Issue Type: Bug
>          Components: name-node
>    Affects Versions: 0.21.0
>            Reporter: Konstantin Shvachko
>            Assignee: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: failing-tests.zip, HDFS-456.patch, HDFS-456.patch, 
> HDFS-456.patch, HDFS-456.patch, HDFS-456.patch
>
>
> There are several problems with recent commit of HDFS-396.
> # It does not work with default configuration "file:///". Throws 
> {{IllegalArgumentException}}.
> # *ALL* hdfs tests fail on Windows because "C:\mypath" is treated as an 
> illegal URI. Backward compatibility is not provided.
> # {{IllegalArgumentException}} should not be thrown within hdfs code because 
> it is a {{RuntimException}}. We should throw {{IOException}} instead. This 
> was recently discussed in another jira.
> # Why do we commit patches without running unit tests and test-patch? This is 
> the minimum requirement for a patch to qualify as committable, right?

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to