Deprecate using jinja tests as filters

[sivel]

Proposal: Deprecate using jinja tests as filters

Author: Matt Martz <@sivel>

Date: 2017/10/20

• Status: New
• Proposal type: Deprecation
• Targeted Release: 2.5
• Associated PR: https://github.com/ansible/ansible/pull/32361
• Estimated time to implement: 1 day to 1 week

Motivation

Historically we did not expose jinja tests in ansible. As such everything was a filter. Eventually we began exposing certain things as tests, but we kept also registering them as filters to avoid backwards incompatible issues.

Unfortunately we continue to conflate the ideas. People don't understand they are different, they don't know to look on a different docs page for tests. To them they are all just filters.

This also causes problems when you want to use a filter that accepts a test as an argument such as (this example is contrived):

whatever|selectattr('some_attr', 'regex_match', '.*')

The docs for selectattr read:

Filters a sequence of objects by applying a test to the specified attribute of each object, and only selecting the objects with the test succeeding.

If no test is specified, the attribute’s value will be evaluated as a boolean.

Due to our continued conflation of tests and filters, users are going to continue to be confused, and not understand what they can and cannot use where.

Problems

I describe the potential problems this solves above, but it does potentially create problems.

This will add a warning to every use of things like:

result|failed
result|success
result|skipped
result.stdout|regex_search

So this will cause some "headaches" for users, and potentially complaints that come through via issues and emails. However we can make it better by helping the users more in the deprecation warning:

[DEPRECATION WARNING]: Using tests as filters is deprecated. Instead of using result|failed instead use result is failed. This feature will be removed in version 2.9.

Solution proposal

1. Create a closure function that can wrap the test functions that enables us to warn on use
2. In Templar._get_filters iterate through the results of Template._get_tests and wrap all of the test functions with the above mentioned closure
3. The test will still fire as it would have, it will also generate a deprecation warning

Dependencies (optional)

N/A

Testing (optional)

Some tests should be created for this.

Documentation (optional)

Throughout the documentation, we will need to convert our use of tests as filters to correct test syntax. Additionally some notes about the change should be included.

Anything else?

I already have a branch with this implemented, it's just not pushed, and no PR opened yet. I can do this easily enough if it's warranted.

[mikedlr]

I'd like to see a playbook modernisation script which does things like:

• tries to fix these filters and at least warns if it can't
• tries to change from unquoted to quoted tests in cases where the unquoted ones don't work any more
• move from old rds module to new rds_instance module

and all similar things. This will make it less likely that people get stuck behind on older Ansible versions.

[gundalow]

• +1 to useful deprecation warnings
• Will need to be documented on https://github.com/ansible/ansible/blob/devel/docs/docsite/rst/porting_guide_2.5.rst (which should also link to the main documentation on tests)
• Lots of integration tests use the old format. We need to update this (thanks to sdoran who's already been doing this)

[sivel]

This is tentatively approved. @thaumos will talk to "the business" about this change.

[sivel]

I'd like to see a playbook modernisation script which does things like:

I may do something similar to fix the integration tests, I'll see what comes of it.

[thaumos]

I say let's go ahead with the 2.9 removal of this. 4 versions is more than enough time to get people to Do The Right Thing™.

@sivel mentioned his script to fix the integration tests as a thing customer's use; this will be helpful for support (cc @michelleperz) to use as well! I'll make sure they see it.

[sivel]

This has been completed, but will require that we run python hacking/fix_test_syntax.py . from time to time to ensure we are updating our integration tests after merging older PRs.

I've just submitted a new PR for recent changes in https://github.com/ansible/ansible/pull/33346

I'm going to go ahead and close this as the PR to implement this deprecation is merged.