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.

Sign up for GitHub

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

fix websocket ping variables issues #1909

Merged
merged 4 commits into from
Sep 28, 2020
Merged

fix websocket ping variables issues #1909

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
33aa4da
fixed the problem that the websocket ping_timeout and ping_interval p…
Aug 13, 2020
13094e0
Revert check for websocket protocol to use hasattr
ahopkins Sep 28, 2020
3483e7b
Fix linting issues
ahopkins Sep 28, 2020
65a7060
Merge branch 'master' into master
ahopkins Sep 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions sanic/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from signal import SIG_IGN, SIGINT, SIGTERM, Signals
from signal import signal as signal_func
from time import time
from typing import Type
from typing import Dict, Type, Union

from httptools import HttpRequestParser # type: ignore
from httptools.parser.errors import HttpParserError # type: ignore
Expand Down Expand Up @@ -955,15 +955,15 @@ def serve(

def _build_protocol_kwargs(
protocol: Type[HttpProtocol], config: Config
) -> dict:
if hasattr(protocol, "websocket_timeout"):
) -> Dict[str, Union[int, float]]:
if hasattr(protocol, "websocket_handshake"):
return {
"max_size": config.WEBSOCKET_MAX_SIZE,
"max_queue": config.WEBSOCKET_MAX_QUEUE,
"read_limit": config.WEBSOCKET_READ_LIMIT,
"write_limit": config.WEBSOCKET_WRITE_LIMIT,
"ping_timeout": config.WEBSOCKET_PING_TIMEOUT,
"ping_interval": config.WEBSOCKET_PING_INTERVAL,
"websocket_max_size": config.WEBSOCKET_MAX_SIZE,
"websocket_max_queue": config.WEBSOCKET_MAX_QUEUE,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these being renamed websocket_?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left this change in, but I updated the tests so that the change was carried through consistently.

"websocket_read_limit": config.WEBSOCKET_READ_LIMIT,
"websocket_write_limit": config.WEBSOCKET_WRITE_LIMIT,
"websocket_ping_timeout": config.WEBSOCKET_PING_TIMEOUT,
"websocket_ping_interval": config.WEBSOCKET_PING_INTERVAL,
}
return {}

Expand Down
20 changes: 14 additions & 6 deletions tests/test_app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,20 @@ async def handler(request, ws):

websocket_protocol_call_args = websocket_protocol_mock.call_args
ws_kwargs = websocket_protocol_call_args[1]
assert ws_kwargs["max_size"] == app.config.WEBSOCKET_MAX_SIZE
assert ws_kwargs["max_queue"] == app.config.WEBSOCKET_MAX_QUEUE
assert ws_kwargs["read_limit"] == app.config.WEBSOCKET_READ_LIMIT
assert ws_kwargs["write_limit"] == app.config.WEBSOCKET_WRITE_LIMIT
assert ws_kwargs["ping_timeout"] == app.config.WEBSOCKET_PING_TIMEOUT
assert ws_kwargs["ping_interval"] == app.config.WEBSOCKET_PING_INTERVAL
assert ws_kwargs["websocket_max_size"] == app.config.WEBSOCKET_MAX_SIZE
assert ws_kwargs["websocket_max_queue"] == app.config.WEBSOCKET_MAX_QUEUE
assert ws_kwargs["websocket_read_limit"] == app.config.WEBSOCKET_READ_LIMIT
assert (
ws_kwargs["websocket_write_limit"] == app.config.WEBSOCKET_WRITE_LIMIT
)
assert (
ws_kwargs["websocket_ping_timeout"]
== app.config.WEBSOCKET_PING_TIMEOUT
)
assert (
ws_kwargs["websocket_ping_interval"]
== app.config.WEBSOCKET_PING_INTERVAL
)


def test_handle_request_with_nested_exception(app, monkeypatch):
Expand Down