Fix python 3.10 issue #583
Conversation
WalkthroughThis pull request refactors exception handling in the shared module and updates testing behavior. In the shared module, the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ExceptionList as _get_exception_lst
participant ExceptionHelper as _get_exception
Caller->>ExceptionList: Invoke _get_exception_lst(future_lst)
loop For each Future in future_lst
ExceptionList->>ExceptionHelper: Process Future object
ExceptionHelper-->>ExceptionList: Return exception (if any)
end
ExceptionList-->>Caller: Return list of exceptions
Possibly related PRs
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #583 +/- ##
==========================================
+ Coverage 95.89% 95.96% +0.07%
==========================================
Files 26 26
Lines 1193 1191 -2
==========================================
- Hits 1144 1143 -1
+ Misses 49 48 -1 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
executorlib/interactive/shared.py (1)
679-684: Consider adjusting the timeout value.The current timeout of 10^-10 seconds (0.1 nanoseconds) is extremely small and might be too aggressive. Consider using a slightly larger value like 10^-6 (microseconds) or 10^-3 (milliseconds) to allow for minor system delays while still maintaining non-blocking behavior.
- excp = future_obj.exception(timeout=10**-10) + excp = future_obj.exception(timeout=10**-3) # Use milliseconds timeout
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
executorlib/interactive/shared.py(2 hunks)tests/test_dependencies_executor.py(0 hunks)
💤 Files with no reviewable changes (1)
- tests/test_dependencies_executor.py
⏰ Context from checks skipped due to timeout of 90000ms (17)
- GitHub Check: unittest_mpich (ubuntu-latest, 3.10)
- GitHub Check: unittest_openmpi (ubuntu-latest, 3.10)
- GitHub Check: unittest_mpich (ubuntu-latest, 3.11)
- GitHub Check: unittest_openmpi (ubuntu-latest, 3.11)
- GitHub Check: unittest_mpich (ubuntu-latest, 3.12)
- GitHub Check: unittest_openmpi (ubuntu-latest, 3.12)
- GitHub Check: unittest_openmpi (ubuntu-latest, 3.13)
- GitHub Check: unittest_win
- GitHub Check: unittest_mpich (ubuntu-latest, 3.13)
- GitHub Check: unittest_openmpi (macos-latest, 3.13)
- GitHub Check: unittest_mpich (macos-latest, 3.13)
- GitHub Check: unittest_old
- GitHub Check: unittest_flux_mpich
- GitHub Check: unittest_flux_openmpi
- GitHub Check: notebooks
- GitHub Check: benchmark (ubuntu-latest, 3.13, .ci_support/environment-mpich.yml)
- GitHub Check: benchmark (ubuntu-latest, 3.13, .ci_support/environment-openmpi.yml)
🔇 Additional comments (3)
executorlib/interactive/shared.py (3)
7-7: LGTM! Added TimeoutError import for improved exception handling.The addition of
TimeoutErrorimport supports the new non-blocking exception handling implementation.
675-676: LGTM! Improved type safety and readability.The function signature now explicitly accepts a list of
Futureobjects, and the implementation is more concise using list comprehension.
679-684:✅ Verification successful
Verify exception handling behavior across Python versions.
The changes aim to fix Python 3.10 issues. Let's verify the behavior of
Future.exception()with timeout across different Python versions.
🌐 Web query:
How does concurrent.futures.Future.exception() with timeout parameter behave across Python 3.8, 3.9, and 3.10?💡 Result:
The
concurrent.futures.Future.exception()method with a timeout parameter behaves consistently across Python 3.8–3.10 based on documentation and implementation details:Core Behavior
When called with atimeoutargument (int/float), it:
- Blocks until either:
- The task raises an exception (returns it immediately)
- The task completes without error (returns
None)- The timeout expires (raises
TimeoutError)- Raises
TimeoutErrorif no result/exception is available within the specified duration[1][3][5].Version-Specific Notes
No documented behavioral changes exist between Python 3.8–3.10 for this method:
- Official docs for all versions specify identical semantics[5][7].
- Key implementation details like error types (
TimeoutErrorvs legacy aliases) stabilized before 3.8[5][7].- No version-specific bug fixes or behavioral adjustments appear in changelogs for these releases[2][4][8][10].
Example Usage (Works Identically in All Versions)
from concurrent.futures import ThreadPoolExecutor def task(): raise ValueError("Simulated error") with ThreadPoolExecutor() as executor: future = executor.submit(task) try: # Wait max 1 second for exception exc = future.exception(timeout=1) print(f"Exception: {exc}") except TimeoutError: print("Timeout occurred")Key Documentation References
- "If the call hasn’t completed in timeout seconds, then a TimeoutError will be raised"[5]
- "If timeout is specified... TimeoutError will be raised"[3]
This consistency stems from
concurrent.futuresbeing part of Python’s stable standard library API since its introduction in Python 3.2[5].Citations:
- 1: https://superfastpython.com/threadpoolexecutor-timeouts/
- 2: https://docs.python.org/3/whatsnew/3.11.html
- 3: https://myjavaadventures.com/blog/2022/06/09/concurrent-futures-with-python/
- 4: https://docs.python.org/3.10/whatsnew/changelog.html
- 5: https://docs.python.org/3/library/concurrent.futures.html
- 6: https://mrcfile.readthedocs.io/_/downloads/en/stable/pdf/
- 7: https://docs.python.org/3/library/asyncio-future.html
- 8: https://docs.python.org/3.10/whatsnew/changelog.html?highlight=borrowed+reference
- 9: python3 futures.as_completed timeout broken if future contains undefined reference python/cpython#85860
- 10: https://www.andy-pearce.com/blog/posts/2022/Nov/whats-new-in-python-39-library-changes/
Verified: Exception Handling Behavior is Consistent Across Versions
The investigation confirmed that
concurrent.futures.Future.exception()with a timeout behaves consistently across Python 3.8, 3.9, and 3.10. The current implementation inexecutorlib/interactive/shared.py(lines 679-684) properly checks for exceptions, correctly handling the case where aTimeoutErroris raised if the timeout expires. No changes are necessary.
- Confirmed consistent behavior per official documentation and implementation details.
- The usage of a minimal timeout (
10**-10) is acceptable as long as it meets the intended behavior.
Summary by CodeRabbit