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

Avoid iteration error when getting module path #8533

Merged
merged 1 commit into from Feb 26, 2024

Conversation

jrbourbeau
Copy link
Member

While running Client.restart() as part of a Prefect workflow, I got the following error

12:47:35.902 | ERROR   | distributed.core - Exception while handling op restart
Traceback (most recent call last):
  File "/Users/james/projects/dask/distributed/distributed/utils.py", line 832, in wrapper
    return await func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/james/projects/dask/distributed/distributed/scheduler.py", line 6292, in restart
    raise TimeoutError(msg) from None
TimeoutError: Waited for 1 worker(s) to reconnect after restarting, but after 120s, only 0 have returned. Consider a longer timeout, or `wait_for_workers=False`.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/james/projects/dask/distributed/distributed/core.py", line 970, in _handle_comm
    result = await result
             ^^^^^^^^^^^^
  File "/Users/james/projects/dask/distributed/distributed/utils.py", line 831, in wrapper
    with self:
  File "/Users/james/projects/dask/distributed/distributed/utils.py", line 853, in __exit__
    modname = _getmodulename_with_path(frame.filename)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/james/projects/dask/distributed/distributed/utils.py", line 804, in _getmodulename_with_path
    for modname, mod in sys.modules.items():
RuntimeError: dictionary changed size during iteration

due to sys.modules changing.

This PR adds a .copy(), per the official Python docs https://docs.python.org/3/library/sys.html#sys.modules, to avoid this error.

Copy link
Member

@hendrikmakait hendrikmakait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @jrbourbeau! LGTM, assuming CI ends up happy.

Copy link
Contributor

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

    27 files  ±0      27 suites  ±0   9h 58m 59s ⏱️ - 5m 48s
 3 996 tests ±0   3 885 ✅ ±0    110 💤 ±0  1 ❌ ±0 
50 254 runs  ±0  47 957 ✅  - 1  2 296 💤 +1  1 ❌ ±0 

For more details on these failures, see this check.

Results for commit ffc3ff7. ± Comparison against base commit 390a5b5.

@jrbourbeau jrbourbeau merged commit cbf939c into dask:main Feb 26, 2024
33 of 35 checks passed
@jrbourbeau jrbourbeau deleted the _getmodulename_with_path-fixup branch February 26, 2024 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants