New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary workaround for Python 2.6 OrderedDict #50391

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
5 participants
@jdufresne
Copy link
Contributor

jdufresne commented Dec 30, 2018

collections.OrderedDict has been available since Python 2.7. Support for Python 2.6 was removed from Ansible in commit ccf41bb. Remove all workarounds.

https://docs.python.org/3/library/collections.html#collections.OrderedDict

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Dec 30, 2018

@jdufresne: Greetings! Thanks for taking the time to open this pullrequest. In order for the community to handle your pullrequest effectively, we need a bit more information.

Here are the items we could not find in your description:

  • issue type

Please set the description of this pullrequest with this template:
https://raw.githubusercontent.com/ansible/ansible/devel/.github/PULL_REQUEST_TEMPLATE.md

click here for bot help

@ansibot

This comment has been minimized.

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Dec 30, 2018

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/plugins/callback/dense.py:483:33: undefined-variable Undefined variable 'HAS_OD'

The test ansible-test sanity --test import --python 2.6 [explain] failed with 3 errors:

lib/ansible/modules/cloud/amazon/cloudfront_distribution.py:1101:0: ImportError: cannot import name OrderedDict
lib/ansible/modules/network/f5/bigip_gtm_server.py:403:0: ImportError: cannot import name OrderedDict
lib/ansible/modules/network/f5/bigip_ucs.py:182:0: ImportError: cannot import name OrderedDict

click here for bot help

@jdufresne jdufresne force-pushed the jdufresne:ordereddict branch from 1460f1d to 257054f Dec 30, 2018

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Dec 30, 2018

The test ansible-test sanity --test import --python 2.6 [explain] failed with 3 errors:

lib/ansible/modules/cloud/amazon/cloudfront_distribution.py:1101:0: ImportError: cannot import name OrderedDict
lib/ansible/modules/network/f5/bigip_gtm_server.py:403:0: ImportError: cannot import name OrderedDict
lib/ansible/modules/network/f5/bigip_ucs.py:182:0: ImportError: cannot import name OrderedDict

click here for bot help

@abadger

This comment has been minimized.

Copy link
Member

abadger commented Dec 31, 2018

I'm afraid this is premature. (For two reasons)

  • Ansible has dropped support for python 2.6 on the controller. This pr addresses the modules as well. So I'm afraid it can't happen yet.
  • Our testing harness for continuous integration currently can't test controller side code separately from module side so we have to unofficially maintain controller side python 2.6 support for now. Separating those is planned for ansible 2.8

If you'd like to change this pr so it only applies to non-module code we can put this in the tracker of items blocked by the ci testing harness changes.

/Cc @mattclay

@ansibot ansibot removed the needs_triage label Dec 31, 2018

Remove unnecessary workaround for Python 2.6 OrderedDict
collections.OrderedDict has been available since Python 2.7. Support for
Python 2.6 was removed from Ansible in commit
ccf41bb. Remove all workarounds.

https://docs.python.org/3/library/collections.html#collections.OrderedDict

@jdufresne jdufresne force-pushed the jdufresne:ordereddict branch from 257054f to 5b5d54d Dec 31, 2018

@jdufresne

This comment has been minimized.

Copy link
Contributor

jdufresne commented Dec 31, 2018

Thanks for the informative response. That makes sense. I've reverted the changes to the modules directory.

@ansibot ansibot added the stale_ci label Jan 9, 2019

@mattclay

This comment has been minimized.

Copy link
Member

mattclay commented Jan 9, 2019

As @abadger pointed out, our test infrastructure hasn't been updated yet to drop Python 2.6 controller testing. These changes happen to pass in CI because our Python 2.6 test container includes OrderedDict -- but we shouldn't rely on that. Running the tests on a clean Python 2.6 install will fail.

This should be good to merge after I've finished updating our test infrastructure to drop Python 2.6 testing on the controller.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment