Redesign: switch to HotReloadMiddleware

[florimondmanca]

This PR builds upon #25 and introduces a HotReloadMiddleware.

The main change is that most things are now fully transparent. The middleware responds to lifespan to do its startup/shutdown on its own, and it responds to websocket requests on its own as well. No need to register any event handlers or routes externally anymore.

As the middleware basically does all the work now, arel.HotReload becomes obsolete.

A revamp of the README ensues this redesign...

TODO:

• Manually test the quickstart example on Hypercorn and Daphne. Let's ensure their reload functionality does the same as Uvicorn, i.e. reboot the server process which triggers a disconnect...
• Make sure Uvicorn, Hypercorn, Daphne etc don't reload on non-Python files. Otherwise, maybe we could just drop our custom directory file watching? People could do uvicorn --reload --reload-dir server/templates --reload-dir custom_directory_etc and arel would just handle "reload on reconnect" (if the above is correct).
• Expand tests beyond the example app. Ideally we'd setup ad-hoc Starlette apps and test against them. Unfortunately HTTPX doesn't do WebSocket yet. So I guess we'll have to spin up actual servers.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 24.21053% with 144 lines in your changes missing coverage. Please review.
✅ Project coverage is 32.47%. Comparing base (3790178) to head (a34b6f2).
⚠️ Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
src/arel/_middleware.py	26.66%	88 Missing ⚠️
tests/test_middleware.py	12.50%	49 Missing ⚠️
src/arel/_watch.py	0.00%	4 Missing ⚠️
tests/test_deprecations.py	60.00%	2 Missing ⚠️
src/arel/_app.py	66.66%	1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff              @@
##            master      #31       +/-   ##
============================================
- Coverage   100.00%   32.47%   -67.53%     
============================================
  Files           10        9        -1     
  Lines          196      271       +75     
============================================
- Hits           196       88      -108     
- Misses           0      183      +183     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[florimondmanca]

django-browser-reload uses Server-sent events rather than WebSocket. It's true we don't need WebSocket, as we only do unidirectional server->client messaging (the browser never needs to send anything to the server). But WebSocket seems to have an even better browser support than EventSource. So let's stick to WS.

[florimondmanca]

In case this might be useful…

https://github.com/frankie567/httpx-ws

[Kludex]

How would that potentially be useful here?

[florimondmanca]

For one of the TODO items here…

Expand tests beyond the example app. Ideally we'd setup ad-hoc Starlette apps and test against them. Unfortunately HTTPX doesn't do WebSocket yet. So I guess we'll have to spin up actual servers.

[Kludex]

Manually test the quickstart example on Hypercorn and Daphne. Let's ensure their reload functionality does the same as Uvicorn, i.e. reboot the server process which triggers a disconnect...

Daphne doesn't have a reload option, and for some reason it doesn't work for Hypercorn. I still didn't find why... 👀

[zboyles]

This middleware PR allowed me to get client side auto reloading working with Gradio's FastAPI mount_gradio_app function. I'm connected to the project through ssh in vscode which auto forwards the local port incrementing it by one. I manually changed the client.js to reflect that by const ws = new WebSocket("ws://127.0.0.1:8001/arel"); which got it connected. If I wanted to clean it up and make it available, how do you recommend I add arel? Right now I'm just using the loose files from this branch. Thanks.

[Kludex]

@florimondmanca How can I help to get this merged?

[florimondmanca]

@Kludex Hey, thanks, help is appreciated. We’d need to review the TODO items I left in the PR description. This should be some manual testing only, from what I recall. Then I would love a proper review of the PR, checking if things look correct, tested, documented, and functional, of course. Also, CI looks like it didn’t pass last time it ran…

[florimondmanca]

@Kludex Hey, thanks, help is appreciated. We’d need to review the TODO items I left in the PR description. This should be some manual testing only, from what I recall. Then I would love a proper review of the PR, checking if things look correct, tested, documented, and functional, of course. Also, CI looks like it didn’t pass last time it ran…

[Kludex]

Perfect. I'll check it. Thanks 🙏

[oyvinev]

Content length is here updated with the script, but script is not attached, creating a RuntimeError in Starlette (RuntimeError: Response content shorter than content length).

This check should be moved to the conditional above: if message["type"] == "http.response.start": ..., e.g. like this on L185:

if b"</body>" not in message["body"]:
      await send(message)
      return

[y2kbugger]

Make sure Uvicorn, Hypercorn, Daphne etc don't reload on non-Python files. Otherwise, maybe we could just drop our custom directory file watching? People could do uvicorn --reload --reload-dir server/templates --reload-dir custom_directory_etc and arel would just handle "reload on reconnect" (if the above is correct).

Without Arel, on Uvicorn I had to manually include css and html (jinja). With Arel I still want server restart for jinja templates but not on css.

uvicorn insync.app:app --reload --reload-dir insync --reload-include='*.css' --reload-include='*.html'

vs

uvicorn insync.app:app --reload --reload-dir insync --reload-include='*.html'

Trying to think through how this interplays with #37. How do you intend to "Make sure" that devs correctly don't accidently config thier dev servers to unnecessarily restart their server on css changes, or even should you? It might be obvious when reloading takes longer than expect for say, a css file. sorry I see that you just mean manual testing. the following still applies.

Given that #37 takes reloading static assets down to milliseconds, I think you definitely don't want to just rely on letting "arel [...] just handle "reload on reconnect"

Anecdotally, about a month ago, my team was really confused about the Arel reload message on the server side, thinking it was coming from uvicorn --reload. At least including [Arel] at the beginning of that message would have made it clearer what was going on.