Skip to content

Commit

Permalink
chore: upgrade static analysis
Browse files Browse the repository at this point in the history
  • Loading branch information
lucas03 committed Aug 14, 2024
1 parent 9e1a4e2 commit 9368207
Show file tree
Hide file tree
Showing 12 changed files with 46 additions and 61 deletions.
9 changes: 2 additions & 7 deletions .github/workflows/push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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$
Expand Down Expand Up @@ -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]"]
Expand All @@ -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/).*$
Expand Down
3 changes: 0 additions & 3 deletions .pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions request_session/_compat.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Module ensuring lib compatibilty."""

from __future__ import absolute_import

try:
Expand Down
1 change: 1 addition & 0 deletions request_session/exceptions.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down
1 change: 1 addition & 0 deletions request_session/protocols.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Simple protocols to duck type dependency injections."""

from typing import Any, List, Optional


Expand Down
45 changes: 18 additions & 27 deletions request_session/request_session.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Main RequestSession module."""

import re
import time
from collections import namedtuple
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -149,17 +152,7 @@ def set_user_agent(self):
# type: () -> None
"""Set proper user-agent string to header according to RFC22."""
pattern = r"^(?P<service_name>\S.+?)\/(?P<version>\S.+?) \((?P<organization>\S.+?) (?P<environment>\S.+?)\)(?: ?(?P<sys_info>.*))$"
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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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):
Expand All @@ -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):
Expand All @@ -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)

Expand Down Expand Up @@ -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
Expand Down
10 changes: 4 additions & 6 deletions request_session/utils.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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):
Expand Down
4 changes: 2 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions test/conftest.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Just a conftest."""

from typing import Any, Callable

import httpbin as Httpbin
Expand Down
25 changes: 12 additions & 13 deletions test/test_request_session.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Test the main module."""

import itertools
import sys
from typing import Any, Callable, Dict, Iterator, List, Union
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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"],
)
Expand Down Expand Up @@ -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"],
Expand Down
1 change: 1 addition & 0 deletions test/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Test the utilities."""

import sys
from typing import Dict, List

Expand Down

0 comments on commit 9368207

Please sign in to comment.