Skip to content
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

Don't change dictionary keys during iteration #56806

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
4 participants
@hroncok
Copy link

commented May 22, 2019

SUMMARY

With Python 3.8.0a4+, we get the following RuntimeError in Fedora:

PYTHONPATH=../../lib ../bin/dump_keywords.py --template-dir=../templates --output-dir=rst/reference_appendices/ -d ./keyword_desc.yml
Traceback (most recent call last):
  File "../bin/dump_keywords.py", line 49, in <module>
    for a in oblist[name]:
RuntimeError: dictionary keys changed during iteration

Python change: python/cpython#12596

Downstream bug: https://bugzilla.redhat.com/show_bug.cgi?id=1712531

ISSUE TYPE
  • Bugfix Pull Request
  • Docs Pull Request
COMPONENT NAME

docs?

@hroncok hroncok marked this pull request as ready for review May 22, 2019

@hroncok hroncok changed the title Don't change dictionary keys during iteration WIP: Don't change dictionary keys during iteration May 22, 2019

@hroncok

This comment has been minimized.

Copy link
Author

commented May 22, 2019

Apparently, not enough:

=================================== FAILURES ===================================
__________ TestDellos9Facts.test_dellos9_facts_gather_subset_default ___________
[gw7] linux -- Python 3.8.0 /usr/bin/python3
self = <units.modules.network.dellos9.test_dellos9_facts.TestDellos9Facts testMethod=test_dellos9_facts_gather_subset_default>

    def test_dellos9_facts_gather_subset_default(self):
        set_module_args(dict())
>       result = self.execute_module()

test/units/modules/network/dellos9/test_dellos9_facts.py:69: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
test/units/modules/network/dellos9/dellos9_module.py:62: in execute_module
    result = self.changed(changed)
test/units/modules/network/dellos9/dellos9_module.py:83: in changed
    self.module.main()
lib/ansible/modules/network/dellos9/dellos9_facts.py:559: in main
    inst.populate()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

    def populate(self):
        super(Interfaces, self).populate()
        self.facts['all_ipv4_addresses'] = list()
        self.facts['all_ipv6_addresses'] = list()
    
        data = self.responses[0]
        interfaces = self.parse_interfaces(data)
    
>       for key in interfaces.keys():
E       RuntimeError: dictionary keys changed during iteration

lib/ansible/modules/network/dellos9/dellos9_facts.py:255: RuntimeError
_________ TestDellos9Facts.test_dellos9_facts_gather_subset_interfaces _________
[gw7] linux -- Python 3.8.0 /usr/bin/python3
self = <units.modules.network.dellos9.test_dellos9_facts.TestDellos9Facts testMethod=test_dellos9_facts_gather_subset_interfaces>

    def test_dellos9_facts_gather_subset_interfaces(self):
        set_module_args({'gather_subset': 'interfaces'})
>       result = self.execute_module()

test/units/modules/network/dellos9/test_dellos9_facts.py:100: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
test/units/modules/network/dellos9/dellos9_module.py:62: in execute_module
    result = self.changed(changed)
test/units/modules/network/dellos9/dellos9_module.py:83: in changed
    self.module.main()
lib/ansible/modules/network/dellos9/dellos9_facts.py:559: in main
    inst.populate()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

    def populate(self):
        super(Interfaces, self).populate()
        self.facts['all_ipv4_addresses'] = list()
        self.facts['all_ipv6_addresses'] = list()
    
        data = self.responses[0]
        interfaces = self.parse_interfaces(data)
    
>       for key in interfaces.keys():
E       RuntimeError: dictionary keys changed during iteration

lib/ansible/modules/network/dellos9/dellos9_facts.py:255: RuntimeError
=============================== warnings summary ===============================
Don't change dictionary keys during iteration
With Python 3.8.0a4+, we get the following RuntimeError in Fedora:

PYTHONPATH=../../lib ../bin/dump_keywords.py --template-dir=../templates --output-dir=rst/reference_appendices/ -d ./keyword_desc.yml
Traceback (most recent call last):
  File "../bin/dump_keywords.py", line 49, in <module>
    for a in oblist[name]:
RuntimeError: dictionary keys changed during iteration

And:

    def populate(self):
        super(Interfaces, self).populate()
        self.facts['all_ipv4_addresses'] = list()
        self.facts['all_ipv6_addresses'] = list()

        data = self.responses[0]
        interfaces = self.parse_interfaces(data)

>       for key in interfaces.keys():
E       RuntimeError: dictionary keys changed during iteration

In TestDellos9Facts.test_dellos9_facts_gather_subset_default
and TestDellos9Facts.test_dellos9_facts_gather_subset_interfaces.

Python change: python/cpython#12596

Downstream bug: https://bugzilla.redhat.com/show_bug.cgi?id=1712531

@hroncok hroncok force-pushed the hroncok:patch-1 branch from daa8b95 to 702e044 May 22, 2019

@hroncok hroncok changed the title WIP: Don't change dictionary keys during iteration Don't change dictionary keys during iteration May 22, 2019

@hroncok

This comment has been minimized.

Copy link
Author

commented May 22, 2019

amended

@abadger

This comment has been minimized.

Copy link
Member

commented May 22, 2019

The general strategy here looks correct once you have it passing all the tests. We probably would want to backport this to 2.8.x as well so if you could add a changelog fragment that would be great. http://docs.testing.ansible.com/ansible/latest/community/development_process.html#backporting-merged-prs

@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 22, 2019

@hroncok

This comment has been minimized.

Copy link
Author

commented May 22, 2019

@abadger this is getting more complicated than I anticipated. Could you please take it from here? I don't consider this changelog worthy. 3.8 is still not released, so claiming 3.8 support is premature. Also, it should change no behavior what so ever.

@abadger

This comment has been minimized.

Copy link
Member

commented May 22, 2019

I'm afraid I don't work on ansible core anymore so I can't take over. I just saw the bugzilla entry which lead me here.

@sivel

This comment has been minimized.

Copy link
Member

commented May 22, 2019

I'll try to take a look as soon as I have some free time.

We are running our unit tests against 3.8, but these files don't have units, and don't necessarily need them, so I'm unsure why it's not failing in our CI.

Update 2

CI is using 3.8.0a3 right now.

@sivel sivel self-assigned this May 23, 2019

@ansibot ansibot added the stale_ci label May 31, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.