Skip to content

Fix hot reloading for Python 3.9#182

Closed
MtkN1 wants to merge 6 commits into
sphinx-doc:mainfrom
MtkN1:fix-reload-3.9
Closed

Fix hot reloading for Python 3.9#182
MtkN1 wants to merge 6 commits into
sphinx-doc:mainfrom
MtkN1:fix-reload-3.9

Conversation

@MtkN1

@MtkN1 MtkN1 commented Oct 29, 2024

Copy link
Copy Markdown

Fixes #178

1. Move asyncio.Event creation to lifespan

This PR moves the creation of the asyncio.Event from RebuildServer.__init__ to RebuildServer.lifespan. This ensures asyncio.Event is created within the event loop context, resolving the relevant error.

Note

The error does not occur in Python>=3.10 because the implementation was changed by python/cpython#86558.

2. Add rebuild test

Since there was no rebuild test, I added one.

  • Added a rebuild test.
    • Added a test to check refresh messages from /websocket-reload after modifying the index.rst file.
  • Changed the target test function to an asynchronous function.
    • Starlette's TestClient uses threads, while RebuildServer uses multiprocessing. In my view, this affected the rebuild test execution and cause it to fail. (0662fc9)
    • Changed Starlette's TestClient to http.AsyncClient. (ref)
    • Added test dependencies asgi-lifespan and httpx-ws required for testing lifespan and WebSocket.

@MtkN1 MtkN1 left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

After four months, the CI is finally green! ✅

@z-aki

z-aki commented Mar 20, 2025

Copy link
Copy Markdown

@AA-Turner @hugovk requesting a review.

@hugovk

hugovk commented Mar 20, 2025

Copy link
Copy Markdown
Contributor

I'm not a maintainer here, but Python 3.9 is only receiving security updates and is EOL in October:

https://devguide.python.org/versions/

And Sphinx itself supports 3.11+:

https://www.sphinx-doc.org/en/master/internals/release-process.html#python-version-support-policy

Can you upgrade to a newer Python?

@MtkN1

MtkN1 commented Mar 20, 2025

Copy link
Copy Markdown
Author

Thank you for your comment.

sphinx-autobuild itself requires Python >= 3.9, and many library projects have this requirement. Therefore, it still has value for at least half a year. I am not particularly attached to Python 3.9, but I believe the changes in the implementation are reasonable. Additionally, I have added tests for hot reload, which may also be valuable for this project.

I would appreciate it if a maintainer could review the changes.

@hugovk

hugovk commented Mar 20, 2025

Copy link
Copy Markdown
Contributor

Thank you for your comment.

sphinx-autobuild itself requires Python >= 3.9, and many library projects have this requirement. Therefore, it still has value for at least half a year. I am not particularly attached to Python 3.9, but I believe the changes in the implementation are reasonable.

Because Sphinx is an application for generating docs, and not a library dependency, projects can build docs with a single (recent) Python version, even if they support the full 3.9-3.13 range.

I've opened #189 as an alternative to match the same versions as Sphinx itself, in case the maintainers wish to keep parity.

Additionally, I have added tests for hot reload, which may also be valuable for this project.

Yes, these sound useful.

@AA-Turner AA-Turner closed this May 28, 2025
Comment thread tests/test_application.py

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Happy to consider a PR with just these tests

@MtkN1 MtkN1 May 28, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'll do that since it's a good opportunity :)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I opened this #191

@MtkN1 MtkN1 mentioned this pull request May 28, 2025
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.

2024.09.03 - Autobuild does not hot reload when changes happen

4 participants