From ab464f99456d12a9dcc18a216849a89ffd7ede5e Mon Sep 17 00:00:00 2001 From: Vincent Wong Date: Sat, 13 Sep 2014 12:14:26 -0700 Subject: Added method TaggedEC2Object.remove_tags() Pull Request #2259 already created an add_tags method to supplement the add_tag call. Not sure why there isn't an analogous remove_tags method, since the DeleteTags API also supports removng multiple tags at once. An immediate use case is to be able to delete all the tags of an instance; with only remove_tag, this will unnecessarily involve many round trips to AWS. --- boto/ec2/ec2object.py | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/boto/ec2/ec2object.py b/boto/ec2/ec2object.py index 9edf12ee..7cc7e424 100644 --- a/boto/ec2/ec2object.py +++ b/boto/ec2/ec2object.py @@ -65,7 +65,7 @@ class TaggedEC2Object(EC2Object): def add_tag(self, key, value='', dry_run=False): """ - Add a tag to this object. Tag's are stored by AWS and can be used + Add a tag to this object. Tags are stored by AWS and can be used to organize and filter resources. Adding a tag involves a round-trip to the EC2 service. @@ -117,12 +117,12 @@ class TaggedEC2Object(EC2Object): :type value: str :param value: An optional value that can be stored with the tag. - If a value is provided, it must match the value - currently stored in EC2. If not, the tag will not - be removed. If a value of None is provided, all - tags with the specified name will be deleted. - NOTE: There is an important distinction between - a value of '' and a value of None. + If a value is provided, it must match the value currently + stored in EC2. If not, the tag will not be removed. If + a value of None is provided, the tag will be + unconditionally deleted. + NOTE: There is an important distinction between a value + of '' and a value of None. """ if value is not None: tags = {key: value} @@ -135,3 +135,26 @@ class TaggedEC2Object(EC2Object): ) if key in self.tags: del self.tags[key] + + def remove_tags(self, tags, dry_run=False): + """ + Removes tags from this object. Removing tags involves a round-trip + to the EC2 service. + + :type tags: dict + :param tags: A dictionary of key-value pairs for the tags being removed. + For each key, the provided value must match the value + currently stored in EC2. If not, that particular tag will + not be removed. However, if a value of None is provided, + the tag will be unconditionally deleted. + NOTE: There is an important distinction between a value of + '' and a value of None. + """ + status = self.connection.delete_tags( + [self.id], + tags, + dry_run=dry_run + ) + for key, value in tags.iteritems(): + if key in self.tags: + del self.tags[key] -- cgit v1.2.1 From 74b37400324ad31039f38618d05b9fdaf05815d4 Mon Sep 17 00:00:00 2001 From: Vincent Wong Date: Sat, 13 Sep 2014 12:23:08 -0700 Subject: Removed duplicate code in add_tag and remove_tag Since add_tags is a more general version of add_tag that uses the same API call, add_tag can simply call add_tags with the correct parameters. Likewise for remove_tag and remove_tags. For remove_tag in particular, these lines were redundant: if value is not None: tags = {key : value} else: tags = [key] Since EC2Connection.delete_tags just converts the list back to a dict of {key: None} anyway, these extra checks serve no purpose. --- boto/ec2/ec2object.py | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/boto/ec2/ec2object.py b/boto/ec2/ec2object.py index 7cc7e424..8bc428db 100644 --- a/boto/ec2/ec2object.py +++ b/boto/ec2/ec2object.py @@ -77,14 +77,7 @@ class TaggedEC2Object(EC2Object): If you want only the tag name and no value, the value should be the empty string. """ - status = self.connection.create_tags( - [self.id], - {key: value}, - dry_run=dry_run - ) - if self.tags is None: - self.tags = TagSet() - self.tags[key] = value + self.add_tags({key: value}, dry_run) def add_tags(self, tags, dry_run=False): """ @@ -124,17 +117,7 @@ class TaggedEC2Object(EC2Object): NOTE: There is an important distinction between a value of '' and a value of None. """ - if value is not None: - tags = {key: value} - else: - tags = [key] - status = self.connection.delete_tags( - [self.id], - tags, - dry_run=dry_run - ) - if key in self.tags: - del self.tags[key] + self.remove_tags({key: value}, dry_run) def remove_tags(self, tags, dry_run=False): """ -- cgit v1.2.1 From c269cbe996d28fef383ab7a5dbb0b1ae6b0015af Mon Sep 17 00:00:00 2001 From: Vincent Wong Date: Sat, 13 Sep 2014 13:24:00 -0700 Subject: Fixed remove_tags not checking values, fixed test Fixed Issue #2414. remove_tags now locally verifies that the value given for a key matches the one in self.tags before deleting it. Fixed TestRemoveTags.test_remove_tag_empty_value in ec2/test_ec2object. An empty string is _not_ supposed to act like None to unconditionally delete the tag. The original test acted like it does. --- boto/ec2/ec2object.py | 3 ++- tests/unit/ec2/test_ec2object.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/boto/ec2/ec2object.py b/boto/ec2/ec2object.py index 8bc428db..2a43ed47 100644 --- a/boto/ec2/ec2object.py +++ b/boto/ec2/ec2object.py @@ -140,4 +140,5 @@ class TaggedEC2Object(EC2Object): ) for key, value in tags.iteritems(): if key in self.tags: - del self.tags[key] + if value is None or value == self.tags[key]: + del self.tags[key] diff --git a/tests/unit/ec2/test_ec2object.py b/tests/unit/ec2/test_ec2object.py index 4ece8d92..5664c1bc 100644 --- a/tests/unit/ec2/test_ec2object.py +++ b/tests/unit/ec2/test_ec2object.py @@ -138,7 +138,8 @@ class TestRemoveTags(AWSMockServiceTestCase): 'SignatureVersion', 'Timestamp', 'Version']) - self.assertEqual(taggedEC2Object.tags, {"key2": "value2"}) + self.assertEqual(taggedEC2Object.tags, + {"key1": "value1", "key2": "value2"}) if __name__ == '__main__': -- cgit v1.2.1 From 2a8c5729e917d4161ea90cab950408131b851b36 Mon Sep 17 00:00:00 2001 From: Vincent Wong Date: Sat, 13 Sep 2014 13:35:49 -0700 Subject: Added tests for TaggedEC2Object.remove_tags There are 3 tests: test_remove_tags simply makes sure multiple tags are deleted in one request. test_remove_tags_wrong_values makes sure tags that are provided wrong values are not deleted. test_remove_tags_none_values make sure that tags provided with None values are unconditionally deleted. --- tests/unit/ec2/test_ec2object.py | 65 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/tests/unit/ec2/test_ec2object.py b/tests/unit/ec2/test_ec2object.py index 5664c1bc..14841e91 100644 --- a/tests/unit/ec2/test_ec2object.py +++ b/tests/unit/ec2/test_ec2object.py @@ -141,6 +141,71 @@ class TestRemoveTags(AWSMockServiceTestCase): self.assertEqual(taggedEC2Object.tags, {"key1": "value1", "key2": "value2"}) + def test_remove_tags(self): + self.set_http_response(status_code=200) + taggedEC2Object = TaggedEC2Object(self.service_connection) + taggedEC2Object.id = "i-abcd1234" + taggedEC2Object.tags["key1"] = "value1" + taggedEC2Object.tags["key2"] = "value2" + + taggedEC2Object.remove_tags({"key1": "value1", "key2": "value2"}) + + self.assert_request_parameters({ + 'ResourceId.1': 'i-abcd1234', + 'Action': 'DeleteTags', + 'Tag.1.Key': 'key1', + 'Tag.1.Value': 'value1', + 'Tag.2.Key': 'key2', + 'Tag.2.Value': 'value2'}, + ignore_params_values=['AWSAccessKeyId', 'SignatureMethod', + 'SignatureVersion', 'Timestamp', + 'Version']) + + self.assertEqual(taggedEC2Object.tags, {}) + + def test_remove_tags_wrong_values(self): + self.set_http_response(status_code=200) + taggedEC2Object = TaggedEC2Object(self.service_connection) + taggedEC2Object.id = "i-abcd1234" + taggedEC2Object.tags["key1"] = "value1" + taggedEC2Object.tags["key2"] = "value2" + + taggedEC2Object.remove_tags({"key1": "value1", "key2": "value3"}) + + self.assert_request_parameters({ + 'ResourceId.1': 'i-abcd1234', + 'Action': 'DeleteTags', + 'Tag.1.Key': 'key1', + 'Tag.1.Value': 'value1', + 'Tag.2.Key': 'key2', + 'Tag.2.Value': 'value3'}, + ignore_params_values=['AWSAccessKeyId', 'SignatureMethod', + 'SignatureVersion', 'Timestamp', + 'Version']) + + self.assertEqual(taggedEC2Object.tags, {"key2": "value2"}) + + def test_remove_tags_none_values(self): + self.set_http_response(status_code=200) + taggedEC2Object = TaggedEC2Object(self.service_connection) + taggedEC2Object.id = "i-abcd1234" + taggedEC2Object.tags["key1"] = "value1" + taggedEC2Object.tags["key2"] = "value2" + + taggedEC2Object.remove_tags({"key1": "value1", "key2": None}) + + self.assert_request_parameters({ + 'ResourceId.1': 'i-abcd1234', + 'Action': 'DeleteTags', + 'Tag.1.Key': 'key1', + 'Tag.1.Value': 'value1', + 'Tag.2.Key': 'key2'}, + ignore_params_values=['AWSAccessKeyId', 'SignatureMethod', + 'SignatureVersion', 'Timestamp', + 'Version']) + + self.assertEqual(taggedEC2Object.tags, {}) + if __name__ == '__main__': unittest.main() -- cgit v1.2.1 From 97b67523e5511498371502db1ef5bc72ee2e4c8f Mon Sep 17 00:00:00 2001 From: Vincent Wong Date: Sat, 13 Sep 2014 14:49:12 -0700 Subject: Fixed TaggedEC2Object.remove_tags failing in Python 3 --- boto/ec2/ec2object.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boto/ec2/ec2object.py b/boto/ec2/ec2object.py index 2a43ed47..fa50a9fc 100644 --- a/boto/ec2/ec2object.py +++ b/boto/ec2/ec2object.py @@ -138,7 +138,7 @@ class TaggedEC2Object(EC2Object): tags, dry_run=dry_run ) - for key, value in tags.iteritems(): + for key, value in tags.items(): if key in self.tags: if value is None or value == self.tags[key]: del self.tags[key] -- cgit v1.2.1