From 37012f7bf529735ee4454ccd8398841bcd16d318 Mon Sep 17 00:00:00 2001 From: Nate Prewitt Date: Wed, 27 Apr 2022 13:43:39 -0600 Subject: [PATCH] Fix flake8 issues --- .pre-commit-config.yaml | 18 ++++----- docs/dev/contributing.rst | 71 +++++++-------------------------- pyproject.toml | 13 +++++++ pytest.ini | 3 -- requests/adapters.py | 6 +-- requests/auth.py | 2 +- requests/cookies.py | 5 ++- requests/models.py | 11 +++--- requests/sessions.py | 9 ++++- requests/utils.py | 5 ++- setup.cfg | 9 +++-- tests/__init__.py | 1 - tests/conftest.py | 1 - tests/test_help.py | 4 -- tests/test_lowlevel.py | 82 ++++++++++++++++++++++++++------------- tests/test_requests.py | 41 +++++++++++--------- tests/test_testserver.py | 4 +- 17 files changed, 144 insertions(+), 141 deletions(-) create mode 100644 pyproject.toml delete mode 100644 pytest.ini diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7ee8c680c0..cac5ddccb4 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -15,16 +15,14 @@ repos: - repo: https://github.com/psf/black rev: 22.3.0 hooks: - - id: black - exclude: tests/test_lowlevel.py + - id: black + exclude: tests/test_lowlevel.py - repo: https://github.com/asottile/pyupgrade rev: v2.31.1 hooks: - - id: pyupgrade - args: [--py37-plus] -# TODO: Add flake8 changes after we're happy -# with above formatting changes. -#- repo: https://gitlab.com/pycqa/flake8 -# rev: 4.0.1 -# hooks: -# - id: flake8 + - id: pyupgrade + args: [--py37-plus] +- repo: https://gitlab.com/pycqa/flake8 + rev: 4.0.1 + hooks: + - id: flake8 diff --git a/docs/dev/contributing.rst b/docs/dev/contributing.rst index 63bfdfdbf2..961f7c3aba 100644 --- a/docs/dev/contributing.rst +++ b/docs/dev/contributing.rst @@ -93,6 +93,21 @@ event that you object to the code review feedback, you should make your case clearly and calmly. If, after doing so, the feedback is judged to still apply, you must either apply the feedback or withdraw your contribution. +Code Style +~~~~~~~~~~ + +Requests uses a collection of tools to ensure the code base has a consistent +style as it grows. We have these orchestrated using a tool called +`pre-commit`_. This can be installed locally and run over your changes prior +to opening a PR, and will also be run as part of the CI approval process +before a change is merged. + +You can find the full list of formatting requirements specified in the +`.pre-commit-config.yaml`_ at the top level directory of Requests. + +.. _pre-commit: https://pre-commit.com/ +.. _.pre-commit-config.yaml: https://github.com/psf/requests/blob/main/.pre-commit-config.yaml + New Contributors ~~~~~~~~~~~~~~~~ @@ -103,62 +118,6 @@ asking for help. Please also check the :ref:`early-feedback` section. -Kenneth Reitz's Code Style™ -~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -The Requests codebase uses the `PEP 8`_ code style. - -In addition to the standards outlined in PEP 8, we have a few guidelines: - -- Line-length can exceed 79 characters, to 100, when convenient. -- Line-length can exceed 100 characters, when doing otherwise would be *terribly* inconvenient. -- Always use single-quoted strings (e.g. ``'#flatearth'``), unless a single-quote occurs within the string. - -Additionally, one of the styles that PEP8 recommends for `line continuations`_ -completely lacks all sense of taste, and is not to be permitted within -the Requests codebase:: - - # Aligned with opening delimiter. - foo = long_function_name(var_one, var_two, - var_three, var_four) - -No. Just don't. Please. This is much better:: - - foo = long_function_name( - var_one, - var_two, - var_three, - var_four, - ) - -Docstrings are to follow the following syntaxes:: - - def the_earth_is_flat(): - """NASA divided up the seas into thirty-three degrees.""" - pass - -:: - - def fibonacci_spiral_tool(): - """With my feet upon the ground I lose myself / between the sounds - and open wide to suck it in. / I feel it move across my skin. / I'm - reaching up and reaching out. / I'm reaching for the random or - whatever will bewilder me. / Whatever will bewilder me. / And - following our will and wind we may just go where no one's been. / - We'll ride the spiral to the end and may just go where no one's - been. - - Spiral out. Keep going... - """ - pass - -All functions, methods, and classes are to contain docstrings. Object data -model methods (e.g. ``__repr__``) are typically the exception to this rule. - -Thanks for helping to make the world a better place! - -.. _PEP 8: https://pep8.org/ -.. _line continuations: https://www.python.org/dev/peps/pep-0008/#indentation Documentation Contributions --------------------------- diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000000..996bf14c83 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,13 @@ +[tool.isort] +profile = "black" +src_paths = ["requests", "test"] +honor_noqa = true + +[tool.pytest.ini_options] +addopts = "-p no:warnings --doctest-modules" +doctest_optionflags = "NORMALIZE_WHITESPACE ELLIPSIS" +minversion = "6.2" +testpaths = [ + "requests", + "tests", +] diff --git a/pytest.ini b/pytest.ini deleted file mode 100644 index 89f424b830..0000000000 --- a/pytest.ini +++ /dev/null @@ -1,3 +0,0 @@ -[pytest] -addopts = -p no:warnings --doctest-modules -doctest_optionflags= NORMALIZE_WHITESPACE ELLIPSIS diff --git a/requests/adapters.py b/requests/adapters.py index 0e0163fad7..d3b2d5bb1e 100644 --- a/requests/adapters.py +++ b/requests/adapters.py @@ -7,7 +7,7 @@ """ import os.path -import socket +import socket # noqa: F401 from urllib3.exceptions import ClosedPoolError, ConnectTimeoutError from urllib3.exceptions import HTTPError as _HTTPError @@ -537,9 +537,9 @@ def send( preload_content=False, decode_content=False, ) - except: + except Exception: # If we hit any problems here, clean up the connection. - # Then, reraise so that we can handle the actual exception. + # Then, raise so that we can handle the actual exception. low_conn.close() raise diff --git a/requests/auth.py b/requests/auth.py index 85aee33407..9733686ddb 100644 --- a/requests/auth.py +++ b/requests/auth.py @@ -173,7 +173,7 @@ def sha512_utf8(x): hash_utf8 = sha512_utf8 - KD = lambda s, d: hash_utf8(f"{s}:{d}") + KD = lambda s, d: hash_utf8(f"{s}:{d}") # noqa:E731 if hash_utf8 is None: return None diff --git a/requests/cookies.py b/requests/cookies.py index cbfc3522f0..bf54ab237e 100644 --- a/requests/cookies.py +++ b/requests/cookies.py @@ -476,8 +476,9 @@ def create_cookie(name, value, **kwargs): badargs = set(kwargs) - set(result) if badargs: - err = "create_cookie() got unexpected keyword arguments: %s" - raise TypeError(err % list(badargs)) + raise TypeError( + f"create_cookie() got unexpected keyword arguments: {list(badargs)}" + ) result.update(kwargs) result["port_specified"] = bool(result["port"]) diff --git a/requests/models.py b/requests/models.py index 6b6a828ec5..7e1522837f 100644 --- a/requests/models.py +++ b/requests/models.py @@ -10,7 +10,7 @@ # Import encoding now, to avoid implicit import later. # Implicit import within threads may cause LookupError when standard library is in a ZIP, # such as in Embedded Python. See https://github.com/psf/requests/issues/3578. -import encodings.idna +import encodings.idna # noqa: F401 from io import UnsupportedOperation from urllib3.exceptions import ( @@ -965,6 +965,8 @@ def json(self, **kwargs): # and the server didn't bother to tell us what codec *was* # used. pass + except JSONDecodeError as e: + raise RequestsJSONDecodeError(e.msg, e.doc, e.pos) try: return complexjson.loads(self.text, **kwargs) @@ -979,17 +981,16 @@ def links(self): header = self.headers.get("link") - # l = MultiDict() - l = {} + resolved_links = {} if header: links = parse_header_links(header) for link in links: key = link.get("rel") or link.get("url") - l[key] = link + resolved_links[key] = link - return l + return resolved_links def raise_for_status(self): """Raises :class:`HTTPError`, if one occurred.""" diff --git a/requests/sessions.py b/requests/sessions.py index 68a89ffc2c..6cb3b4dae3 100644 --- a/requests/sessions.py +++ b/requests/sessions.py @@ -30,10 +30,15 @@ from .hooks import default_hooks, dispatch_hook # formerly defined here, reexposed here for backward compatibility -from .models import DEFAULT_REDIRECT_LIMIT, REDIRECT_STATI, PreparedRequest, Request +from .models import ( # noqa: F401 + DEFAULT_REDIRECT_LIMIT, + REDIRECT_STATI, + PreparedRequest, + Request, +) from .status_codes import codes from .structures import CaseInsensitiveDict -from .utils import ( +from .utils import ( # noqa: F401 DEFAULT_PORTS, default_headers, get_auth_from_url, diff --git a/requests/utils.py b/requests/utils.py index 0fa884637c..7a881b6444 100644 --- a/requests/utils.py +++ b/requests/utils.py @@ -25,7 +25,7 @@ from .__version__ import __version__ # to_native_string is unused here, but imported here for backwards compatibility -from ._internal_utils import to_native_string +from ._internal_utils import to_native_string # noqa: F401 from .compat import ( Mapping, basestring, @@ -764,7 +764,8 @@ def should_bypass_proxies(url, no_proxy): """ # Prioritize lowercase environment variables over uppercase # to keep a consistent behaviour with other http projects (curl, wget). - get_proxy = lambda k: os.environ.get(k) or os.environ.get(k.upper()) + def get_proxy(key): + return os.environ.get(key) or os.environ.get(key.upper()) # First check whether no_proxy is defined. If it is, check that the URL # we're getting isn't in the no_proxy list. diff --git a/setup.cfg b/setup.cfg index a82b28fb9a..5ff5d4274e 100644 --- a/setup.cfg +++ b/setup.cfg @@ -9,6 +9,9 @@ requires-dist = idna>=2.5,<4 urllib3>=1.21.1,<1.27 -[isort] -profile = black -honor_noqa = true +[flake8] +ignore = E203, E501, W503 +per-file-ignores = + requests/__init__.py:E402, F401 + requests/compat.py:E402, F401 + tests/compat.py:F401 diff --git a/tests/__init__.py b/tests/__init__.py index d54e79b327..04385be18a 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -2,7 +2,6 @@ import warnings -import urllib3 from urllib3.exceptions import SNIMissingWarning # urllib3 sets SNIMissingWarning to only go off once, diff --git a/tests/conftest.py b/tests/conftest.py index ba8e6b2b2a..530a4c2a5f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,7 +5,6 @@ from SimpleHTTPServer import SimpleHTTPRequestHandler import ssl -import tempfile import threading import pytest diff --git a/tests/test_help.py b/tests/test_help.py index ea098b284d..fb4e967c53 100644 --- a/tests/test_help.py +++ b/tests/test_help.py @@ -1,7 +1,3 @@ -import sys - -import pytest - from requests.help import info diff --git a/tests/test_lowlevel.py b/tests/test_lowlevel.py index a889024ca5..859d07e8a5 100644 --- a/tests/test_lowlevel.py +++ b/tests/test_lowlevel.py @@ -1,9 +1,10 @@ import threading import pytest +from tests.testserver.server import Server, consume_socket_content import requests -from tests.testserver.server import Server, consume_socket_content +from requests.compat import JSONDecodeError from .utils import override_environ @@ -12,10 +13,11 @@ def echo_response_handler(sock): """Simple handler that will take request and echo it back to requester.""" request_content = consume_socket_content(sock, timeout=0.5) - text_200 = (b"HTTP/1.1 200 OK\r\n" b"Content-Length: %d\r\n\r\n" b"%s") % ( - len(request_content), - request_content, - ) + text_200 = ( + b"HTTP/1.1 200 OK\r\n" + b"Content-Length: %d\r\n\r\n" + b"%s" + ) % (len(request_content), request_content) sock.send(text_200) @@ -41,7 +43,10 @@ def incomplete_chunked_response_handler(sock): request_content = consume_socket_content(sock, timeout=0.5) # The server never ends the request and doesn't provide any valid chunks - sock.send(b"HTTP/1.1 200 OK\r\n" + b"Transfer-Encoding: chunked\r\n") + sock.send( + b"HTTP/1.1 200 OK\r\n" + b"Transfer-Encoding: chunked\r\n" + ) return request_content @@ -51,7 +56,7 @@ def incomplete_chunked_response_handler(sock): with server as (host, port): url = f"http://{host}:{port}/" with pytest.raises(requests.exceptions.ChunkedEncodingError): - r = requests.get(url) + requests.get(url) close_server.set() # release server block @@ -98,12 +103,14 @@ def test_conflicting_content_lengths(): def multiple_content_length_response_handler(sock): request_content = consume_socket_content(sock, timeout=0.5) - - sock.send(b"HTTP/1.1 200 OK\r\n" + - b"Content-Type: text/plain\r\n" + - b"Content-Length: 16\r\n" + - b"Content-Length: 32\r\n\r\n" + - b"-- Bad Actor -- Original Content\r\n") + response = ( + b"HTTP/1.1 200 OK\r\n" + b"Content-Type: text/plain\r\n" + b"Content-Length: 16\r\n" + b"Content-Length: 32\r\n\r\n" + b"-- Bad Actor -- Original Content\r\n" + ) + sock.send(response) return request_content @@ -113,7 +120,7 @@ def multiple_content_length_response_handler(sock): with server as (host, port): url = f"http://{host}:{port}/" with pytest.raises(requests.exceptions.InvalidHeader): - r = requests.get(url) + requests.get(url) close_server.set() @@ -307,10 +314,12 @@ def redirect_resp_handler(sock): consume_socket_content(sock, timeout=0.5) location = f'//{host}:{port}/{path}' sock.send( - b'HTTP/1.1 301 Moved Permanently\r\n' - b'Content-Length: 0\r\n' - b'Location: ' + location.encode('utf8') + b'\r\n' - b'\r\n' + ( + b'HTTP/1.1 301 Moved Permanently\r\n' + b'Content-Length: 0\r\n' + b'Location: %s\r\n' + b'\r\n' + ) % location.encode('utf8') ) redirect_request.append(consume_socket_content(sock, timeout=0.5)) sock.send(b'HTTP/1.1 200 OK\r\n\r\n') @@ -329,18 +338,11 @@ def redirect_resp_handler(sock): close_server.set() + def test_fragment_not_sent_with_request(): """Verify that the fragment portion of a URI isn't sent to the server.""" - def response_handler(sock): - req = consume_socket_content(sock, timeout=0.5) - sock.send( - b'HTTP/1.1 200 OK\r\n' - b'Content-Length: '+bytes(len(req))+b'\r\n' - b'\r\n'+req - ) - close_server = threading.Event() - server = Server(response_handler, wait_to_close_event=close_server) + server = Server(echo_response_handler, wait_to_close_event=close_server) with server as (host, port): url = f'http://{host}:{port}/path/to/thing/#view=edit&token=hunter2' @@ -358,6 +360,7 @@ def response_handler(sock): close_server.set() + def test_fragment_update_on_redirect(): """Verify we only append previous fragment if one doesn't exist on new location. If a new fragment is encountered in a Location header, it should @@ -388,7 +391,6 @@ def response_handler(sock): with server as (host, port): url = f'http://{host}:{port}/path/to/thing/#view=edit&token=hunter2' r = requests.get(url) - raw_request = r.content assert r.status_code == 200 assert len(r.history) == 2 @@ -400,3 +402,27 @@ def response_handler(sock): assert r.url == f'http://{host}:{port}/final-url/#relevant-section' close_server.set() + + +def test_json_decode_compatibility_for_alt_utf_encodings(): + + def response_handler(sock): + consume_socket_content(sock, timeout=0.5) + sock.send( + b'HTTP/1.1 200 OK\r\n' + b'Content-Length: 18\r\n\r\n' + b'\xff\xfe{\x00"\x00K0"\x00=\x00"\x00\xab0"\x00\r\n' + ) + + close_server = threading.Event() + server = Server(response_handler, wait_to_close_event=close_server) + + with server as (host, port): + url = f'http://{host}:{port}/' + r = requests.get(url) + r.encoding = None + with pytest.raises(requests.exceptions.JSONDecodeError) as excinfo: + r.json() + assert isinstance(excinfo.value, requests.exceptions.RequestException) + assert isinstance(excinfo.value, JSONDecodeError) + assert r.text not in str(excinfo.value) diff --git a/tests/test_requests.py b/tests/test_requests.py index 3d9d6ebb88..ccd8636bad 100644 --- a/tests/test_requests.py +++ b/tests/test_requests.py @@ -32,7 +32,6 @@ ConnectTimeout, ContentDecodingError, InvalidHeader, - InvalidJSONError, InvalidProxyURL, InvalidSchema, InvalidURL, @@ -89,7 +88,7 @@ def test_entry_points(self): requests.patch requests.post # Not really an entry point, but people rely on it. - from requests.packages.urllib3.poolmanager import PoolManager + from requests.packages.urllib3.poolmanager import PoolManager # noqa:F401 @pytest.mark.parametrize( "exception, url", @@ -934,9 +933,8 @@ def test_invalid_ssl_certificate_files(self, httpbin_secure): with pytest.raises(IOError) as e: requests.get(httpbin_secure(), cert=(".", INVALID_PATH)) - assert ( - str(e.value) - == f"Could not find the TLS key file, invalid path: {INVALID_PATH}" + assert str(e.value) == ( + f"Could not find the TLS key file, invalid path: {INVALID_PATH}" ) @pytest.mark.parametrize( @@ -1112,7 +1110,9 @@ def hook(resp, **kwargs): s.send(prep) def test_session_hooks_are_used_with_no_request_hooks(self, httpbin): - hook = lambda x, *args, **kwargs: x + def hook(*args, **kwargs): + pass + s = requests.Session() s.hooks["response"].append(hook) r = requests.Request("GET", httpbin()) @@ -1121,8 +1121,12 @@ def test_session_hooks_are_used_with_no_request_hooks(self, httpbin): assert prep.hooks["response"] == [hook] def test_session_hooks_are_overridden_by_request_hooks(self, httpbin): - hook1 = lambda x, *args, **kwargs: x - hook2 = lambda x, *args, **kwargs: x + def hook1(*args, **kwargs): + pass + + def hook2(*args, **kwargs): + pass + assert hook1 is not hook2 s = requests.Session() s.hooks["response"].append(hook2) @@ -1691,15 +1695,15 @@ def test_header_value_not_str(self, httpbin): # Test for int with pytest.raises(InvalidHeader) as excinfo: - r = requests.get(httpbin("get"), headers=headers_int) + requests.get(httpbin("get"), headers=headers_int) assert "foo" in str(excinfo.value) # Test for dict with pytest.raises(InvalidHeader) as excinfo: - r = requests.get(httpbin("get"), headers=headers_dict) + requests.get(httpbin("get"), headers=headers_dict) assert "bar" in str(excinfo.value) # Test for list with pytest.raises(InvalidHeader) as excinfo: - r = requests.get(httpbin("get"), headers=headers_list) + requests.get(httpbin("get"), headers=headers_list) assert "baz" in str(excinfo.value) def test_header_no_return_chars(self, httpbin): @@ -1712,13 +1716,13 @@ def test_header_no_return_chars(self, httpbin): # Test for newline with pytest.raises(InvalidHeader): - r = requests.get(httpbin("get"), headers=headers_ret) + requests.get(httpbin("get"), headers=headers_ret) # Test for line feed with pytest.raises(InvalidHeader): - r = requests.get(httpbin("get"), headers=headers_lf) + requests.get(httpbin("get"), headers=headers_lf) # Test for carriage return with pytest.raises(InvalidHeader): - r = requests.get(httpbin("get"), headers=headers_cr) + requests.get(httpbin("get"), headers=headers_cr) def test_header_no_leading_space(self, httpbin): """Ensure headers containing leading whitespace raise @@ -1729,10 +1733,11 @@ def test_header_no_leading_space(self, httpbin): # Test for whitespace with pytest.raises(InvalidHeader): - r = requests.get(httpbin("get"), headers=headers_space) + requests.get(httpbin("get"), headers=headers_space) + # Test for tab with pytest.raises(InvalidHeader): - r = requests.get(httpbin("get"), headers=headers_tab) + requests.get(httpbin("get"), headers=headers_tab) @pytest.mark.parametrize("files", ("foo", b"foo", bytearray(b"foo"))) def test_can_send_objects_with_files(self, httpbin, files): @@ -2655,7 +2660,7 @@ def test_preparing_bad_url(self, url): @pytest.mark.parametrize("url, exception", (("http://localhost:-1", InvalidURL),)) def test_redirecting_to_bad_url(self, httpbin, url, exception): with pytest.raises(exception): - r = requests.get(httpbin("redirect-to"), params={"url": url}) + requests.get(httpbin("redirect-to"), params={"url": url}) @pytest.mark.parametrize( "input, expected", @@ -2730,7 +2735,7 @@ def test_parameters_for_nonstandard_schemes(self, input, params, expected): def test_post_json_nan(self, httpbin): data = {"foo": float("nan")} with pytest.raises(requests.exceptions.InvalidJSONError): - r = requests.post(httpbin("post"), json=data) + requests.post(httpbin("post"), json=data) def test_json_decode_compatibility(self, httpbin): r = requests.get(httpbin("bytes/20")) diff --git a/tests/test_testserver.py b/tests/test_testserver.py index c637cfce22..c73a3f1f59 100644 --- a/tests/test_testserver.py +++ b/tests/test_testserver.py @@ -3,9 +3,9 @@ import time import pytest +from tests.testserver.server import Server import requests -from tests.testserver.server import Server class TestTestServer: @@ -42,7 +42,7 @@ def test_server_closes(self): def test_text_response(self): """the text_response_server sends the given text""" server = Server.text_response_server( - "HTTP/1.1 200 OK\r\n" + "Content-Length: 6\r\n" + "\r\nroflol" + "HTTP/1.1 200 OK\r\n" "Content-Length: 6\r\n" "\r\nroflol" ) with server as (host, port):