LIBCLOUD 480: Add Snapshot Properties

[cderamus]

https://issues.apache.org/jira/browse/LIBCLOUD-480

I debated adding name as a parameter of the base VolumeSnapshot class to remain consistent with Node, StorageVolume, NodeImage and KeyPair. I decided against it for now, but am happy to make that change if desired.

[Kami]

Standard API only takes volume and name argument so for consistency with the base API, I would get rid of description all together.

[Kami]

Not sure If you have seen this comment.

[cderamus]

Sorry about that, I did actually miss this one. My thought in separating out the description and name was based on Amazon resource tagging. If you look at other resources within the driver, we call ex_create_tags() if the name is set. I just thought it would be advantageous to allow for a shortened snapshot name (eg: DB Backup 1) with a more descriptive string for the snapshot description (eg: Database backup prior to upgrading to MySQL 5.6.12 on Dec. 29th.).

[Kami]

Sorry, maybe I wasn't clear enough in my original comment.

I do like what you did with the name argument. It's the correct approach. I just think we should remove the description argument to comply with the base API.

The reason for that is that the method in the base API doesn't take description argument and it looks like this:

def create_volume_snapshot(self, volume, name):

Currently, not all of the implemented methods comply with the base API, but this is one of the big inconsistencies and pain points we want to fix going forward (see #211).

[cderamus]

Got it. Thanks and I'll keep that in mind moving forward.

[Kami]

So to sum it up - I do think description argument is useful, but until we have a better and automatic API quality checking process in place, I would like to avoid adding additional, non-standard arguments to the methods which are part for the base API (extension methods are fine).

[Kami]

It would also be good to update a fixture which is used with list_volume_snapshots test case to include a tag with a key Name and update a test case to assert that the name is parsed out correctly.

[cderamus]

You got it. I'll update that now.

[Kami]

This is unrelated to this pull request, but I was thinking if we should, for readability, move all those "definitions" outside of the functions to the top of the file where the other "constants' live.

Basically, we would build one big RESOURCE_EXTRA_ATTRIBUTES_MAP dict which maps "resource" (node, storage volume, network, etc.) to extra attributes definition for that resource.

[cderamus]

Ah, similar to how you have INSTANCE_TYPES defined? I do like that and as the driver expands to possibly support additional resources (routes, virtual private gateways, etc.) it would certainly help. I'm actually in the process of updating the _to_image method to use the extra_attributes_map format as well so that's yet another resource which will build its extra dictionary using this method.

[Kami]

Yep.

And at some point later on, we could even make a new libcloud.constants package (libcloud.constants.aws, libcloud.constants.openstack, etc.) where we could put all those driver-specific "constants" to remove clutter from the driver files.

[Kami]

Merged into trunk, thanks!