[WIP] force jinja to preserve native types

[jctanner]

Subclasses the CodeGenerator class and refactors visit_Output to exclude the default to_string() class it injects.

SUMMARY

Subclasses the CodeGenerator class and refactors visit_Output to exclude the default to_string() class it injects.

Upstream effort: pallets/jinja#708

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

templar

ANSIBLE VERSION

2.4

[bcoca]

s/stry,bytes,unicode/string_types,text_types/

[bcoca]

this is not compatible across python versions use text_strings, byte_strings from our 'compatiblity' code instead

outdated

[agaffney]

generator, not gnerator

[agaffney]

Camel case and underscores? :)

[bcoca]

sometimes you have to allow for all kinds ...

[abadger]

We could make this more efficient. what types can be given here? Since the original jinja2 code is u"".join(invals) it seems like the minimum assumptions we can make are that invals is an iterable and that all the elements can convert to a text type without unicode exceptions. So maybe something like this:

# We can omit this block if invals is never a string type
if isinstance(invals, six.text_type):
    return invals
elif isinstance(invals, six.binary_type):
    return to_text(invals, errors='surrogate_or_strict')

try:
    if len(invals) == 1:
        # Break out the single value
        return invals[0]
except TypeError:
    # Iterable (by contract) but not a Sequence
    pass

return u''.join(invals)

• This keeps us from instantiating two lists.
• This should be faster even than the jinja2 concat if strings (not inside of a container) are allowed as invals.
• If we don't have to deal with strings, then we can get rid of that block and reduce the number of conditionals.

I can adapt that further if we need to make less assumptions (or if we can make more of them).

[abadger]

Okay, jctanner says that this is always a generator. So this is a better implementation:

from itertools import chain
start = []
for val in invals:
    start.append(val)
    if len(start) > 1:
        break
else:
    if start:
        return start[0]
    return []      
return u''.join(chain(start, invals))

[abadger]

Profiling and jctanner's version is faster. So the only things I can see to do differently:

• remove "if isinstance(invals, list):". after the list comprehension, invals should always be a list.
• u'%s' % x for x in invals note that this isn't 100% safe (for instance b'café' will raise a unicode error) but I assume you took it from some part of jinja2 code so we aren't making a new assumption then.

[jctanner]

Another usecase that came up on IRC today ...

  vars:
    A: '{{ a_node | default(False) }}'
    B: '{{ b_node | default(True) }}'

A and B will preserve their bool types in that case with this patch and no downstream consumers would have to guess at the intent.

[bcoca]

another example of same boolean:

  vars:
    A: '{{ a_node is defined }}'
    B: '{{ b_node is undefined }}'

[cognifloyd]

I'm happy to see that the upstream changes have landed and will be in 2.10. 👍

Any chance this will make it in to Ansible before then to make this feature available when using, say, Jinja 2.9? Or is the plan to wait until 2.10 gets released, and use that?

[halberom]

@cognifloyd, I commented in #ansible-devel with much the same query, and jtanner responded with

12:50 <@jtanner> we can vendor our own copy of the relevant code until it's in mainstream
12:50 <@jtanner> https://github.com/ansible/ansible/pull/23943

[cognifloyd]

Sorry about mis-posting this message moments ago.

Jinja 2.10 is out. What is involved in increasing ansible's version requirements? Is that something that can take a year or two or more? What makes the release "mainstream"? RPM releases in RHEL or packages in other distros or ...?
https://pypi.python.org/pypi/Jinja2/2.10
http://jinja.pocoo.org/docs/2.10/nativetypes/

[jctanner]

New PR #32738