Skip to content
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

Add aliyun ecs instance join leave security group #992

Merged

Conversation

SnailJie
Copy link
Contributor

@SnailJie SnailJie commented Feb 24, 2017

Add aliyun ecs instance join leave security group

Description

When I use libcloud in my project, I need use function about "Join/Leave Security Group" for Aliyun ECS instance. But I can not find it in libcloud. That's why I make this pull request.

Two functions are added: join_security_group/leave_security_group

Status

  • done, ready for review

Checklist (tick everything that applies)

  • Code linting (required, can be done after the PR checks)
  • Documentation
  • Tests
  • ICLA (required for bigger changes)

@wsbmilk
Copy link

wsbmilk commented Mar 31, 2017

+10086

Copy link
Contributor

@tonybaloney tonybaloney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution, please follow the changes requested in the code review.

if group_id is None:
raise AttributeError('group_id is required')

if (node.state != NodeState.RUNNING) and \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no parentheses in Python :-) please use a more Pythonic evaluation statement.

:type node: :class:`Node`

:param group_id: security group id.
:type ex_filters: ``str``
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be :type group_id:

if group_id is None:
raise AttributeError('group_id is required')

if (node.state != NodeState.RUNNING) and \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, no parens in Python

:type node: :class:`Node`

:param group_id: security group id.
:type ex_filters: ``str``
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this docstring title is wrong, it should be :type group_id

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, Thank you, I will fix it

fix doc
fix doc
fix for code review
@tonybaloney
Copy link
Contributor

thanks, merging.

asfgit pushed a commit that referenced this pull request Apr 10, 2017
@tonybaloney
Copy link
Contributor

this is merged. I don't know why this is still here.

@asfgit asfgit merged commit 60eafe1 into apache:trunk Apr 21, 2017
asfgit pushed a commit that referenced this pull request Apr 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants