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

Added TaggedEC2Object.remove_tags #2610

Merged
merged 5 commits into from Oct 7, 2014

Conversation

Projects
None yet
2 participants
@EaterOA
Contributor

EaterOA commented Sep 13, 2014

Pull #2259 already created an add_tags method to supplement the add_tag call. There should be an analogous remove_tags method, since the DeleteTags API also supports removing multiple tags at once. An immediate use case is being able to delete all the tags of an instance in one request; with only remove_tag, this will unnecessarily require many round trips to AWS.

Tests are included in test_ec2object.py's TestRemoveTags class.

This pull request also fixes Issue #2414 by adding a local check that the values match before deleting the tags.

EaterOA added some commits Sep 13, 2014

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.
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.
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.
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.
@danielgtaylor

This comment has been minimized.

Show comment
Hide comment
@danielgtaylor

danielgtaylor Oct 7, 2014

Member

Thanks for the pull request. LGTM 👍

Member

danielgtaylor commented Oct 7, 2014

Thanks for the pull request. LGTM 👍

@danielgtaylor danielgtaylor self-assigned this Oct 7, 2014

danielgtaylor added a commit that referenced this pull request Oct 7, 2014

Merge pull request #2610 from EaterOA/develop
Added TaggedEC2Object.remove_tags. Fixes #2269, #2414, #2610.

@danielgtaylor danielgtaylor merged commit bce8fcf into boto:develop Oct 7, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment