From 9368207f025bb02f29c116bf8ba99d9a184f6e8e Mon Sep 17 00:00:00 2001 From: Lukas Vojt Date: Wed, 14 Aug 2024 15:10:52 +0200 Subject: [PATCH] chore: upgrade static analysis --- .github/workflows/push.yml | 9 ++---- .pre-commit-config.yaml | 6 ++-- .pylintrc | 3 -- request_session/_compat.py | 1 + request_session/exceptions.py | 1 + request_session/protocols.py | 1 + request_session/request_session.py | 45 ++++++++++++------------------ request_session/utils.py | 10 +++---- setup.py | 4 +-- test/conftest.py | 1 + test/test_request_session.py | 25 ++++++++--------- test/test_utils.py | 1 + 12 files changed, 46 insertions(+), 61 deletions(-) diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index ca8b1bc..2c9d70c 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -70,18 +70,13 @@ jobs: steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Set up Python 3.11 - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: python-version: 3.11 - - name: Cache - uses: actions/cache@v2 - with: - path: .pre-commit-cache - key: static-checks-${{ hashFiles('.pre-commit-config.yaml') }} # git checkout is not creating a working git folder, it has dubious ownership if not configured # The pre-commit hook will not work on this original git folder # https://github.com/pre-commit/pre-commit/issues/2125 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ac5ba81..e6a2fb3 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -4,7 +4,7 @@ default_language_version: exclude: "^.github.*" repos: - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v3.4.0 + rev: v4.6.0 hooks: - id: trailing-whitespace exclude: ^.*\.md$ @@ -35,7 +35,7 @@ repos: language_version: system - repo: https://github.com/pycqa/isort - rev: 5.7.0 + rev: 5.13.2 hooks: - id: isort additional_dependencies: [".[pyproject]"] @@ -46,7 +46,7 @@ repos: - id: black - repo: https://github.com/PyCQA/pylint - rev: pylint-2.6.0 + rev: v3.2.6 hooks: - id: pylint exclude: ^(docs/).*$ diff --git a/.pylintrc b/.pylintrc index 8bde44c..9fb7c69 100644 --- a/.pylintrc +++ b/.pylintrc @@ -11,16 +11,13 @@ disable = no-member, unused-argument, broad-except, - relative-import, wrong-import-position, bare-except, locally-disabled, protected-access, abstract-method, - no-self-use, fixme, too-few-public-methods, - bad-continuation, useless-object-inheritance, too-many-arguments, too-many-locals, diff --git a/request_session/_compat.py b/request_session/_compat.py index 0f58073..3fa0b08 100644 --- a/request_session/_compat.py +++ b/request_session/_compat.py @@ -1,4 +1,5 @@ """Module ensuring lib compatibilty.""" + from __future__ import absolute_import try: diff --git a/request_session/exceptions.py b/request_session/exceptions.py index a50ee05..8fed82c 100644 --- a/request_session/exceptions.py +++ b/request_session/exceptions.py @@ -1,4 +1,5 @@ """Module contains exceptions from requests and request_session.""" + from typing import Any # pylint: disable=unused-import from requests.exceptions import ConnectionError # pylint: disable=redefined-builtin diff --git a/request_session/protocols.py b/request_session/protocols.py index e378772..9f1b2ef 100644 --- a/request_session/protocols.py +++ b/request_session/protocols.py @@ -1,4 +1,5 @@ """Simple protocols to duck type dependency injections.""" + from typing import Any, List, Optional diff --git a/request_session/request_session.py b/request_session/request_session.py index d870404..691d9a3 100644 --- a/request_session/request_session.py +++ b/request_session/request_session.py @@ -1,4 +1,5 @@ """Main RequestSession module.""" + import re import time from collections import namedtuple @@ -113,7 +114,9 @@ def __init__( self.logger = logger self.log_prefix = log_prefix self.allowed_log_levels = allowed_log_levels - self.retriable_client_errors = retriable_client_errors if retriable_client_errors else [408] + self.retriable_client_errors = ( + retriable_client_errors if retriable_client_errors else [408] + ) self.prepare_new_session() @@ -149,17 +152,7 @@ def set_user_agent(self): # type: () -> None """Set proper user-agent string to header according to RFC22.""" pattern = r"^(?P\S.+?)\/(?P\S.+?) \((?P\S.+?) (?P\S.+?)\)(?: ?(?P.*))$" - string = ( - "{service_name}/{version} ({organization} {environment}) {sys_info}".format( - service_name=self.user_agent_components.service_name, # type: ignore - version=self.user_agent_components.version, # type: ignore - organization=self.user_agent_components.organization, # type: ignore - environment=self.user_agent_components.environment, # type: ignore - sys_info=self.user_agent_components.sys_info # type: ignore - if self.user_agent_components.sys_info # type: ignore - else "", - ).strip() - ) + string = f"{self.user_agent_components.service_name}/{self.user_agent_components.version} ({self.user_agent_components.organization} {self.user_agent_components.environment}) {self.user_agent_components.sys_info if self.user_agent_components.sys_info else ''}".strip() if not re.match(pattern, string): raise InvalidUserAgentString("Provided User-Agent string is not valid.") self.user_agent = string @@ -273,7 +266,7 @@ def _process( sleep_before_repeat=None, # type: Optional[float] tags=None, # type: Optional[list] raise_for_status=None, # type: Optional[bool] - **request_kwargs # type: Any + **request_kwargs, # type: Any ): # pylint: disable=too-many-statements # type: (...) -> Optional[requests.Response] r"""Run a request against a service depending on a request type. @@ -366,9 +359,11 @@ def _process( attempt=run, ) - if self.is_server_error(error, status_code) or self.retry_on_client_errors(status_code): + if self.is_server_error( + error, status_code + ) or self.retry_on_client_errors(status_code): if is_econnreset_error: - self.log("info", "{}.session_replace".format(request_category)) + self.log("info", f"{request_category}.session_replace") self.remove_session() self.prepare_new_session() @@ -444,9 +439,7 @@ def _send_request(self, request_type, request_params, tags, run, request_categor :param str request_category: Category for log and metric reporting. :return requests.Response: HTTP Response Object. """ - metric_name = "{request_category}.response_time".format( - request_category=request_category - ) + metric_name = f"{request_category}.response_time" if not self.statsd: return self.session.request(method=request_type, **request_params) @@ -491,7 +484,7 @@ def _log_with_params( status_code=response.status_code, attempt=attempt, url=url, - **extra_params + **extra_params, ) def sleep(self, seconds, request_category, tags): @@ -518,12 +511,10 @@ def metric_increment(self, metric, request_category, tags, attempt=None): """ new_tags = list(tags) if tags else [] if attempt: - new_tags.append("attempt:{attempt}".format(attempt=attempt)) + new_tags.append(f"attempt:{attempt}") if self.statsd is not None: - metric_name = "{metric_base}.{metric_type}".format( - metric_base=request_category, metric_type=metric - ) + metric_name = f"{request_category}.{metric}" self.statsd.increment(metric_name, tags=new_tags) def log(self, level, event, **kwargs): @@ -537,7 +528,7 @@ def log(self, level, event, **kwargs): """ if not level in self.allowed_log_levels: raise AttributeError("Provided log level is not allowed.") - event_name = "{prefix}.{event}".format(prefix=self.log_prefix, event=event) + event_name = f"{self.log_prefix}.{event}" if self.logger is not None: getattr(self.logger, level)(event_name, **kwargs) @@ -586,17 +577,17 @@ def _exception_log_and_metrics( self.log( "exception", - "{}.failed".format(request_category), + f"{request_category}.failed", error_type=error_type, status_code=status_code, attempt=attempt, - **extra_params + **extra_params, ) self.metric_increment( metric="request", request_category=request_category, - tags=tags + ["attempt:{}".format(attempt)], + tags=tags + [f"attempt:{attempt}"], ) @staticmethod diff --git a/request_session/utils.py b/request_session/utils.py index f8e90e9..a65ca9d 100644 --- a/request_session/utils.py +++ b/request_session/utils.py @@ -1,8 +1,8 @@ """Utilites used in RequestSession.""" -import logging + import sys import time -from typing import Any, Dict, Iterator, List, Optional, Text +from typing import Any, Dict, List, Optional from .protocols import Ddtrace @@ -36,11 +36,9 @@ def split_tags_and_update(dictionary, tags): def dict_to_string(dictionary): - # type: (Dict[str, Any]) -> Text + # type: (Dict[str, Any]) -> str """Convert dictionary to key=value pairs separated by a space.""" - return " ".join( - ["{}={}".format(key, value) for key, value in sorted(dictionary.items())] - ) + return " ".join([f"{key}={value}" for key, value in sorted(dictionary.items())]) def traced_sleep(trace_name, seconds, ddtrace, tags=None): diff --git a/setup.py b/setup.py index d647ab1..0806d1f 100644 --- a/setup.py +++ b/setup.py @@ -5,10 +5,10 @@ with open("README.md", encoding="utf-8") as f: readme = f.read() -with open("requirements.in") as f: +with open("requirements.in", encoding="utf-8") as f: install_requires = [line for line in f if line and line[0] not in "#-"] -with open("test-requirements.in") as f: +with open("test-requirements.in", encoding="utf-8") as f: tests_require = [line for line in f if line and line[0] not in "#-"] setup( diff --git a/test/conftest.py b/test/conftest.py index 6466e69..57a49df 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -1,4 +1,5 @@ """Just a conftest.""" + from typing import Any, Callable import httpbin as Httpbin diff --git a/test/test_request_session.py b/test/test_request_session.py index 4557560..742d4b3 100644 --- a/test/test_request_session.py +++ b/test/test_request_session.py @@ -1,4 +1,5 @@ """Test the main module.""" + import itertools import sys from typing import Any, Callable, Dict, Iterator, List, Union @@ -29,7 +30,7 @@ def test_init(mocker, httpbin): # type: (Mock, Httpbin) -> None """Test initialization of RequestSession.""" mock_ddtrace = mocker.Mock(spec_set=Ddtrace) - mock_tracing_config = dict() # type: Dict[Any, Any] + mock_tracing_config: dict = {} mock_ddtrace.config.get_from.return_value = mock_tracing_config session = RequestSession( @@ -179,13 +180,13 @@ def test_raise_for_status(mocker, httpbin, status_code, raises): mock_sys.exc_info.return_value = (HTTPError, HTTPError(), "fake_traceback") if raises: with pytest.raises(raises): - session.get(path="/status/{status_code}".format(status_code=status_code)) + session.get(path=f"/status/{status_code}") if isinstance(raises, HTTPError): assert mock_sys.exc_info()[1].__sentry_source == "third_party" assert mock_sys.exc_info()[1].__sentry_pd_alert == "disabled" else: - session.get(path="/status/{status_code}".format(status_code=status_code)) + session.get(path=f"/status/{status_code}") @pytest.mark.parametrize( @@ -239,9 +240,7 @@ def _prepare_new_session(self): # type: ignore client.get("/status/500") actual_call_count = sum(session.request.call_count for session in used_sessions) - mock_log.assert_called_with( - "info", "{category}.session_replace".format(category=client.request_category) - ) + mock_log.assert_called_with("info", f"{client.request_category}.session_replace") assert mock_exception_log_and_metrics.call_count == 1 assert actual_call_count == expected_call_count @@ -305,7 +304,7 @@ def test_logging(mocker, request_session, inputs, expected): ) client = request_session(max_retries=inputs["max_retries"]) client._exception_log_and_metrics = mock_exception_log_and_metrics - expected["request_params"]["url"] = "{}{}".format(client.host, inputs["path"]) + expected["request_params"]["url"] = f"{client.host}{inputs['path']}" calls = [] for attempt in range(1, expected["call_count"] + 1): @@ -360,11 +359,11 @@ def test_metric_increment( calls = [] for attempt in range(1, call_count + 1): - metric = "{}.{}".format(client._get_request_category(), "request") - tags = ["status:{}".format(status)] + metric = f"{client._get_request_category()}.request" + tags = [f"status:{status}"] if error: - tags.append("error:{}".format(error)) - calls.append(mocker.call(metric, tags=tags + ["attempt:{}".format(attempt)])) + tags.append(f"error:{error}") + calls.append(mocker.call(metric, tags=tags + [f"attempt:{attempt}"])) assert mock_statsd.increment.call_count == call_count mock_statsd.increment.assert_has_calls(calls) @@ -458,7 +457,7 @@ def test_send_request(request_session, mocker, inputs, expected): assert isinstance(response, requests.Response) mock_statsd.timed.assert_called_once_with( - "{}.response_time".format(client.request_category), + f"{client.request_category}.response_time", use_ms=True, tags=inputs["tags"], ) @@ -608,7 +607,7 @@ def test_exception_and_log_metrics(request_session, mocker, inputs, expected): mock_log.assert_called_once_with( "exception", - "{}.failed".format(client.request_category), + f"{client.request_category}.failed", error_type=expected["error_type"], status_code=inputs["status_code"], **expected["extra_params"], diff --git a/test/test_utils.py b/test/test_utils.py index c1f819d..3895e7e 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -1,4 +1,5 @@ """Test the utilities.""" + import sys from typing import Dict, List