Skip to content

Drop the pre-3.9 thread pool shutdown path - #3754

Open
Herklos wants to merge 1 commit into
devfrom
cursor/drop-pre-39-executor-shutdown-51f7
Open

Herklos wants to merge 1 commit into
devfrom
cursor/drop-pre-39-executor-shutdown-51f7

Conversation

@Herklos

@Herklos Herklos commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Thank you for sending your pull request.
But first, have you included unit tests, and is your code PEP8 conformant? More details

Summary

Always wait for thread pool shutdown now that Python 3.13 is required.

Changelog

  • Drop the Python < 3.9 private-thread clear in stop_thread_pool_executor_non_gracefully.
  • Close websocket connector pools with shutdown(wait=True) only.
  • Replace the uncollected sleep-forever helper test with one pytest collects.

What's new?

stop_thread_pool_executor_non_gracefully still had a branch that cleared private ThreadPoolExecutor thread tables. That branch ran only before Python 3.9. On 3.13 it never ran: both the helper and websocket close_sockets already called shutdown(wait=True).

The helper keeps its name. The docstring now says it waits. Websocket close no longer checks sys.version_info.

packages/commons/tests/thread_util.py was not collected by pytest. It submitted workers that sleep forever and then called the helper, which would hang on 3.13. It is now tests/test_thread_util.py and checks that a short task finishes and a later submit raises RuntimeError.

packages/commons pytest: tests/test_thread_util.py passed (1 test). Extended linter against origin/dev: 0 violations.

Open in Web Open in Cursor 

OctoBot requires Python 3.13, so the private-thread clear branch never ran. Websocket close now always waits for the pool, and the helper test no longer sleeps forever.

Co-authored-by: herklos.dev <herklos.dev@protonmail.com>
@Herklos
Herklos requested a review from GuillaumeDSM as a code owner October 13, 2026 15:00
@Herklos Herklos self-assigned this Oct 13, 2026

This branch has not been deployed

No deployments
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.

2 participants