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

[LIBCLOUD-554] Add support for providing filters and VPC IDs to EC2: ex_list_networks() #294

Closed
wants to merge 3 commits into from

Conversation

@zerthimon
Copy link
Contributor

zerthimon commented May 15, 2014

EC2 API allows VPC ID and filters to be provided to list only specific VPCs.
http://docs.aws.amazon.com/AWSEC2/latest/APIReference/ApiReference-query-DescribeVpcs.html
Add support for specifying filters and vpc ids when listing networks (vpc's) with ec2 driver's method ex_list_networks()

:rtype: ``list`` of :class:`EC2Network`
"""
params = {'Action': 'DescribeVpcs'}

if network_ids is not None:
for network_idx, network_id in enumerate(network_ids, 1):

This comment has been minimized.

@Kami

Kami May 16, 2014 Member

We still support Python 2.5 which doesn't support second argument (start) to enumerate function so you need to do it like this - https://github.com/zerthimon/libcloud/blob/LIBCLOUD-554_filters/libcloud/compute/drivers/ec2.py#L4307

"""
Return a list of :class:`EC2Network` objects for the
current region.
:param network_ids: One or more VPC IDs

This comment has been minimized.

@Kami

Kami May 16, 2014 Member

Better docstring would be in order here. For example, something along the lines of "Return only networks which match the provided network IDs. If not provided, a list of all the networks on your account is returned."

@Kami
Copy link
Member

Kami commented May 16, 2014

Adding some tests for this new functionality would be good.

@zerthimon
Copy link
Contributor Author

zerthimon commented May 16, 2014

Something is wrong with the tests:

Providing VPC IDs and filters returns correct number of matching objects when working with EC2 API. In tests the calls return all objects in the fixture (ignores the filter)

Please advice!

@asfgit asfgit closed this in 819f167 May 20, 2014
@Kami
Copy link
Member

Kami commented May 20, 2014

I went ahead, squashed the commits, fixed the tests and merged changes into trunk.

The problem in your case was that you asserted on the response, but you returned the same fixture which contains all the networks. I've changed the tests to assert that the correct query parameters are sent, because that's the thing we are actually interested in and want to test in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.