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

Trivial code cleanup #448

Merged
merged 2 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ requires = ["setuptools >= 41"]
build-backend = "setuptools.build_meta"

[tool.black]
target-version = ['py35', 'py36', 'py37', 'py38']
target-version = ['py39', 'py310', 'py311', 'py312', 'py313']
exclude = '''
/(
\.git
Expand Down
2 changes: 1 addition & 1 deletion src/waitress/channel.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ def write_soon(self, data):
raise ClientDisconnected
num_bytes = len(data)

if data.__class__ is ReadOnlyFileBasedBuffer:
if isinstance(data, ReadOnlyFileBasedBuffer):
Copy link
Member

Choose a reason for hiding this comment

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

There was a reason this was a data.__class__ check instead of isinstance, and something broke in the past when I tried to update this. I just can't remember what exactly.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wasn't able to find anything in the history. That line goes back well over a decade, and Chris didn't leave any explanation of why it uses .__class__ is ... rather than isinstance(...). I can revert the code to use the former and instead leave a "there be dragons" comment here and in task.py, if that works?

Copy link
Member

Choose a reason for hiding this comment

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

It was probably me avoiding a funccall for performance reasons while I was doing profiling.

Copy link
Member

Choose a reason for hiding this comment

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

I'm a big fan of "if tests pass..." so let's let it ride for now.

I will re-review this tomorrow when I am not sleep deprived, and hope that I can remember what the issue was.

# they used wsgi.file_wrapper
self.outbufs.append(data)
nextbuf = OverflowableBuffer(self.adj.outbuf_overflow)
Expand Down
6 changes: 3 additions & 3 deletions src/waitress/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def create_server(
_start=True, # test shim
_sock=None, # test shim
_dispatcher=None, # test shim
**kw # adjustments
**kw, # adjustments
):
"""
if __name__ == '__main__':
Expand Down Expand Up @@ -193,7 +193,7 @@ def __init__(
adj=None, # adjustments
sockinfo=None, # opaque object
bind_socket=True,
**kw
**kw,
):
if adj is None:
adj = Adjustments(**kw)
Expand Down Expand Up @@ -387,7 +387,7 @@ def __init__(
dispatcher=None, # dispatcher
adj=None, # adjustments
sockinfo=None, # opaque object
**kw
**kw,
):
if sockinfo is None:
sockinfo = (socket.AF_UNIX, socket.SOCK_STREAM, None, None)
Expand Down
9 changes: 4 additions & 5 deletions src/waitress/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
##############################################################################

from collections import deque
import socket
import sys
import threading
import time
Expand Down Expand Up @@ -389,7 +388,7 @@ def start_response(status, headers, exc_info=None):

self.complete = True

if status.__class__ is not str:
if not isinstance(status, str):
raise AssertionError("status %s is not a string" % status)
if "\n" in status or "\r" in status:
raise ValueError(
Expand All @@ -400,11 +399,11 @@ def start_response(status, headers, exc_info=None):

# Prepare the headers for output
for k, v in headers:
if k.__class__ is not str:
if not isinstance(k, str):
raise AssertionError(
f"Header name {k!r} is not a string in {(k, v)!r}"
)
if v.__class__ is not str:
if not isinstance(v, str):
raise AssertionError(
f"Header value {v!r} is not a string in {(k, v)!r}"
)
Expand Down Expand Up @@ -437,7 +436,7 @@ def start_response(status, headers, exc_info=None):

can_close_app_iter = True
try:
if app_iter.__class__ is ReadOnlyFileBasedBuffer:
if isinstance(app_iter, ReadOnlyFileBasedBuffer):
cl = self.content_length
size = app_iter.prepare(cl)
if size:
Expand Down
7 changes: 1 addition & 6 deletions src/waitress/wasyncore.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,14 @@

from errno import (
EAGAIN,
EALREADY,
EBADF,
ECONNABORTED,
ECONNRESET,
EINPROGRESS,
EINTR,
EINVAL,
EISCONN,
ENOTCONN,
EPIPE,
ESHUTDOWN,
EWOULDBLOCK,
errorcode,
)
import logging
import os
Expand All @@ -75,7 +70,7 @@
import time
import warnings

from . import compat, utilities
from . import utilities

_DISCONNECTED = frozenset({ECONNRESET, ENOTCONN, ESHUTDOWN, ECONNABORTED, EPIPE, EBADF})

Expand Down
1 change: 0 additions & 1 deletion tests/test_adjustments.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import socket
import sys
import unittest
import warnings

Expand Down
2 changes: 0 additions & 2 deletions tests/test_channel.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import io
import unittest

import pytest


class TestHTTPChannel(unittest.TestCase):
def _makeOne(self, sock, addr, adj, map=None):
Expand Down
14 changes: 5 additions & 9 deletions tests/test_receiver.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,6 @@ def test_received_chunk_not_properly_terminated(self):
self.assertEqual(inst.error.__class__, BadRequest)

def test_received_multiple_chunks(self):
from waitress.utilities import BadRequest

buf = DummyBuffer()
inst = self._makeOne(buf)
data = (
Expand All @@ -196,13 +194,11 @@ def test_received_multiple_chunks(self):
)
result = inst.received(data)
self.assertEqual(result, len(data))
self.assertEqual(inst.completed, True)
self.assertTrue(inst.completed)
self.assertEqual(b"".join(buf.data), b"Wikipedia in\r\n\r\nchunks.")
self.assertEqual(inst.error, None)
self.assertIsNone(inst.error)

def test_received_multiple_chunks_split(self):
from waitress.utilities import BadRequest

buf = DummyBuffer()
inst = self._makeOne(buf)
data1 = b"4\r\nWiki\r"
Expand Down Expand Up @@ -245,7 +241,7 @@ def test_received_invalid_extensions(self, invalid_extension):
data = b"4;" + invalid_extension + b"\r\ntest\r\n"
result = inst.received(data)
assert result == len(data)
assert inst.error.__class__ == BadRequest
assert isinstance(inst.error, BadRequest)
assert inst.error.body == "Invalid chunk extension"

@pytest.mark.parametrize(
Expand All @@ -260,7 +256,7 @@ def test_received_valid_extensions(self, valid_extension):
data = b"4;" + valid_extension + b"\r\ntest\r\n"
result = inst.received(data)
assert result == len(data)
assert inst.error == None
assert inst.error is None

@pytest.mark.parametrize(
"invalid_size", [b"0x04", b"+0x04", b"x04", b"+04", b" 04", b" 0x04"]
Expand All @@ -273,7 +269,7 @@ def test_received_invalid_size(self, invalid_size):
data = invalid_size + b"\r\ntest\r\n"
result = inst.received(data)
assert result == len(data)
assert inst.error.__class__ == BadRequest
assert isinstance(inst.error, BadRequest)
assert inst.error.body == "Invalid chunk size"


Expand Down
1 change: 0 additions & 1 deletion tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,6 @@ def test_create_with_unix_socket(self):
from waitress.server import (
BaseWSGIServer,
MultiSocketServer,
TcpWSGIServer,
UnixWSGIServer,
)

Expand Down
1 change: 0 additions & 1 deletion tests/test_wasyncore.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
from errno import EALREADY, EINPROGRESS, EINVAL, EISCONN, EWOULDBLOCK, errorcode
import functools
import gc
from io import BytesIO
import os
import re
import select
Expand Down