Menu

#482 DEFAULT_MAPPING_TAG not effective with add_constructor since 0.17.29

closed
nobody
None
minor
bug
2023-10-19
2023-10-17
yan12125
No

Here is an example:

# Simplified from https://github.com/aws/aws-cli/blob/v2/awscli/customizations/eks/ordered_yaml.py
import ruamel.yaml
from collections import OrderedDict

def _ordered_constructor(loader, node):
    loader.flatten_mapping(node)
    return OrderedDict(loader.construct_pairs(node))

content = 'foo: bar'

""" Load an OrderedDict object from a yaml stream."""
yaml = ruamel.yaml.YAML(typ="safe", pure=True)
yaml.Constructor.add_constructor(
    ruamel.yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG,
    _ordered_constructor)

print(yaml.load(content))

And the result changes with newer ruamel.yaml:

  • ruamel.yaml 0.17.28: the output is OrderedDict([('foo', 'bar')]) as expected
  • ruamel.yaml 0.17.29: an exception is thrown as mentioned in https://sourceforge.net/p/ruamel-yaml/tickets/467/
  • ruamel.yaml 0.17.30 and 0.17.35 (the latest version as of writing): the result is a dict {'foo': 'bar'} instead of an OrderedDict.

It seems the issue introduced in 0.17.29 is not completely fixed. Such a difference causes issues for aws-cli, which is strict about data types: https://github.com/aws/aws-cli/issues/7966, https://github.com/aws/aws-cli/issues/8018.

Discussion

  • Anthon van der Neut

    Thanks for the report and the concise example, this made it easy to reproduce.

    The problem is caused by DEFAULT_MAPPING_TAG now being a Tag() instance and since that is a hashable type, it gets inserted in the mapping as such and not like other tags that are inserted as strings.

    This will be solved in the next version (>0.17.35), so you should be able to do something like:

    dmt = yaml.Resolver.DEFAULT_MAPPING_TAG
    if ruamel.yaml.version_info < (0, 17, 37):
        dmt = str(dmt)
    yaml.Constructor.add_constructor(dmt, _ordered_constructor)
    

    which will probably make it obvious to some future reviewer when those extra lines can be removed. I take the DEFAULT_MAPPING_TAG from the actual Resolver that yaml will instantiate on loading, that is shorter it is the same Tag() instance but I think it is better/shorter/potentially_more_future_proof.

     
  • yan12125

    yan12125 - 2023-10-19

    Nice! Looks like hg repo is not updated yet? I'm interested in testing integration with aws-cli.

     
  • Anthon van der Neut

    • status: open --> closed
     
  • Anthon van der Neut

    This should be solved in 0.17.36

    I also makde add_constructor return the old constructor (or None if there was no constructor for the tag) So you can more easily revert if necessary. Since you (and many others) don't subclass the ruamel.yaml.convertor.SafeConvertor, and the tag lookup is currently a class variable, setting the constructor will affect all other instances of YAML(typ='safe') as well,

     
  • yan12125

    yan12125 - 2023-10-19

    Thank you! I can confirm 0.17.39 works fine with unpatched aws-cli.

    This will be solved in the next version (>0.17.35), so you should be able to do something like:

    It's not uncommon that aws-cli only accepts a small range of dependency versions, as the official binary already bundles all dependencies. As a result, I propose to simply require the latest ruamel-yaml: https://github.com/aws/aws-cli/pull/8072

     

Log in to post a comment.