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-514: Add create/delete tag support into the CloudStack driver. ... #248

Closed
wants to merge 3 commits into
base: trunk
from

Conversation

Projects
None yet
2 participants
@cderamus
Contributor

cderamus commented Feb 8, 2014

...New tests and fixtures are also included.

Chris DeRamus
LIBCLOUD-514: Add create/delete tag support into the CloudStack drive…
…r. New tests and fixtures are also included.
@@ -1692,6 +1692,64 @@ def ex_limits(self):
return limits
def ex_create_tags(self, resource_ids, resource_type, tags):

This comment has been minimized.

@Kami

Kami Feb 8, 2014

Member

Since you need to specify resource_type I assume this means that all the resource IDs need to refer to the resource of the same type.

If this is indeed the case, it would be good to document it.

@Kami

Kami Feb 8, 2014

Member

Since you need to specify resource_type I assume this means that all the resource IDs need to refer to the resource of the same type.

If this is indeed the case, it would be good to document it.

This comment has been minimized.

@cderamus

cderamus Feb 8, 2014

Contributor

You got it.

@cderamus

cderamus Feb 8, 2014

Contributor

You got it.

@Kami

View changes

Show outdated Hide outdated libcloud/compute/drivers/cloudstack.py
:rtype: ``bool``
"""
params = {'resourcetype': resource_type,
'resourceids': ",".join(resource_ids)}

This comment has been minimized.

@Kami

Kami Feb 8, 2014

Member

Single quotes for consistency please.

@Kami

Kami Feb 8, 2014

Member

Single quotes for consistency please.

This comment has been minimized.

@cderamus

cderamus Feb 8, 2014

Contributor

You are battling years of PHP/Python/JS muscle memory where I instinctively use double quotes. I'll try to be better about this :).

@cderamus

cderamus Feb 8, 2014

Contributor

You are battling years of PHP/Python/JS muscle memory where I instinctively use double quotes. I'll try to be better about this :).

@Kami

View changes

Show outdated Hide outdated libcloud/compute/drivers/cloudstack.py
:rtype: ``bool``
"""
params = {'resourceType': resource_type,
'resourceIds': ",".join(resource_ids)}

This comment has been minimized.

@Kami

Kami Feb 8, 2014

Member

Same here.

@Kami

Kami Feb 8, 2014

Member

Same here.

:param resource_type: Resource type (eg: UserVm)
:type resource_type: ``str``
:param tags: A dictionary or other mapping of strings to strings,

This comment has been minimized.

@Kami

Kami Feb 8, 2014

Member

I've checked the docs and it says that for the delete operation, only tag keys are mandatory.

Because of that, it would imo be better and feel more natural if user only needs to specify a list of tag keys to be deleted.

@Kami

Kami Feb 8, 2014

Member

I've checked the docs and it says that for the delete operation, only tag keys are mandatory.

Because of that, it would imo be better and feel more natural if user only needs to specify a list of tag keys to be deleted.

This comment has been minimized.

@cderamus

cderamus Feb 8, 2014

Contributor

Ah okay that works.

@cderamus

cderamus Feb 8, 2014

Contributor

Ah okay that works.

@Kami

View changes

Show outdated Hide outdated libcloud/test/compute/test_cloudstack.py
@@ -496,6 +496,18 @@ def test_ex_limits(self):
self.assertEqual(limits['max_volumes'], 20)
self.assertEqual(limits['max_snapshots'], 20)
def test_ex_create_tags(self):
node = self.driver.list_nodes()[0]
tags = {'Region': 'Candada'}

This comment has been minimized.

@Kami

Kami Feb 8, 2014

Member

O Canada!

@Kami

Kami Feb 8, 2014

Member

O Canada!

This comment has been minimized.

@cderamus

cderamus Feb 8, 2014

Contributor

:)

@cderamus

cderamus Feb 8, 2014

Contributor

:)

Chris DeRamus
Updating ex_delete_tags to use a list of keys as opposed to a diction…
…ary of key/value pairs. Removing double quotes aroud join statements and updated the docstrings.
@Kami

View changes

Show outdated Hide outdated libcloud/test/compute/test_cloudstack.py
def test_ex_delete_tags(self):
node = self.driver.list_nodes()[0]
tags = {'Region': 'Candada'}
resp = self.driver.ex_create_tags([node.id], 'UserVm', tags)

This comment has been minimized.

@Kami

Kami Feb 8, 2014

Member

This should probably be ex_delete_tags and pass tag_keys :)

@Kami

Kami Feb 8, 2014

Member

This should probably be ex_delete_tags and pass tag_keys :)

@Kami

This comment has been minimized.

Show comment
Hide comment
@Kami

Kami Feb 9, 2014

Member

Looks good. Changes merged into trunk.

Thanks.

Member

Kami commented Feb 9, 2014

Looks good. Changes merged into trunk.

Thanks.

@asfgit asfgit closed this in 7d1d1cf Feb 9, 2014

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