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

list images does not break in case of invalid OS image types #447

Closed
wants to merge 1 commit into
base: trunk
from

Conversation

Projects
None yet
3 participants
@mgogoulos
Contributor

mgogoulos commented Feb 9, 2015

Make sure that list_images will not break if one of the IMAGE_PROJECTS keys (OS image types) changes in future. Right now if you run list_images with an invalid project name, the connection.request_path is not restored after the exception is run, so all requests that follow will be invalid.

from libcloud.compute.providers import get_driver; from libcloud.compute.types import Provider; driver = get_driver('gce')

c = driver('email', 'key_path', project='name')

c.list_images(ex_project='centos-cloudX')
InvalidRequestError

c.list_nodes()
InvalidRequestError
@erjohnso

This comment has been minimized.

Show comment
Hide comment
@erjohnso

erjohnso Feb 9, 2015

Member

Thanks @mgogoulos - this looks great. @Kami just cut 0.17.0, so I don't think it will get added to the release, but definitely will merge when travis catches up.

Member

erjohnso commented Feb 9, 2015

Thanks @mgogoulos - this looks great. @Kami just cut 0.17.0, so I don't think it will get added to the release, but definitely will merge when travis catches up.

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Feb 9, 2015

Member

@erjohnso If you think this is a big issue, you can also vote -1 and we can cancel the vote and roll out a new release.

Or we can wait and release 0.17.1 soon after 0.17.0 is out.

Member

Kami commented Feb 9, 2015

@erjohnso If you think this is a big issue, you can also vote -1 and we can cancel the vote and roll out a new release.

Or we can wait and release 0.17.1 soon after 0.17.0 is out.

@asfgit asfgit closed this in 7ca1336 Feb 9, 2015

@erjohnso

This comment has been minimized.

Show comment
Hide comment
@erjohnso

erjohnso Feb 9, 2015

Member

I think it's a great fix and hardens the driver, but I don't think we need to hold off on 0.17.0. Adding it the next release is fine.

Thanks again @mgogoulos!

Member

erjohnso commented Feb 9, 2015

I think it's a great fix and hardens the driver, but I don't think we need to hold off on 0.17.0. Adding it the next release is fine.

Thanks again @mgogoulos!

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