permitio / fastapi_websocket_rpc

⚡ FASTAPI Websocket RPC- A fast and durable bidirectional JSON RPC channel over Websockets.
https://permit.io
MIT License
205 stars 23 forks source link

Purpose of WebsocketRPCEndpoint.on_connect #39

Closed 5p4k closed 1 month ago

5p4k commented 1 month ago

I was trying out the package and I was going to inherit from WebsocketRPCEndpoint and override on_connect. https://github.com/permitio/fastapi_websocket_rpc/blob/d0f322e362a50f9bb955e5a0ec66c2670c3c5f77/fastapi_websocket_rpc/websocket_rpc_endpoint.py#L110 However in my test example it's never called, and in fact, it seems like it's never referred to at any point; the handlers in the private member _on_connect are passed directly to RpcChannel: https://github.com/permitio/fastapi_websocket_rpc/blob/d0f322e362a50f9bb955e5a0ec66c2670c3c5f77/fastapi_websocket_rpc/websocket_rpc_endpoint.py#L85

So wonder what is the purpose of on_connect? Is that supposed to be called externally by e.g. FastAPI? Was it supposed to be passed to register_connect_handler?

orweis commented 1 month ago

on_connect is exposed for deriving layers that extended or build on top of the RPC channel - see https://github.com/permitio/fastapi_websocket_pubsub for example (https://github.com/permitio/fastapi_websocket_pubsub/blob/c757bd707622252aab761a59115897dd12413478/fastapi_websocket_pubsub/pub_sub_client.py#L194)

5p4k commented 1 month ago

The purpose of the on_connect argument of __init__ (which the example you linked makes use of) is clear.

I was wondering about the on_connect method of WebsocketRPCEndpoint. It creates a task that executes self._on_connect handlers, but it's never called and it duplicates the functionality already offered by RpcChannel.on_handler_event. It looks like it's not called by anything.

orweis commented 1 month ago

Yes, you seem to be right. I guess it's seem artifact missed in a refactor - this part is 4 years old.

orweis commented 1 month ago

Would you like to open a PR to fix this? :)