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

Aaron Kimball commented on HADOOP-6108:
---------------------------------------

README.txt
34] you seem to pick an arbitrary AMI. How's this chosen? Is there a list of 
appropriate AMIS somewhere?

58] "with with"

integration-test/transient-cluster.sh]
1) /bin/bash -> /usr/bin/env bash
25) Is it really necessary to source ~/.bashrc? The /bin/bash on the shebang 
line should take care of this.

persistent-cluster.sh]
same two concerns as above.

34) the `sleep 5` can be pushed above the {{if}} and used only once.


teststorage.py]
TestJsonVolumeSpecManager really needs some comments; there's a number of 
multiply-nested arrays being dereferenced and it's not obvious what these 
prove.  Is this just testing that the "spec" record earlier in the file was 
parsed correctly? 

hadoop-ec2-init-remote.sh ]
96) consider putting this in /etc/profile too, so that it affects all users, 
not just root.
171) given EBS replication, is it ok to go to dfs rep 2 here? 

VERSION ] should this file match the version associated with the hadoop-common 
project at large?

cli.py]

instead of a million 'from.... import' commands, consider 'import 
hadoop.cloud.commands as commands', then use commands.MASTER, 
commands.attach_storage(), etc.


254) {{_prompt}} consider being case-insensitive here.
291) in the event of a timeout, should you still print the master url? Is there 
definitely a master? Please add a comment saying why further error handling is 
not needed. A message suggesting follow-up action items for the user (e.g. 
"check hadoop-ec2 list to make sure it really booted") would be helpful too.

329) ditto.

346) since we're already parsing a config file, can the proxy port be 
configurable? What if the user's got two clusters running simultaneously?

461) Add a message saying how to get usage/help text out of the app?


cluster.py]

get_cluster() 33)  unpythonic key check; this should throw KeyError if provider 
is not found.

Cluster stub functions ) For mandatory methods (e.g. get_provider_code), how 
about {{raise Exception("Unimplemented")}} instead of {{pass}}, or maybe add 
UnimplementedException / NotYetImplemented / Something similar?

commands.py]

36) Maybe add a comment describing high-level purpose of ROLES, and examples of 
potential future use (e.g., "HBase support might add new roles here, as would 
divorced JT/NN, etc.")

launch_master() 83) better error msg / comment explaining how this timeout is 
handled in the rest of the method's logic?


launch_slaves() 190) ditto.

231) Since we support both Hadoop 0.18 and 0.20 in the documentation, is there 
additional logic required to gracefully support both cluster versions in this 
code here?

storage.py]

JsonVolumeManager._load() ) why not just percolate the IOError? Do we want file 
errors to silently initialize an empty vol manager? (Is this file something the 
user specifies, or an optional file? If the former, this should fail hard. If 
the latter, silent continue seems ok.)

class Storage ) same comment as above re. NotYetImplemented vs. {{pass}}


util.py ]

bash_quote() ) do you need to double-escape backslashes? The unit tests also 
don't cover internal backslash characters.

ec2.py ]
authorize_role() 150) Why not just catch InvalidPermission.Duplicate?


> Add support for EBS storage on EC2
> ----------------------------------
>
>                 Key: HADOOP-6108
>                 URL: https://issues.apache.org/jira/browse/HADOOP-6108
>             Project: Hadoop Common
>          Issue Type: New Feature
>          Components: contrib/ec2
>            Reporter: Tom White
>            Assignee: Tom White
>         Attachments: HADOOP-6108.patch, HADOOP-6108.patch
>
>
> By using EBS for namenode and datanode storage we can have persistent, 
> restartable Hadoop clusters running on EC2.

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