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

websocket_handshake invalid function signature #2272

Closed
cnicodeme opened this issue Oct 15, 2021 · 7 comments
Closed

websocket_handshake invalid function signature #2272

cnicodeme opened this issue Oct 15, 2021 · 7 comments
Labels

Comments

@cnicodeme
Copy link
Contributor

Describe the bug
The function websocket_handshake located in sanic.server.protocols.websocket_protocol has an invalid signature in Sanic 21.9.2 (at least)

Code snippet
Current code is:

    async def websocket_handshake(
        self, request, subprotocols=Optional[Sequence[str]]
    ):

Expected behavior
Code should be:

    async def websocket_handshake(
        self, request, subprotocols:Optional[Sequence[str]] = None
    ):

=> Typing should be set with ":", not "=" ;)

Environment (please complete the following information):

  • Version 21.9.2
@ahopkins
Copy link
Member

@cnicodeme Can you add a PR for this, and I can get a patch released this weekend.

@cnicodeme
Copy link
Contributor Author

Let me know if that works :)

@ahopkins
Copy link
Member

Awesome thanks. I'll take a look at why it's failing tomorrow 😎

I appreciate you calling this out and fixing it.

@cnicodeme
Copy link
Contributor Author

You are welcome! That was an easy one to fix ;)

(But I don't know why it refuses my PR, maybe some wrong code formatting)

@ahopkins
Copy link
Member

ahopkins commented Oct 15, 2021

That could be. I'm on my phone so it's hard to see. Maybe line length? We run black on the code base so that might help.

If you run make pretty it should hopefully autofix.

@ahopkins
Copy link
Member

Nope. Looks like mypy

tox -e type-checking

@ashleysommer Maybe related to what you said you were seeing?

ahopkins added a commit that referenced this issue Oct 27, 2021
* Replacing assignation by typing for `websocket_handshake`

Related to #2272

* Fix some type hinting issues

* Cleanup websocket handchake response concat

* Optimize concat encoding

Co-authored-by: Adam Hopkins <[email protected]>
@stale
Copy link

stale bot commented Mar 2, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is incorrect, please respond with an update. Thank you for your contributions.

@stale stale bot added the stale label Mar 2, 2022
@ahopkins ahopkins closed this as completed Mar 2, 2022
ChihweiLHBird pushed a commit to ChihweiLHBird/sanic that referenced this issue Jun 1, 2022
…2273)

* Replacing assignation by typing for `websocket_handshake`

Related to sanic-org#2272

* Fix some type hinting issues

* Cleanup websocket handchake response concat

* Optimize concat encoding

Co-authored-by: Adam Hopkins <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants