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

Added location filter in list_nodes of CloudStack #737

Closed
wants to merge 1 commit into
base: trunk
from

Conversation

Projects
None yet
2 participants
@schaubl
Contributor

schaubl commented Apr 6, 2016

No description provided.

@@ -1348,7 +1348,7 @@ def list_locations(self):
return locations
def list_nodes(self, project=None):
def list_nodes(self, project=None, location=None):

This comment has been minimized.

@Kami

Kami Apr 6, 2016

Member

It looks like we missed this while merging the original changes - project should really be prefixed with ex_ since it's not part of the standard API.

In any case, I'm fine with this for now.

@Kami

Kami Apr 6, 2016

Member

It looks like we missed this while merging the original changes - project should really be prefixed with ex_ since it's not part of the standard API.

In any case, I'm fine with this for now.

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Apr 6, 2016

Member

Thanks.

As noted in the in-line comment, change looks good to me, but it would also be great if you can add a test case for it :)

Member

Kami commented Apr 6, 2016

Thanks.

As noted in the in-line comment, change looks good to me, but it would also be great if you can add a test case for it :)

@schaubl

This comment has been minimized.

Show comment
Hide comment
@schaubl

schaubl Apr 6, 2016

Contributor

Ok, I will add a test case.

Contributor

schaubl commented Apr 6, 2016

Ok, I will add a test case.

@asfgit asfgit closed this in 2f777ca Apr 13, 2016

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Apr 13, 2016

Member

I merged this change into trunk.

Please follow up with tests as soon as you can.

Thanks!

Member

Kami commented Apr 13, 2016

I merged this change into trunk.

Please follow up with tests as soon as you can.

Thanks!

@schaubl

This comment has been minimized.

Show comment
Hide comment
@schaubl

schaubl Apr 13, 2016

Contributor

@Kami: I added a test. Should I create a new PR since this one is already merged ?

Contributor

schaubl commented Apr 13, 2016

@Kami: I added a test. Should I create a new PR since this one is already merged ?

@schaubl

This comment has been minimized.

Show comment
Hide comment
@schaubl

schaubl Apr 15, 2016

Contributor

@Kami I created a new PR for the test: #754

Contributor

schaubl commented Apr 15, 2016

@Kami I created a new PR for the test: #754

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment