boto / boto3

AWS SDK for Python
https://aws.amazon.com/sdk-for-python/
Apache License 2.0
9.01k stars 1.87k forks source link

ec2 Instance.tags #264

Open wt opened 9 years ago

wt commented 9 years ago

Should ec2 instance tags be a dict?

Instead they are None when there a no tags or a list of dicts when there are tags. It's not very convenient. It would be nicer it were always a dict that had the tag keys and keys and the tag values as values.

Is this possible?

jamesls commented 9 years ago

To clarify, you're saying that that on an EC2 instance resource, sometimes the .tags attr will be None if you have no tags set:

[i.tags for i in boto3.resource('ec2', 'us-west-2').instances.all()]
[[{u'Key': 'Name', u'Value': 'has a tag'}],
 [{u'Key': 'Name', u'Value': 'also-has-a-tag'}],
 None]  # <--- This is None

Marking as a feature request. We'll need to figure out how feasible this is. I'm thinking we'd do this just at the resources level for now.

kapilt commented 9 years ago

none corresponds to empty dict

wt commented 9 years ago

It would certainly make it easier if it were an empty dict since I can't iterate over the elements in the tags without checking if the tags attribute is None.

russellballestrini commented 8 years ago

I wrote the following function in my botoform project to produce dicts out of the list of dicts that many ec2 objects return.

https://github.com/russellballestrini/botoform/blob/master/botoform/util.py#L124-L130

def make_tag_dict(ec2_object):
    """Given an tagable ec2_object, return dictionary of existing tags."""
    tag_dict = {}
    if ec2_object.tags is None: return tag_dict
    for tag in ec2_object.tags:
        tag_dict[tag['Key']] = tag['Value']
    return tag_dict
jonathanwcrane commented 8 years ago

Chiming in that, indeed, it should return an empty dict rather than "None" if an instance has no tags.

Note this should go for all resources that support tags, not necessarily just EC2 Instances.

wt commented 8 years ago

None is a poor substitute for an empty dict. It make code more verbose in most cases.

rushiagr commented 8 years ago

I'm not clear here.. So you're saying when an instance has no tags, it should be an empty list "[]"? or an empty dict "{}"? Or empty dict in an empty list "[{}]"? I guess what everybody wants, including me, is that instance.tags should return empty list "[]".

russellballestrini commented 8 years ago

He wants boto3 to behave like boto2 in regards to tags.

  1. Empty dictionary instead of None
  2. A dictionary with keys as keys, and tag values as values instead of a list if dictionaries.

On Sun, Dec 13, 2015, 1:13 AM Rushi Agrawal notifications@github.com wrote:

I'm not clear here.. So you're saying when an instance has no tags, it should be an empty list "[]"? or an empty dict "{}"? Or empty dict in an empty list "[{}]"? I guess what everybody wants, including me, is that instance.tags should return empty list "[]".

— Reply to this email directly or view it on GitHub https://github.com/boto/boto3/issues/264#issuecomment-164232217.

wt commented 8 years ago

@russelballestrini, nice summary.

jonathanwcrane commented 8 years ago

I would take a simply empty dict, but I love @russellballestrini 's idea of having boto3's return values be the same as boto's. Though the full implementation of that should be another issue.

wt commented 8 years ago

Has any thought been given to this? This is a pretty serious wart in the API of boto3, which is otherwise so much better than the predecessor.

jonathanwcrane commented 8 years ago

Agreed with @wt 's sentiments of "wart" and "otherwise much better."

russellballestrini commented 8 years ago

For the most part Boto3 has lots of these sorts of odd behaviors compared to boto2. I have found myself writing lots of helpers or wrappers to make the library appear more native to Python.

I still prefer Boto3 to Boto2 because in many regards Boto2 is hopelessly broken for some of its services.

On Wed, Dec 23, 2015, 12:49 PM Warren Turkal notifications@github.com wrote:

Has any thought been given to this? This is a pretty serious wart in the API of boto3, which is otherwise so much better than the predecessor.

— Reply to this email directly or view it on GitHub https://github.com/boto/boto3/issues/264#issuecomment-166955881.

jonathanwcrane commented 8 years ago

Care to share any of those libraries, @russellballestrini ? Maybe we could start using those and eventually they could get merged into boto3 itself.

russellballestrini commented 8 years ago

@jonathanwcrane most of them are in this file: https://github.com/russellballestrini/botoform/blob/master/botoform/util.py

nchammas commented 8 years ago

@russellballestrini

I have found myself writing lots of helpers or wrappers to make the library appear more native to Python.

Same here. I think there is room in the world for a boto3util library or something that makes common operations like tag lookup more Pythonic. Or perhaps the boto3 maintainers will consider adding something like that directly to the main library.

boto:

tag_value = instance.tags['tag_key']

boto3:

# Method 1
tag_value = [tag['Value'] for tag in instance.tags if tag['Key'] == 'tag_key'][0]

# Method 2
for tag in instance.tags:
    if tag['Key'] == 'tag_key':
        tag_value = tag['Value']
        break
else:
    raise KeyError(...)

In the case of tag lookup, the original boto definitely beats boto3 in terms of being idiomatic Python. A given instance's tags represent a set of keys and values, and the keys must be unique. This is pretty much what a dictionary is, and accessing tags as anything other than a single dictionary seems off.

Unfortunately, changing boto3 to follow this behavior at this point would be a backwards-incompatible change, so it will probably never happen. The most practical way out is probably through separate utility methods that the user has to call.

russellballestrini commented 8 years ago

@nchammas I will pair up with you to build out the boto3util library, do you use IRC?

nchammas commented 8 years ago

I'm not in a position to co-own a new library at this time, unfortunately. If you take this on separately, though, I'd be more than happy to provide feedback here on GitHub. (I don't use IRC much these days.)

adamtemple commented 8 years ago

Just to keep the momentum here - I would agree with @russellballestrini and the others in the thread. Having tags for all resources returned simply as dictionary rather than a list of dictionaries is much easier to understand and work with. I actually ended up writing a simply function to 'make_tags_dict' which I have to use in every script to make it easier to parse.

If the data structure must stay as a list of dicts is it not possible to integrate much a helper function to provide this functionality anyway?

russellballestrini commented 8 years ago

@adamtemple I'm considering creating a boto3util library like @nchammas suggested and documenting it really well so people can just include it when working with Boto3 library.

I already started documenting here but I think it should be pulled out into a different repo.

https://botoform.readthedocs.org/en/latest/reference/util.html

adubkov commented 8 years ago

This will create dict from tags.

# convert to old style tags
i = ec2.Instance('i-01234567')
tags = dict(map(lambda x: (x['Key'], x['Value']), i.tags or []))

# convert to new style tags for use in `create_tags` method
t = {'Name': 'test'}
tags = map(lambda (k,v): {'Key':k, 'Value':v}, list(t.items()))

What was the reason to change tags to list of dicts?

russellballestrini commented 8 years ago

Your lambda won't work in the case that instance.tags is None. Check out botoform.util 'make_tag_dict' for a function that works with all tagable objects.

On Fri, Apr 22, 2016, 4:11 PM Alexey Dubkov notifications@github.com wrote:

This will create dict from tags.

i = ec2.Instance('i-01234567') tags = dict(map(lambda x: (x['Key'], x['Value']), i.tags))

What was the reason to change tags to list of dicts?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/boto/boto3/issues/264#issuecomment-213573980

adubkov commented 8 years ago

updated. now it support None

wt commented 8 years ago

Why not add a flag that defaults to the old behavior but allows getting the tags as a dict instead of the current behavior. This really shouldn't be relegated to some other lib. That's just adding unnecessary boilerplate. Eventually, maybe even the default of the flag could be changed (maybe at a major version change) to provide tags as a dict by default.

nchammas commented 8 years ago

Just speaking as a random user, at this point I don't think it's reasonable to expect Instance.tags() to change to return a dictionary, even with a major version bump.

I think the reason the tags come back as a list is because Boto 3 is now automatically structuring output to match the EC2 API. This is a major design goal of Boto 3. It makes things more manageable for the maintainers, who have to keep up with all of AWS's services, but it also makes it extremely unlikely that they will implement custom handling for this one method.

Having tags come back as a list of key-value dictionaries is un-Pythonic and annoying, but it's a minor issue with easy workarounds. I don't think any maintainer would break backwards compatibility to resolve that. The tradeoff doesn't make sense.

Hopefully a maintainer can step in here and clarify things so we don't hold out on false hope that this method will change. Or perhaps I am wrong and there is a practical way to fix this.

wt commented 8 years ago

When (if ever) the default for how tags are represented changes is certainly a reasonable question. However, having an arg to enable tags as a dict doesn't break backward compatibility and minimizes the boilerplate code needed to a single arg. I think adding such a arg a better solution than the current solution of forcing every user to deal with this problem in there own code or the proposed solution of post-processing the results with a helper lib.

Another perhaps better solution may be to have a .tags_dict object that is a dict of the tags. That wouldn't require any boilerplate on the part of folks calling the API and would maintain backward compatibility and would give the dict of tags a home in the lib without any additional manual boilerplate steps for users of the lib.

jonathanwcrane commented 8 years ago

Bumping this for visibility. Having to write some convoluted helper code to deal with tags and googled "boto3 EC2 instance tags" and this is the second result. Please fix this, @danielgtaylor @kyleknap @jamesls @JordonPhillips @rayluo @mtdowling !

wt commented 8 years ago

👍

tedivm commented 7 years ago

This is just silly. Can this get fixed at some point so people don't need weird work arounds?

sbrile commented 7 years ago

bump

wesleykirkland commented 7 years ago

@russellballestrini Thank you for the make_tag_dict function, that made my life much easier.

silviot commented 7 years ago

I'm currently embedding these two functions in every script. It would be nice if they made it into boto3.utils, to confine the boilerplate to an import:

def t2d(tags):
    """Convert a tag list to a dictionary.

    Example:
        >>> t2d([{'Key': 'Name', 'Value': 'foobar'}])
        {'Name': 'foobar'}
    """
    if tags is None:
        return {}
    return dict((el['Key'], el['Value']) for el in tags)

def d2t(tag_dict=None, **kwargs):
    """Convert a dictionary to a tag list.

    Example:
        >>> d2t({'Name': 'foobar'}) == [{'Value': 'foobar', 'Key': 'Name'}]
        True

        >>> d2t(Name='foobar') == [{'Value': 'foobar', 'Key': 'Name'}]
        True
    """
    tag_dict = {} if tag_dict is None else dict(tag_dict)
    tag_dict.update(kwargs)
    return [{'Key': k, 'Value': v} for k, v in tag_dict.items()]
dead10ck commented 5 years ago

Why not just add a method to the ec2.Instance object, like get_tag_value? That would maintain backwards compatibility and eliminate the need for every consumer of the lib to write their own helper function. (Honestly, I'm surprised this doesn't already exist.)

tedivm commented 5 years ago

Seriously, the first thing I do when working with boto3 projects is add this helper function to the system. The fact that this issue has been open for three years is a bit ridiculous at this point. No one wants to have to iterate through a list simply to see what the tag values are for a specific key.