Add jinja2 groupby filter override to cast namedtuple to tuple. Fixes #20098 #20362

sivel · 2017-01-17T18:50:24Z

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

plugins/filters/core.py

ANSIBLE VERSION

v2.3

SUMMARY

On the heels of #20098, it was decided that we wanted a solution for people who didn't have access to jinja2<2.9 and while jinja2==2.9.5 was not yet released.

This PR overrides the groupby filter from jinja2, calls the jinja2 groupby filter, and casts the elements of the return to tuples.

Based on jinja2<2.9 I have established what the return looked like as a repr, and ensured that the fix for 2.9.4 matches.

sivel · 2017-01-17T18:51:54Z

This was submitted per request by @jimi-c

…nsible#20098

mattclay · 2017-01-17T22:38:14Z

test/integration/targets/groupby_filter/runme.sh

@@ -0,0 +1,28 @@
+#!/usr/bin/env bash
+
+set -e


Use set -eux here. This will catch use of undefined variables and will make it easier to see what's going on when the test runs (particularly handy when reading Shippable logs).

Unfortunately, I cannot use -u, I had that there originally. It results in a failure when attempting to activate a virtualenv, with the following error:

bash: _OLD_VIRTUAL_PATH: unbound variable

There is apparently a long history as documented at pypa/virtualenv#150

I started to try defining the missing vars, and stopped at 5 variables.

mattclay · 2017-01-17T22:41:50Z

test/integration/targets/groupby_filter/runme.sh

+
+virtualenv --system-site-packages $PYTHON "${MYTMPDIR}/jinja2"
+
+source "${MYTMPDIR}/jinja2/bin/activate"


Running which pythonfollowing this might be a good idea. It will make it easy to see the correct python is in use.

I'll give it another shot. I had it there, but it remember it failing on the osx build, which selected a different python than the venv that is used in the tests. Just not setting --python in virtualenv seemed to fix it.

Ah, sorry I wasn't clear. I didn't mean to change how you're selecting the python version. I just meant that it would be good to run which python after activating the venv, so it was clear which python was now used.

I actually need to improve ansible-test a bit in this area, as using which python has some odd behavior with the --python option, but that's something I'll deal with later.

I think that is more my mistake for reading your comments on my phone. I've restored my original logic, and applied what you were referencing.

mattclay · 2017-01-17T22:43:38Z

test/integration/targets/groupby_filter/runme.sh

+
+pip install -U jinja2==2.9.4
+
+ansible-playbook -i ../../inventory test_jinja2_groupby.yml -v


Adding "$@" at the end of this (and the following) call to ansible-playbook will allow increasing verbosity of the output by passing in additional args.

mattclay · 2017-01-17T22:51:20Z

test/integration/targets/groupby_filter/runme.sh

+
+MYTMPDIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'mytmpdir')
+
+if [ -f /usr/bin/python3 ]


Could you provide a comment explaining why this is needed?

mattclay · 2017-01-18T00:10:28Z

test/integration/targets/groupby_filter/runme.sh

+#!/usr/bin/env bash
+
+# We don't set -u here, due to pypa/virtualenv#150
+set -e


Looks like -x got dropped with the last changes. On a side-note, if you use -x, you don't need to be so verbose with the messaging about the python version below, since the commands are all shown.

This would suffice with -x enabled:

which python python -V

Sorry, my life apparently revolves around food, and I pushed before I was ready so I could eat dinner. The recent update should restore -x and remove some explicit verbosity in my printing.

…20098 (#20362) * Add jinja2 groupby filter override to cast namedtuple to tuple. Fixes #20098 * Address some of the requested changes * Quoting * Print the python path and version * Be less explicitly verbose, rely on implicit verbosity

abadger · 2017-01-19T20:24:20Z

Merged to devel and cherry-picked (minus tests) to the stable-2.2 branch.

sivel force-pushed the issue/20098 branch from ee766a9 to a1bb01c Compare January 17, 2017 19:45

ansibot added affects_2.3 This issue/PR affects Ansible v2.3 bugfix_pull_request c:plugins/filter needs_triage Needs a first human triage before being processed. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jan 17, 2017

sivel force-pushed the issue/20098 branch from a1bb01c to 9e589c5 Compare January 17, 2017 20:19

Add jinja2 groupby filter override to cast namedtuple to tuple. Fixes a…

970cee9

…nsible#20098

sivel force-pushed the issue/20098 branch from 9e589c5 to 970cee9 Compare January 17, 2017 20:48

sivel mentioned this pull request Jan 17, 2017

Exception in safe_eval() on expr with jinja2 >= 2.9 #20098

Closed

mattclay requested changes Jan 17, 2017

View reviewed changes

sivel added 3 commits January 17, 2017 17:20

Address some of the requested changes

8b39b1e

Quoting

cd136dd

Print the python path and version

60f06cf

mattclay reviewed Jan 18, 2017

View reviewed changes

Be less explicitly verbose, rely on implicit verbosity

ef42e84

mattclay approved these changes Jan 18, 2017

View reviewed changes

gundalow requested a review from jimi-c January 18, 2017 14:44

gundalow assigned jimi-c Jan 18, 2017

gundalow removed the needs_triage Needs a first human triage before being processed. label Jan 18, 2017

ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jan 18, 2017

abadger merged commit 537b3b7 into ansible:devel Jan 19, 2017

sivel mentioned this pull request Apr 11, 2017

Jinja groupby behaves differently on ansible 2.2.2.0 #23495

Closed

ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 5, 2018

ansible locked and limited conversation to collaborators Apr 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add jinja2 groupby filter override to cast namedtuple to tuple. Fixes #20098 #20362

Add jinja2 groupby filter override to cast namedtuple to tuple. Fixes #20098 #20362

sivel commented Jan 17, 2017 •

edited

sivel commented Jan 17, 2017

mattclay Jan 17, 2017

sivel Jan 17, 2017

mattclay Jan 17, 2017

sivel Jan 17, 2017

mattclay Jan 17, 2017

sivel Jan 18, 2017

mattclay Jan 17, 2017

mattclay Jan 17, 2017

mattclay Jan 18, 2017

mattclay Jan 18, 2017

sivel Jan 18, 2017

abadger commented Jan 19, 2017


		virtualenv --system-site-packages $PYTHON "${MYTMPDIR}/jinja2"

		source "${MYTMPDIR}/jinja2/bin/activate"


		pip install -U jinja2==2.9.4

		ansible-playbook -i ../../inventory test_jinja2_groupby.yml -v


		MYTMPDIR=$(mktemp -d 2>/dev/null \|\| mktemp -d -t 'mytmpdir')

		if [ -f /usr/bin/python3 ]

Add jinja2 groupby filter override to cast namedtuple to tuple. Fixes #20098 #20362

Add jinja2 groupby filter override to cast namedtuple to tuple. Fixes #20098 #20362

Conversation

sivel commented Jan 17, 2017 • edited

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

SUMMARY

sivel commented Jan 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abadger commented Jan 19, 2017

sivel commented Jan 17, 2017 •

edited