[ 
https://issues.apache.org/jira/browse/LIBCLOUD-309?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13608568#comment-13608568
 ] 

Tomaz Muraus commented on LIBCLOUD-309:
---------------------------------------

Thanks.

The patch looks mostly good, here are some potential issues:

- Mutating an argument which is passed by reference to a method it's usually a 
bad idea, unless it's explicit and documented. In this 
`ssh_alternate_usernames` which means is passed by reference and where you pop 
an element from the list you actually modify the list used passed to the method.

You have two options to avoid that:

1. Create a copy of the list and mutate it
2. Iterate over a list

I personally think copying adds unnecessary work here so I would go with option 
2.

- You catch Exception which is probably not what we want. IIRC, we can catch a 
more specific exception thrown by paramiko which indicats an authentication 
error.
                
> cycle through ssh usernames for script deployment
> -------------------------------------------------
>
>                 Key: LIBCLOUD-309
>                 URL: https://issues.apache.org/jira/browse/LIBCLOUD-309
>             Project: Libcloud
>          Issue Type: Improvement
>          Components: Compute
>            Reporter: Chris Psaltis
>         Attachments: ssh_script_deploy.patch
>
>
> Following up the discussion in the mailing list 
> (http://mail-archives.apache.org/mod_mbox/libcloud-users/201303.mbox/browser) 
> it would be nice to have this feature as long as error handling is 
> transparent and looping is only done if the user requires it.
>  

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to