-
Notifications
You must be signed in to change notification settings - Fork 925
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CLOUDSTACK: ex_list_nics, ex_attach_nic_to_vm, ex_detach_nic_from_vm + test... #369
Conversation
c217e33
to
2d6de86
Compare
@Runseb let me know if you want to get rid of those indent changes which slipped through, most probably because I switched to pycharm vs pydev. |
Hi @boul yes it would be nice to revert those indent changes. Next release will probably be in couple weeks |
cd5ae38
to
40ae2d3
Compare
Reverted the slipped indents |
|
||
return nics | ||
|
||
def ex_add_nic_to_node(self, node, network, ip_address=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe better and more consistent names would be attach and detach (attach_nic_to_node, detach_nic_from_node).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good suggestion, would you prefer to name the method attach_nic_to_node or ex_attach_nic_to_node, since it's a specific extension?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, sorry, those should include ex_
prefix since they are extension methods.
8c1ed81
to
6cd0d27
Compare
Allright, methods have been renamed (attach/detach vs add/remove) |
Functionality to list NICS and to add/remove them too.
And some extra attribute maps with respect to nics.
Including unit tests
Some cosmetic (indent) change slipped in this commit as well.