Ansible.module_utils.six is deprecated - what to do for collections that support both ansible-core 2.16 (or even before) and 2.20?

Project Discussions Collection Development
, , , ,
1

Recently ansible-core’s ansible-test’s pylint sanity test started to flag ansible.module_utils.six imports as ansible-bad-import-from. This was easy to ignore with some tests/sanity/ignore-2.20.txt entries, but raised the question whether ansible-core is planning to remove their vendored six or not (there was no changelog entry, and there’s no porting guide yet). I created an issue to get clarification on the status of ansible.module_utils.six, and the result was that it should be deprecated (apparently the deprecation will go to ansible-core 2.20, since the removal version is 2.24).

Unfortunately this is implemented in a way that every single plugin and module that imports ansible.module_utils.six will result in a deprecation message shown to the user. This means that every collection that uses ansible.module_utils.six right now will annoy users of ansible-core 2.20, unless they stop importing ansible.module_utils.six. Now getting rid of ansible.module_utils.six is pretty easy if the affected code is Python 3 only, like in plugin code if you support ansible-core 2.12+, or in all code if you support ansible-core 2.17+ (ansible-core Python support matrix). But if you have a collection that still supports Python 2.7 for modules (by supporting ansible-core 2.16 or older), like for example community.general (up to version 11, the currently released major version), you can’t simply get rid of ansible.module_utils.six.

One solution would of course be to vendor six yourself in your own collection. Unfortunately, that doesn’t work well, since six contains a large list of “virtual” submodules (of the form six.moves.xxx) that do not correspond to files, and which lead to errors when using them in ansible-core’s ansiballz module_utils file collector. (It will look for explicit files, which don’t exist, and then it will fail.)

All collections that still need to support Python 2 (for whatever reasons) and also ansible-core 2.20 (for example because they don’t want to do a new major release for no other reason than to stop supporting older ansible-core versions) seem to have basically two choices:

  1. Continue to use ansible.module_utils.six, which means all users using ansible-core 2.20+ will see ugly deprecation message for every plugin/module from that collection they use that uses six;
  2. Somehow work around this by conditional imports, lots of if sys.version_info[0] == 2:, vendoring some own versions of six that don’t make use of “virtual” submodules, etc.

For most collections I maintain, I implemented 2. This is ugly, and in cases like community.docker resulted in a lot of changes and boilerplate that I can remove again for the next major release (PRs 1, 2, 3, 4). I’m not happy about this, but I think it was worth the effort, as I really don’t want to annoy users.

But now there’s community.general. I’ve created a PR to remove all six usage from plugins (community.general hasn’t supported Python 2 for non-modules for some time now), which was easy. But there are also a lot of module utils and modules that use six; in total, there are 105 Python files:

  • 20 module utils (which means a lot more modules that use them);
  • 63 modules;
  • 22 unit tests for modules or module utils.
$ grep -Rh ansible.module_utils.six $(find plugins/ tests/unit/ -name '*.py') | sort | uniq -c
      1 from ansible.module_utils.six import binary_type, string_types, text_type
      1 from ansible.module_utils.six import binary_type, text_type
      1 from ansible.module_utils.six import integer_types
      1 from ansible.module_utils.six import integer_types, string_types
      9 from ansible.module_utils.six import iteritems
      3 from ansible.module_utils.six import iteritems, string_types
      1 from ansible.module_utils.six import PY2
      5 from ansible.module_utils.six import PY3
      1 from ansible.module_utils.six import python_2_unicode_compatible
      1 from ansible.module_utils.six import raise_from
      1 from ansible.module_utils.six import raise_from, PY2
     16 from ansible.module_utils.six import StringIO
      6 from ansible.module_utils.six import string_types
      2 from ansible.module_utils.six import text_type
      1 from ansible.module_utils.six import text_type, binary_type
      2 from ansible.module_utils.six.moves.collections_abc import Mapping
      2 from ansible.module_utils.six.moves.collections_abc import MutableMapping
      2 from ansible.module_utils.six.moves import configparser
      1 from ansible.module_utils.six.moves import configparser, StringIO
      2 from ansible.module_utils.six.moves import http_client
      2 from ansible.module_utils.six.moves import http_cookiejar as cookiejar
      4 from ansible.module_utils.six.moves import shlex_quote
      2 from ansible.module_utils.six.moves import xmlrpc_client
      1 from ansible.module_utils.six.moves import xrange
      1 from ansible.module_utils.six.moves import zip_longest
      3 from ansible.module_utils.six.moves.urllib.error import HTTPError
      2 from ansible.module_utils.six.moves.urllib.error import URLError, HTTPError
      1 from ansible.module_utils.six.moves.urllib import error as urllib_error
      7 from ansible.module_utils.six.moves.urllib.parse import quote
      1 from ansible.module_utils.six.moves.urllib.parse import quote_plus
     26 from ansible.module_utils.six.moves.urllib.parse import urlencode
      1 from ansible.module_utils.six.moves.urllib.parse import urlencode, quote
      3 from ansible.module_utils.six.moves.urllib.parse import urljoin
      7 from ansible.module_utils.six.moves.urllib.parse import urlparse
      1 from ansible.module_utils.six.moves.urllib.parse import urlparse, urlencode, urlunparse
      1     from ansible.module_utils.six.moves.urllib.parse import urlparse, urlunparse
      1 from ansible.module_utils.six.moves.urllib.parse import urlparse, urlunparse
      1 from ansible.module_utils.six.moves.urllib.request import getproxies
      1 from ansible.module_utils.six.moves.urllib.request import pathname2url
      1 import ansible.module_utils.six.moves.urllib.error as urllib_error

Fixing them all would be possible, but require some more effort. Especially for the ansible.module_utils.six.moves.xxx imports this at least doubles the work necessary to eventually move everything to Python 3 only (which will happen for community.general 12.0.0, to be released later this year).

What do you think? Should we fix this, so that community.general 10 and 11 work fine with ansible-core 2.20.0 (as promised)? Or should we add known_issues changelog/porting guide entries that basically tell the user “you have to live with the deprecation messages unless you update community.general to 12.0.0”?

2 Likes
2

I think Python 2 has had support way way beyond its welcome. There’s no way to satisfy everyone, so in that case I would choose the option that points towards the future, rather than the one towards the past.

3

I’m not involved in community.general, but my personal opinion: If 10 and 11 will be EOL before ansible.module_utils.six is finally removed from ansible-core, go the known issue way. Implementing workarounds in 105 Python files means a lot of work, and I think it’s better to invest this time to solve other problems.

At the end of the day, the collection still works with ansible-core 2.20. Users just get an annoying warning.

1 Like
4

Would it be better to revert the follow up change we did for the deprecation and the pylint check?

Then all you need to do is add ignores, and we can re-introduce the deprecation for 2.21.

5

I think the main problem here is that it’s impossible to separate deprecations meant for collection developers from deprecations meant for end-users. Right now, there is only one way to programatically deprecate things in Ansible land, and that’s always shown to the end-user. (And the worst part is that this deprecation is often hidden from the collection developer, unless they also happen to be end-users of their collection.)

The ansible.module_utils.six deprecation is meant for collection developers, and not for end-users, since the latter can do nothing about this deprecation (except stop using the collection, or hoping there’s a newer release that stops using the deprecated part of ansible-core). They cannot even disable (“hide”) the deprecation without hiding all deprecations (including ones aimed at them, where action from their side is required).

Reverting the change (and re-applying it for ansible-core 2.21) only delays the problem, since the same problem will then happen with ansible-core 2.21. At that point there will be more collections that no longer support Python 2.7, but the general issue with deprecations aimed at end-user vs. collection developer is still there.

Regarding user-facing vs. developer-facing deprecations, I’m also not sure how to solve this better. (Except make it easier for collection developers to know about these deprecations so they can react on them. Right now it’s unnecessarily hard for them to find out about these.) And sooner or later collection-facing deprecations should also be known to end-users so they can potentially make decisions based on them. Maybe a more fine-grained way to hide deprecations would help here, so that users can ignore the ansible.module_utils.six deprecation without ignoring all other (and potentially very important) deprecations.

1 Like
6

Would it be possible to get the developer-facing deprecations in the sanity tests only, and no user-facing deprecations at all in a case like this?

I’m not sure how this is implemented, maybe the sanity tests only complain about deprecations that are also shown to users. But if not this would be a good solution, wouldn’t it? Collection owners would see it in the sanity tests, while users wouldn’t be bothered.

BTW I’ve seen the sanity tests failing because ansible.module_utils.six but nothing about ansible.module_utils.common._collections_compat ansible.module_utils._text. I’ve only realised this when running tests to check that my changes work (see here). So it looks like it is possible to have user-facing deprecation warnings without sanity tests complaining. So I guess it should be also possible to do it the other way around…?

7

The main reason the deprecations are user facing is that they are more effective pressure on developer, they’ll normally cause users to open ticket. Relying on developers to run sanity tests does not normally get good results and ends up in catastrophic breakages to the user without warning, which then creates tickets on the wrong tool.

8

Why not have a two-staged approach? First, for n versions of ansible-core, only show deprecation warnings in sanity tests. Then, for m versions of ansible-ocre, show them both in sanity tests and to users. And after n + m versions, actually remove the feature.

Also, there’s still the problem of not being able to disable specific deprecation warnings as a user. This contributes to the problem that the current way how deprecations work in ansible-core is a huge mess.

9

Yes, I understand this. But how about first adding those warnings to the developer-facing sanity tests (for example in 2.20) and add user-facing warnings in the next release (2.21)? This way, collection developers / owners get a chance to fix stuff before users are bothered.

I mean, the current approach bothers users with deprecation warnings before collection developers even had a chance to fix them. (Apropos of nothing, there’s no porting guide yet for 2.20 so I can’t even read up what I should work on.) Additionally, this can lead to users opening issues while collection developers already started to work on fixing them, or even did but just didn’t do a new release yet. That’s a bit unfair on the collection owners. They get those issues and have to answer: Yeah, we know, we’re already working on this.

So my suggestion: Deprecation warnings in the sanity tests first to give collection developers (who run sanity tests :smirking_face:) a chance to work on them, and user-facing warnings later to put pressure on the developers.

edit: Looks like @felixfontein has been slightly faster than me, but proposes basically the same idea that I did :smiley:

10

Since showing deprecations in sanity tests can be hard to implement (since some of them depend on what exactly is called with which parameters), it might be more helpful if unit tests and integration tests would show and allow to fail on deprecations, with a mechanism to mark specific deprecations as “ack, I know this is deprecated, but this still is showing up here” (which is related to being able to ignore specific deprecation messages for end-users).

1 Like
11

@mariolenz that it bothers users before developers see it, means that those developers rarely follow core, nor use devel or milestones to tests, nor betas nor release candidates. More to my point about the low effectiveness of that approach

12

@bcoca that’s not true, it only means that they don’t use their own collection as an end-user. Current development tools (i.e. ansible-test) do not show these deprecations to developers, only the end-user tools like ansible-playbook.

13

ansible-test is not the only thing they should be running, but i do agree, adding an ‘error on deprecations’ flag would be useful

14

@bcoca Good point. I’m testing against milestone and have seen the six deprecation in the sanity tests. And I’m working on this. Actually, I think I’ve fixed this, just didn’t do a new release yet. But I will before 2.20 is out.

However, I didn’t see any sanity failures for using ansible.module_utils._text. So testing with milestone didn’t help me there.

Maybe my sanity tests are somewhat buggy. Any ideas?

15

I want to clarify/confirm one point, tools like ansible-playbook are not used exclusively by collection developers. They’re used directly in day-to-day automation environments, often by people who are not writing or maintaining collections themselves.

That’s why the distinction between “developer-facing” and “end-user-visible” changes really matters here. If a deprecation warning appears during normal execution of ansible-playbook, it’s not just signaling to collection maintainers, it’s surfacing to the people actually running automation.

There is a strong call for these deprecation warnings to be postponed to a later version of ansible-core.

16

depercations start a 4 version cycle before removal, it gives both users, playbook authors and developers time to adjust to either get the dependencies updated or find alternatives. Sometimes it is up to the user as a project can be abandoned or just unresponsive.

17

@bcoca Nevertheless, I still don’t understand why I’ve seen sanity test failures about ansible.module_utils.six being deprecated here, but not about ansible.module_utils._text here / in [DNM] Test CI.

An think it should. Am I doing something wrong there in my sanity tests?

18

@mariolenz not sure, will look into it

1 Like
19

I’m new to here as a contributor, but this discussion has helped me understand how deprecations like this are handled across versions. it is interesting to see how maintainers balance compatibility between Python 2 and newer versions of ansible-core releases.

Just to confirm, if maintaining a small collection that only supports Python 3 and ansible-core 2.17+, its safe to remove all ansible.module_utils.six and use standard python 3 syntax or built-in functions instead, correct?

20

@Srchitiki yes

@mariolenz it seems six got a specific sanity test, which is something new and not something we’ve done across the board, but probably will going forward.
In any case the deprecation for six is being reverted, but will be added back in the future.

1 Like