feat!: Make route handlers functional decorators #3436

provinzkraut · 2024-04-27T08:52:52Z

Make route handler decorators functional decorators.

This gets rids of the __call__ method having to mutate internal state and lets us clean up things a bit.

It also makes for a nicer syntax and improved type/runtime checking of the handler classes, because they can now receive a fn as a required keyword argument, which gets rid of the HandlerClass("/")(handler_fn) pattern.

codecov · 2024-04-27T09:07:13Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.00%. Comparing base (a4d7ea1) to head (293956c).

Additional details and impacted files

@@           Coverage Diff           @@
##             v3.0    #3436   +/-   ##
=======================================
  Coverage   97.99%   98.00%           
=======================================
  Files         326      326           
  Lines       14537    14531    -6     
  Branches     2302     2294    -8     
=======================================
- Hits        14246    14241    -5     
  Misses        154      154           
+ Partials      137      136    -1

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

sonarcloud · 2024-04-28T11:25:17Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

litestar/handlers/asgi_handlers.py

JacobCoffee · 2024-04-29T19:23:54Z

litestar/handlers/asgi_handlers.py

@@ -101,4 +104,28 @@ async def handle(self, connection: ASGIConnection[ASGIRouteHandler, Any, Any, An
 await self.fn(scope=connection.scope, receive=connection.receive, send=connection.send)


-asgi = ASGIRouteHandler
+def asgi(


is this public/documented?

Yes, this is the replacement for v2's asgi type. Needs a docstring?

and a .. versionadded:: directive :D

Actually, I'm not so sure about the versionadded with these. The names aren't newly introduced, and their usage remains largely the same. The fact that they're now implemented as decorator functions instead of callable class instance is an implementation detail. I think it's fair to say this is merely a change and not addition of a new feature.

wdyt @JacobCoffee @peterschutt?

litestar/handlers/base.py

litestar/handlers/websocket_handlers/route_handler.py

peterschutt

Love it 😍

I do have a question about how we are handling layer resolution in the websocket listener _prepare_fn() method. But I'm pretty sure that this, and the refactoring suggestions I've made, are for things that you've just moved around in the PR, not things you've changed.

Overall though, feels way more pythonic, and I like the simplified wording you've used in the docs in places.

docs/release-notes/whats-new-3.rst

litestar/handlers/asgi_handlers.py

peterschutt · 2024-04-30T00:36:32Z

litestar/handlers/asgi_handlers.py

@@ -101,4 +104,28 @@ async def handle(self, connection: ASGIConnection[ASGIRouteHandler, Any, Any, An
 await self.fn(scope=connection.scope, receive=connection.receive, send=connection.send)


-asgi = ASGIRouteHandler
+def asgi(


Yes, this is the replacement for v2's asgi type. Needs a docstring?

litestar/handlers/http_handlers/base.py

peterschutt · 2024-04-30T01:14:14Z

litestar/handlers/http_handlers/base.py

@@ -285,7 +302,7 @@ def __init__(
 self.response_cookies: Sequence[Cookie] | None = narrow_response_cookies(response_cookies)
 self.response_headers: Sequence[ResponseHeader] | None = narrow_response_headers(response_headers)

- self.sync_to_thread = sync_to_thread
+ self.has_sync_callable = has_sync_callable


We can remove the class-level annotation for this on line 133 now that we assign it here.

peterschutt · 2024-04-30T01:29:38Z

litestar/handlers/websocket_handlers/listener.py

- listener=self, fn=fn, parsed_signature=parsed_signature, namespace=self.resolve_signature_namespace()
- )
+ return ListenerHandler(
+ listener=self, fn=fn, parsed_signature=parsed_signature, namespace=self.resolve_signature_namespace()


Its been a while since I've had my head in this part of the code, so I could very well be forgetting something.

However, when this was first rolled out, I recall we had a context object that allowed us to delay resolution of things like the signature namespace until the handler was actually registered on the application. FWICT, this is called as part of BaseRouteHandler.__init__() flow, which will be called when a function is decorated, and at that point it doesn't have any reference to its owner. This means that calling any layer resolution methods is redundant, right?

I can see that this is just transferring logic from the __call__() method in the previous implementation, but how does the layer resolution work here?

For reference, this is what he had in place at that time:

litestar/litestar/handlers/websocket_handlers/listener.py

Lines 152 to 153 in fb0e98a

def on_registration(self, app: Litestar) -> None:

self._set_listener_context()

I can see that this is just transferring logic from the call() method in the previous implementation, but how does the layer resolution work here?

I think it doesn't.. I'll have to check this later, but this might be a bug (one that would also exist in the current implementation).

one that would also exist in the current implementation

Yep, absolutely not introduced here, I just happen to be reading this code now.

peterschutt · 2024-04-30T01:35:43Z

litestar/handlers/websocket_handlers/route_handler.py

+ signature_namespace: Mapping[str, Any] | None = None,
+ websocket_class: type[WebSocket] | None = None,
+ **kwargs: Any,
+) -> Callable[[AsyncAnyCallable], WebsocketRouteHandler]:


This need a docstring?

Co-authored-by: Jacob Coffee <[email protected]>

Co-authored-by: Peter Schutt <[email protected]> Co-authored-by: Jacob Coffee <[email protected]>

github-actions bot added area/handlers This PR involves changes to the handlers area/private-api This PR involves changes to the privatized API area/testing area/types This PR involves changes to the custom types size: large type/feat area/channels pr/internal area/controller labels Apr 27, 2024

github-actions bot added the area/docs This PR involves changes to the documentation label Apr 27, 2024

provinzkraut marked this pull request as ready for review April 27, 2024 14:25

provinzkraut requested review from a team as code owners April 27, 2024 14:25

provinzkraut force-pushed the route-handler-decorators branch from 98bf1ab to 48ab468 Compare April 27, 2024 14:35

provinzkraut requested a review from JacobCoffee as a code owner April 27, 2024 14:35

provinzkraut force-pushed the v3.0 branch from 2536957 to 81081dd Compare April 27, 2024 14:44

provinzkraut mentioned this pull request Apr 27, 2024

Deprecate subclassing route handler decorators #3439

Open

JacobCoffee approved these changes Apr 29, 2024

View reviewed changes

peterschutt approved these changes Apr 30, 2024

View reviewed changes

provinzkraut and others added 2 commits May 19, 2024 16:07

Update litestar/handlers/asgi_handlers.py

f49a241

Co-authored-by: Jacob Coffee <[email protected]>

Apply suggestions from code review

7453d3a

Co-authored-by: Peter Schutt <[email protected]> Co-authored-by: Jacob Coffee <[email protected]>

provinzkraut changed the title ~~feat: Make route handlers functional decorators~~ feat!: Make route handlers functional decorators May 19, 2024

provinzkraut added 2 commits May 19, 2024 16:15

add a docstring

74011b7

Allow handler class customisation

293956c

github-actions bot added the Breaking 🔨 label May 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: Make route handlers functional decorators #3436

feat!: Make route handlers functional decorators #3436

provinzkraut commented Apr 27, 2024 •

edited

codecov bot commented Apr 27, 2024 •

edited

sonarcloud bot commented Apr 28, 2024

JacobCoffee Apr 29, 2024

peterschutt Apr 30, 2024

JacobCoffee Apr 30, 2024

provinzkraut May 19, 2024

peterschutt left a comment

peterschutt Apr 30, 2024

peterschutt Apr 30, 2024

peterschutt Apr 30, 2024

peterschutt Apr 30, 2024

provinzkraut Apr 30, 2024 •

edited

peterschutt Apr 30, 2024

peterschutt Apr 30, 2024

	def on_registration(self, app: Litestar) -> None:
	self._set_listener_context()

feat!: Make route handlers functional decorators #3436

Are you sure you want to change the base?

feat!: Make route handlers functional decorators #3436

Conversation

provinzkraut commented Apr 27, 2024 • edited

codecov bot commented Apr 27, 2024 • edited

Codecov Report

sonarcloud bot commented Apr 28, 2024

Quality Gate passed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterschutt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

provinzkraut Apr 30, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

provinzkraut commented Apr 27, 2024 •

edited

codecov bot commented Apr 27, 2024 •

edited

provinzkraut Apr 30, 2024 •

edited