Skip to content

Conversation

@bernieke
Copy link
Contributor

Implements storage volume methods for the openstack driver.

@Kami
Copy link
Member

Kami commented Jun 28, 2013

Thanks.

I quickly glanced over it and this patch looks good and fits into the current model.

I still need to do a proper review, but I plan to merge it if there aren't any objections after I perform the review.

@alex is currently also looking into improving block storage support in Libcloud so we should also figure out how this fits into his stuff.

@alex
Copy link
Contributor

alex commented Jun 28, 2013

I think this fits in well with the model we're discussing now on the list, so I'll try to give this a review.

@Kami
Copy link
Member

Kami commented Jun 28, 2013

@alex Sweet, thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem to handle snapshots, so it should raise an error if a snapshot is provided.

@alex
Copy link
Contributor

alex commented Jun 28, 2013

Besides these comments, this looks good to me.

@bernieke
Copy link
Contributor Author

Awesome.

I'm on vacation next week, but as soon as I get back I'll check the behavior of openstack in these cases, and make the necessary changes.

Thanks for the quick turnaround!

@bernieke
Copy link
Contributor Author

bernieke commented Jul 8, 2013

I've addressed your remarks in a second commit: https://github.com/bernieke/libcloud/commit/456261fb379f5629ce0f047a757b8307ce181dca

Copy link
Member

Choose a reason for hiding this comment

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

Here, this should probably be OpenStackException and not CloudStackException :)

We recently also added a new ProviderError base exception class (

class ProviderError(LibcloudError):
) for error like this, so please make sure this class inherits from it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've switched to the ProviderError exception class, and fixed the name of the exception (good catch :))

This is the commit with the change: https://github.com/bernieke/libcloud/commit/c72e6bafc3b75716094454fa404236644d11df20

@Kami
Copy link
Member

Kami commented Jul 11, 2013

Looks good, please attach patch to your JIRA ticket and I'll get it merged.

@bernieke
Copy link
Contributor Author

commits squashed, patch attached to https://issues.apache.org/jira/browse/LIBCLOUD-353

@bernieke bernieke closed this Jul 11, 2013
@bernieke bernieke deleted the LIBCLOUD-353_implement_storageVolume_methods_for_openstack branch July 11, 2013 13:32
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.

3 participants