boto / botocore

The low-level, core functionality of boto3 and the AWS CLI.
Apache License 2.0
1.49k stars 1.09k forks source link

Cloudformation get_template API call returning OrdredDict instead of String #1889

Open zepplen opened 4 years ago

zepplen commented 4 years ago

The botocore client is attempting to decode the TemplateBody attribute on the get_template API call. Code: https://github.com/boto/botocore/blob/develop/botocore/handlers.py#L173-L180

My first issue with this is that it is using OrderedDict instead of allowing the json parser to use normal Dicts. This makes it more difficult to work with the results (yaml can not serialize OrdredDicts). It also is unnecessary, as nothing in a CF template requires ordering on Dicts.

it's also not required to use that type on 3.7 and above: https://mail.python.org/pipermail/python-dev/2017-December/151283.html

The second issue is that this does not work for YAML templates, and will instead return a string. So the user is left needing to correctly handle the difference between a pre-parsed result or a yaml string. Which is a strange API contract.

This behavior feels wrong in the botocore layer, it should be in a boto3 resource.

Is there a better workaround than I have come up with to disable this handler?

from boto3 import Session
import botocore.handlers
boto3 = Session(profile_name='my_profile_name', region_name='us-east-1')
boto3._session.unregister('after-call.cloudformation.GetTemplate', botocore.handlers.json_decode_template_body)

I don't like relying on the _session variable. I could create the botocore object explicitly, and have direct access to the session object, but requiring me to directly use the botocore API instead of the boto3 API seems wrong, and is a decent example of why this behavior probably shouldn't be in the botocore lib.

swetashre commented 4 years ago

@zepplen - Thank you for your post. In practice that field is always returned as a dict but the documentation says that it will return string. I will work with the service team to see what we can do about this.

And considering the fact that the API now will return YAML templates we should make our customization consistent for both json and yaml template. I agree that currently we don’t decode YAML template. I would mark this as enhancement.

You can use botocore events to unregister a handler. Something like this:

import boto3

client = boto3.client('cloudformation')
client.meta.events.unregister('after-call.cloudformation.GetTemplate', json_decode_template_body)

or you can use session.events.unregister to unregister a handler.

zepplen commented 3 years ago

I can understand that it is hard to change this behavior now.

However the path to disable it is unintuitive, undocumented, and unfriendly.

Can we get a parameter added to the get_template call to request the result to always be returned as a string? This solves the discrepancy between YAML and JSON template return type, as well as removing the difficulty of using OrderedDict when trying to re-serializing it back to JSON.

varunchopra commented 3 years ago

This is what happens if you use 100% of your brain.

You add support for YAML templates but you don't account for YAML when you pull a template? It's ridiculous that we have to resort to patching like this to avoid bugs that shouldn't exist in the first place.

@swetashre Why was this tagged as an enhancement? This should already work, it's not a feature request.

swirle13 commented 2 years ago

Can we get some visibility into the progress on this? Currently running into this issue when stubbing some dynamodb responses for unit testing. I've grabbed the exact response content I receive from the call and yet when I go to stub the response, every single element of the response says:

E           botocore.exceptions.ParamValidationError: Parameter validation failed:
E           Invalid type for parameter Items[0].S3Key, value: {'DKLZCT205/Home_1020887276.html'}, type: <class 'set'>, valid types: <class 'dict'>

when the attribute literally is a string in dynamo and has no reason to be a dict in the response.

When I try putting everything in a variable named response and typed out exactly as outlined in the boto3 documentation here, I receive an error:

TypeError: unhashable type: 'dict'

It's frustrating to have documentation be completely incorrect and unreliable and now I'm having to dig through github issues to find a workaround or just slowly iterating through fixes by myself.

I'm using a dynamodb resource in my code I'm testing, which doesn't need the dynamodb type definitions, i.e. dynamodb client get_item call defined in the documentation:

response = table.get_item(
    Key={
        'string': 'string'|123|Binary(b'bytes')|True|None|set(['string'])|set([123])|set([Binary(b'bytes')])|[]|{}
    },
...
)

vs the client call for get_item in the documentation:

response = client.get_item(
    TableName='string',
    Key={
        'string': {
            'S': 'string',
            'N': 'string',
            'B': b'bytes',
            'SS': [
                'string',
            ],
            'NS': [
                'string',
            ],
            'BS': [
                b'bytes',
            ],
            'M': {
                'string': {'... recursive ...'}
            },
            'L': [
                {'... recursive ...'},
            ],
            'NULL': True|False,
            'BOOL': True|False
        }
    },
...
)

Is this due to the stubber not stubbing the resource and instead the client?

tim-finnigan commented 1 year ago

Linking related boto3 issues here: https://github.com/boto/boto3/issues/1237, https://github.com/boto/boto3/issues/1468