[ 
https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12719323#action_12719323
 ] 

Luca Telloli edited comment on HADOOP-5832 at 6/15/09 2:06 AM:
---------------------------------------------------------------

> Could you confirm if any of the error handling is different compared current 
> trunk? If there are any differences could you list them here? 

My approach has been to catch any new exception and throw a new IOException 
with its content 

> Please update hdfs-default.xml for dfs.name.dir. Mainly the description needs 
> to be updated for new syntax.

Done

> May be a warning when there is no schema would help get configs updated.

Done 

> Indentation "fixes" : I don't see any reason do indentation fixes far away 
> from actual changes for this patch. Some of these are not even fixes. A good 
> rule of thumb I follow is that if I have a hunk in the patch that has only 
> these changes, I remove it from my patch. That is easy to do. On the plus 
> side, you won't get 'svn blame'd for bugs around there .

It's a consequence of using Eclipse in a different configuration. Just setting 
the values of tabstop to 2 and whitespace instead of tab proved not to be 
sufficient to avoid these situations. Although I tried to fix most of them 
manually, there might be a couple of leftovers. 

Finally, I think the approach should be: 
- non URI: consider it valid, use as file:// and warn the user, as you suggested
- unknown uri scheme: the scheme should be among the ones specified inside the 
JournalType enumeration, otherwise hard error 

I'm posting a new patch that fixes the issues you stated above, please let me 
know your impressions on it

      was (Author: lucat):
    > Could you confirm if any of the error handling is different compared 
current trunk? If there are any differences could you list them here? 

My approach has been to catch any new exception and throw a new IOException 
with its content 

> Please update hdfs-default.xml for dfs.name.dir. Mainly the description needs 
> to be updated for new syntax.

Done

> May be a warning when there is no schema would help get configs updated.

Done 

> Indentation "fixes" : I don't see any reason do indentation fixes far away 
> from actual changes for this patch. Some of these are not even fixes. A good 
> rule of thumb I follow is that if I have a hunk in the patch that has only 
> these changes, I remove it from my patch. That is easy to do. On the plus 
> side, you won't get 'svn blame'd for bugs around there .

It's a consequence of using Eclipse in a different configuration. Just setting 
the values of tabstop to 2 and whitespace instead of tab proved not to be 
sufficient to avoid these situations. Although I tried to fix most of them 
manually, there might be a couple of leftovers. 

Finally, I think the approach should be: 
- non URI: consider it valid, use as file:// and warn the user, as you suggested
- unknown uri scheme: the usi should be specified inside the JournalType 
enumeration, otherwise hard error 

I'm posting a new patch that fixes the issues you stated above, please let me 
know your impressions on it
  
> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different 
> schemes than just file. As an advantage, Java supports the constructor 
> File(URI) so the transition is straightforward for files. 

-- 
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