Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

on_connect and on_disconnect type hints #30

Open
jhk-mjolner opened this issue Nov 27, 2023 · 1 comment
Open

on_connect and on_disconnect type hints #30

jhk-mjolner opened this issue Nov 27, 2023 · 1 comment

Comments

@jhk-mjolner
Copy link

I'm using fastapi_websocket_rpc's on_connect callbacks on WebsocketRPCEndpoint. And it works. But when I use pyright to statically check the types, I get errors.

I define the callbacks like async def on_client_connect(self, channel: RpcChannel): and give them as WebsocketRPCEndpoint(on_connect=[self.on_client_connect]).

Pyright gives errors like

error: Argument of type "list[(channel: RpcChannel) -> Coroutine[Any, Any, None]]" cannot be assigned to parameter "on_connect" of type "List[Coroutine[Unknown, Unknown, Unknown]]" in function "__init__"
    "function" is incompatible with "Coroutine[Unknown, Unknown, Unknown]" (reportGeneralTypeIssues)

Looking further into it. It seems like the type hinting in WebsocketRPCEndpoint constructor doesn't quite match what it will accept at runtime.

A place I've looked: https://stackoverflow.com/a/59177557 and its comments

If I've understood correctly, a better type hint would be List[Callable[[RpcChannel], Awaitable[Any]]] instead of List[Coroutine] to say that it takes a list of something callable that will get an RpcChannel parameter, and that when called, should return something awaitable that can return anything.
At least pyright is happy if I modify the library code to have that type hint.

Is this something that you're interested in changing?

For now I'll use # type: ignore comments to disable type checking when constructing WebsocketRPCEndpoint

@asafc
Copy link
Contributor

asafc commented Nov 30, 2023

Hi @jhk-mjolner thanks for reporting!

We would be willing to review a PR, if you'd be interested in contributing this fix.
cc @roekatz

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

No branches or pull requests

2 participants