Infrastructure/toddlers
1
0
Fork 0
Code Issues 10 Pull requests 1 Projects Releases Packages Wiki Activity Actions

Use factory for requests session objects

Browse source 
The `make_session()` factory creates session objects with default values
for timeouts, number of retries, etc., which can be overridden when
calling it.

Create such (long-lived) session objects in the constructors of the
individual toddlers using requests to access remote APIs. Pull in
module-global functions into the toddler class where necessary and adapt
tests to accommodate.

Also, mock out the factory function in the `toddler´ fixture for testing.

Signed-off-by: Nils Philippsen <nils@redhat.com>
This commit is contained in:
Nils Philippsen 2020-07-10 17:32:11 +02:00
parent 2c13cf8e4c
commit d53bd2ee73
9 changed files with 162 additions and 164 deletions
Download patch file Download diff file Expand all files Collapse all files

14
tests/conftest.py
View file

@ -1,17 +1,31 @@
import sys
from unittest.mock import MagicMock
import pytest
from toddlers.utils.requests import make_session
@pytest.fixture
def toddler(request, monkeypatch):
"""Fixture creating a toddler for a class testing it
The test class must set the toddler class in its `toddler_cls` attribute.
The fixture will mock out the `make_session` function before creating the
object so that any request objects the toddler creates in its constructor
won't be real.
"""
test_cls = request.cls
if not hasattr(test_cls, "toddler_cls"):
raise RuntimeError(f"{test_cls} needs to set `toddler_cls`")
toddler_cls = test_cls.toddler_cls
toddler_module = sys.modules[toddler_cls.__module__]
for name in dir(toddler_module):
obj = getattr(toddler_module, name)
if obj is make_session:
monkeypatch.setattr(toddler_module, name, MagicMock())
toddler_obj = toddler_cls()
return toddler_obj

14
tests/plugins/test_flag_ci_pr.py
View file

@ -1,5 +1,5 @@
import logging
from unittest.mock import MagicMock, patch
from unittest.mock import MagicMock
import fedora_messaging.api
import pytest
@ -85,11 +85,11 @@ class TestFlagCIPRToddler:
"the PR id from it"
)
@patch("toddlers.plugins.flag_ci_pr.requests_session")
def test_process_request_failed(self, mock_requests, toddler, caplog):
mock_requests.request.return_value = MagicMock(
def test_process_request_failed(self, toddler, caplog):
toddler.requests_session.request.return_value = MagicMock(
ok=False, status_code=401, text="invalid"
)
caplog.set_level(logging.INFO)
msg = fedora_messaging.api.Message()
msg.id = 123
@ -115,11 +115,11 @@ class TestFlagCIPRToddler:
caplog.records[-1].message == "Request to https://pagure.io returned: 401"
)
@patch("toddlers.plugins.flag_ci_pr.requests_session")
def test_process_valid(self, mock_requests, toddler, caplog):
mock_requests.request.return_value = MagicMock(
def test_process_valid(self, toddler, caplog):
toddler.requests_session.request.return_value = MagicMock(
ok=True, status_code=200, text="invalid"
)
caplog.set_level(logging.INFO)
msg = fedora_messaging.api.Message()
msg.id = 123

30
tests/plugins/test_flag_commit_build.py
View file

@ -114,12 +114,12 @@ class TestFlagCommitBuildToddler:
assert toddler.process(config=config, message=msg) is None
assert caplog.records[-1].message == "No # in the git_url: foobar"
@patch("toddlers.plugins.flag_commit_build.requests_session")
@patch("toddlers.plugins.flag_commit_build.koji")
def test_process_flag_no_task_id(self, mock_koji, mock_requests, toddler, caplog):
mock_requests.request.return_value = MagicMock(
def test_process_flag_no_task_id(self, mock_koji, toddler, caplog):
toddler.requests_session.request.return_value = MagicMock(
ok=False, status_code=401, text="invalid"
)
client = Mock()
client.getBuild = Mock(
return_value={
@ -132,6 +132,7 @@ class TestFlagCommitBuildToddler:
}
)
mock_koji.ClientSession = Mock(return_value=client)
caplog.set_level(logging.INFO)
msg = fedora_messaging.api.Message()
msg.id = 123
@ -158,12 +159,12 @@ class TestFlagCommitBuildToddler:
== "Request to https://src.fedoraproject.org returned: 401"
)
@patch("toddlers.plugins.flag_commit_build.requests_session")
@patch("toddlers.plugins.flag_commit_build.koji")
def test_process_flag_failed(self, mock_koji, mock_requests, toddler, caplog):
mock_requests.request.return_value = MagicMock(
def test_process_flag_failed(self, mock_koji, toddler, caplog):
toddler.requests_session.request.return_value = MagicMock(
ok=False, status_code=401, text="invalid"
)
client = Mock()
client.getBuild = Mock(
return_value={
@ -176,6 +177,7 @@ class TestFlagCommitBuildToddler:
}
)
mock_koji.ClientSession = Mock(return_value=client)
caplog.set_level(logging.INFO)
msg = fedora_messaging.api.Message()
msg.id = 123
@ -202,14 +204,12 @@ class TestFlagCommitBuildToddler:
== "Request to https://src.fedoraproject.org returned: 401"
)
@patch("toddlers.plugins.flag_commit_build.requests_session")
@patch("toddlers.plugins.flag_commit_build.koji")
def test_process_flag_failed_cancel(
self, mock_koji, mock_requests, toddler, caplog
):
mock_requests.request.return_value = MagicMock(
def test_process_flag_failed_cancel(self, mock_koji, toddler, caplog):
toddler.requests_session.request.return_value = MagicMock(
ok=False, status_code=401, text="invalid"
)
client = Mock()
client.getBuild = Mock(
return_value={
@ -222,6 +222,7 @@ class TestFlagCommitBuildToddler:
}
)
mock_koji.ClientSession = Mock(return_value=client)
caplog.set_level(logging.INFO)
msg = fedora_messaging.api.Message()
msg.id = 123
@ -248,12 +249,12 @@ class TestFlagCommitBuildToddler:
== "Request to https://src.fedoraproject.org returned: 401"
)
@patch("toddlers.plugins.flag_commit_build.requests_session")
@patch("toddlers.plugins.flag_commit_build.koji")
def test_process_flag_pass(self, mock_koji, mock_requests, toddler, caplog):
mock_requests.request.return_value = MagicMock(
def test_process_flag_pass(self, mock_koji, toddler, caplog):
toddler.requests_session.request.return_value = MagicMock(
ok=True, status_code=200, text="woohoo!"
)
client = Mock()
client.getBuild = Mock(
return_value={
@ -266,6 +267,7 @@ class TestFlagCommitBuildToddler:
}
)
mock_koji.ClientSession = Mock(return_value=client)
caplog.set_level(logging.INFO)
msg = fedora_messaging.api.Message()
msg.id = 123

58
tests/plugins/test_pdc_retired_packages.py
View file

@ -61,8 +61,8 @@ class TestPDCRetiredPackagesToddler:
process_dg.assert_called_once_with({"config": "foobar"}, client, msg)
@patch("toddlers.plugins.pdc_retired_packages._is_retired_in_dist_git")
def test_process_dist_git(self, retired_in_dg, toddler):
def test_process_dist_git(self, toddler):
retired_in_dg = toddler._is_retired_in_dist_git = MagicMock()
page_component_branches = [
{
"id": 44,
@ -122,10 +122,8 @@ class TestPDCRetiredPackagesToddler:
]
)
@patch("toddlers.plugins.pdc_retired_packages._is_retired_in_dist_git")
def test_process_dist_git_invalid_pdc_namespace(
self, retired_in_dg, caplog, toddler
):
def test_process_dist_git_invalid_pdc_namespace(self, caplog, toddler):
retired_in_dg = toddler._is_retired_in_dist_git = MagicMock()
caplog.set_level(logging.DEBUG)
page_component_branches = [
{
@ -189,9 +187,9 @@ class TestPDCRetiredPackagesToddler:
in caplog.text
)
@patch("toddlers.plugins.pdc_retired_packages._retire_branch")
@patch("toddlers.plugins.pdc_retired_packages._is_retired_in_dist_git")
def test_process_dist_git_full_distgit(self, retired_in_dg, retire_branch, toddler):
def test_process_dist_git_full_distgit(self, toddler):
retired_in_dg = toddler._is_retired_in_dist_git = MagicMock()
retire_branch = toddler._retire_branch = MagicMock()
page_component_branches = [
{
"id": 44,
@ -497,12 +495,11 @@ class TestPDCRetiredPackagesToddler:
assert caplog.records[-1].message == '"rpms/0ad" not active in PDC in master'
@patch("requests.head")
def test__process_single_package_package_not_retired(self, req, toddler, caplog):
def test__process_single_package_package_not_retired(self, toddler, caplog):
caplog.set_level(logging.INFO)
resp = Mock()
resp.status_code = 404
req.return_value = resp
toddler.requests_session.head.return_value = resp
page_component_branches = {
"count": 1,
@ -553,13 +550,12 @@ class TestPDCRetiredPackagesToddler:
== "Seems not to actually be retired, possibly a merge commit?"
)
@patch("toddlers.plugins.pdc_retired_packages._retire_branch")
@patch("requests.head")
def test__process_single_package(self, req, retire, toddler, caplog):
def test__process_single_package(self, toddler, caplog):
toddler._retire_branch = MagicMock()
caplog.set_level(logging.INFO)
resp = Mock()
resp.status_code = 200
req.return_value = resp
toddler.requests_session.head.return_value = resp
page_component_branches = {
"count": 1,
@ -605,40 +601,36 @@ class TestPDCRetiredPackagesToddler:
msg,
)
retire.assert_called_once_with(client, page_component_branches["results"][0])
toddler._retire_branch.assert_called_once_with(
client, page_component_branches["results"][0]
)
@patch("toddlers.plugins.pdc_retired_packages.requests_session.head")
def test__is_retired_in_dist_git(self, req):
def test__is_retired_in_dist_git(self, toddler):
resp = Mock()
resp.status_code = 200
req.return_value = resp
toddler.requests_session.head.return_value = resp
assert toddlers.plugins.pdc_retired_packages._is_retired_in_dist_git(
"rpms", "guake", "f33"
)
req.assert_called_once_with(
assert toddler._is_retired_in_dist_git("rpms", "guake", "f33")
toddler.requests_session.head.assert_called_once_with(
"https://src.fedoraproject.org//rpms/guake/raw/f33/f/dead.package"
)
@patch("toddlers.plugins.pdc_retired_packages.requests_session.head")
def test__is_retired_in_dist_git_internal_error(self, req):
def test__is_retired_in_dist_git_internal_error(self, toddler):
resp = Mock()
resp.status_code = 500
req.return_value = resp
toddler.requests_session.head.return_value = resp
with pytest.raises(
ValueError,
match=r"The connection to dist_git failed. Retirement status could "
"not be determined. The status code was: 500. The content was: .*",
):
toddlers.plugins.pdc_retired_packages._is_retired_in_dist_git(
"rpms", "guake", "f33"
)
req.assert_called_once_with(
toddler._is_retired_in_dist_git("rpms", "guake", "f33")
toddler.requests_session.head.assert_called_once_with(
"https://src.fedoraproject.org//rpms/guake/raw/f33/f/dead.package"
)
def test_retire_branch(self, caplog):
def test_retire_branch(self, toddler, caplog):
caplog.set_level(logging.INFO)
today = datetime.datetime.utcnow().date()
eol = (datetime.datetime.utcnow() + datetime.timedelta(days=60)).date()
@ -653,7 +645,7 @@ class TestPDCRetiredPackagesToddler:
"critical_path": False,
}
toddlers.plugins.pdc_retired_packages._retire_branch(pdc, branch)
toddler._retire_branch(pdc, branch)
assert (
caplog.records[-1].message

21
toddlers/plugins/flag_ci_pr.py
View file

@ -10,24 +10,12 @@ Authors: Pierre-Yves Chibon <pingou@pingoured.fr>
import hashlib
import logging
import requests
from requests.packages.urllib3.util import retry
from ..base import ToddlerBase
from ..utils.requests import make_session
_log = logging.getLogger(__name__)
timeout = (30, 30)
retries = 3
requests_session = requests.Session()
retry_conf = retry.Retry(total=retries, connect=retries, read=retries, backoff_factor=1)
retry_conf.BACKOFF_MAX = 5
requests_session.mount("http://", requests.adapters.HTTPAdapter(max_retries=retry_conf))
requests_session.mount(
"https://", requests.adapters.HTTPAdapter(max_retries=retry_conf)
)
done_states = {
"passed": {"api": "success", "human": "passed"},
"error": {"api": "error", "human": "errored"},
@ -49,6 +37,9 @@ class FlagCIPR(ToddlerBase):
"org.centos.*.ci.dist-git-pr.test.running",
]
def __init__(self):
self.requests_session = make_session()
def accepts_topic(self, topic):
"""Returns a boolean whether this toddler is interested in messages
from this specific topic.
@ -135,8 +126,8 @@ class FlagCIPR(ToddlerBase):
_log.info("payload: %s" % data)
req = requests_session.request(
method="POST", url=flag_url, headers=headers, data=data,
req = self.requests_session.request(
method="POST", url=flag_url, headers=headers, data=data
)
_log.info("Request to %s returned: %s" % (dist_git_url, req.status_code))
if not req.ok:

20
toddlers/plugins/flag_commit_build.py
View file

@ -10,24 +10,13 @@ Authors: Pierre-Yves Chibon <pingou@pingoured.fr>
import logging
import koji
import requests
from requests.packages.urllib3.util import retry
from ..base import ToddlerBase
from ..utils.requests import make_session
_log = logging.getLogger(__name__)
timeout = (30, 30)
retries = 3
requests_session = requests.Session()
retry_conf = retry.Retry(total=retries, connect=retries, read=retries, backoff_factor=1)
retry_conf.BACKOFF_MAX = 5
requests_session.mount("http://", requests.adapters.HTTPAdapter(max_retries=retry_conf))
requests_session.mount(
"https://", requests.adapters.HTTPAdapter(max_retries=retry_conf)
)
# see koji.TASK_STATES for all values
done_states = {
1: "completed",
@ -56,6 +45,9 @@ class FlagCommitBuild(ToddlerBase):
"org.fedoraproject.*.buildsys.build.state.change",
]
def __init__(self):
self.requests_session = make_session()
def accepts_topic(self, topic):
"""Returns a boolean whether this toddler is interested in messages
from this specific topic.
@ -141,8 +133,8 @@ class FlagCommitBuild(ToddlerBase):
_log.debug("payload: %s" % data)
req = requests_session.request(
method="POST", url=flag_url, headers=headers, data=data,
req = self.requests_session.request(
method="POST", url=flag_url, headers=headers, data=data
)
_log.info("Request to %s returned: %s" % (dist_git_url, req.status_code))
if not req.ok:

12
toddlers/plugins/packager_bugzilla_sync.py
View file

@ -12,8 +12,6 @@ import argparse
import logging
import sys
import requests
from requests.packages.urllib3.util import retry
import toml
try:
@ -28,16 +26,6 @@ from ..utils import fedora_account
_log = logging.getLogger(__name__)
timeout = (30, 30)
retries = 3
requests_session = requests.Session()
retry_conf = retry.Retry(total=retries, connect=retries, read=retries, backoff_factor=1)
retry_conf.BACKOFF_MAX = 5
requests_session.mount("http://", requests.adapters.HTTPAdapter(max_retries=retry_conf))
requests_session.mount(
"https://", requests.adapters.HTTPAdapter(max_retries=retry_conf)
)
class PackagerBugzillaSync(ToddlerBase):
""" Listens to messages sent by playtime (which lives in toddlers) to sync

44
toddlers/plugins/pdc_retired_packages.py
View file

@ -12,23 +12,12 @@ from datetime import datetime
import logging
import pdc_client
import requests
from requests.packages.urllib3.util import retry
from ..base import ToddlerBase
from ..utils.requests import make_session
_log = logging.getLogger(__name__)
timeout = (30, 30)
retries = 3
requests_session = requests.Session()
retry_conf = retry.Retry(total=retries, connect=retries, read=retries, backoff_factor=1)
retry_conf.BACKOFF_MAX = 5
requests_session.mount("http://", requests.adapters.HTTPAdapter(max_retries=retry_conf))
requests_session.mount(
"https://", requests.adapters.HTTPAdapter(max_retries=retry_conf)
)
class PDCRetiredPackages(ToddlerBase):
""" Listens to messages sent by playtime (which lives in toddlers) to sync
@ -42,6 +31,9 @@ class PDCRetiredPackages(ToddlerBase):
"org.fedoraproject.*.git.receive",
]
def __init__(self):
self.requests_session = make_session()
def accepts_topic(self, topic):
"""Returns a boolean whether this toddler is interested in messages
from this specific topic.
@ -71,8 +63,8 @@ class PDCRetiredPackages(ToddlerBase):
_log.debug("Considering {0}".format(branch_str))
retired_in_dist_git = False
try:
retired_in_dist_git = _is_retired_in_dist_git(
namespace=_pdc_to_namespace(branch["type"]),
retired_in_dist_git = self._is_retired_in_dist_git(
namespace=self._pdc_to_namespace(branch["type"]),
repo=branch["global_component"],
branch=branch["name"],
)
@ -81,7 +73,7 @@ class PDCRetiredPackages(ToddlerBase):
continue
if retired_in_dist_git:
_retire_branch(pdc, branch)
self._retire_branch(pdc, branch)
def _process_single_package(self, config, pdc, message):
"""Handle an incoming bus message.
@ -110,7 +102,7 @@ class PDCRetiredPackages(ToddlerBase):
repo = msg["commit"]["repo"]
namespace = msg["commit"]["namespace"]
try:
component_type = _namespace_to_pdc(namespace)
component_type = self._namespace_to_pdc(namespace)
except ValueError as err:
_log.info("Ignoring namespace '%s', error: %s", namespace, err)
return
@ -150,15 +142,14 @@ class PDCRetiredPackages(ToddlerBase):
"file": "dead.package",
}
_log.info("Checking for file: %s", fileurl)
resp = requests.head(fileurl, timeout=15)
resp = self.requests_session.head(fileurl, timeout=15)
if resp.status_code != 200:
_log.info("Seems not to actually be retired, possibly a merge commit?")
return
_retire_branch(pdc, branch)
self._retire_branch(pdc, branch)
def _namespace_to_pdc(namespace):
def _namespace_to_pdc(self, namespace):
""" Internal method to translate a dist-git namespace to a PDC
component type. """
namespace_to_pdc = {
@ -171,8 +162,7 @@ def _namespace_to_pdc(namespace):
else:
return namespace_to_pdc[namespace]
def _pdc_to_namespace(pdc_type):
def _pdc_to_namespace(self, pdc_type):
""" Internal method to translate a PDC component type to a dist-git
namespace. """
pdc_to_namespace = {
@ -185,13 +175,12 @@ def _pdc_to_namespace(pdc_type):
else:
return pdc_to_namespace[pdc_type]
def _is_retired_in_dist_git(namespace, repo, branch):
def _is_retired_in_dist_git(self, namespace, repo, branch):
base = "https://src.fedoraproject.org/"
# Check to see if they have a dead.package file in dist-git
url = "{base}/{namespace}/{repo}/raw/{branch}/f/dead.package"
response = requests_session.head(
url.format(base=base, namespace=namespace, repo=repo, branch=branch,)
response = self.requests_session.head(
url.format(base=base, namespace=namespace, repo=repo, branch=branch)
)
# If there is a dead.package, then the branch is retired in dist_git
@ -204,8 +193,7 @@ def _is_retired_in_dist_git(namespace, repo, branch):
"{1}".format(response.status_code, response.content)
)
def _retire_branch(pdc, branch):
def _retire_branch(self, pdc, branch):
""" Internal method for retiring a branch in PDC. """
today = datetime.utcnow().date()
for sla in branch["slas"]:

31
toddlers/utils/requests.py Normal file
View file

@ -0,0 +1,31 @@
"""Common requests session object."""
import requests
from requests.packages.urllib3.util import retry
__all__ = ["make_session"]
RETRY_DEFAULTS = {
"total": 3,
"backoff_factor": 1,
"BACKOFF_MAX": 5,
}
def make_session(**retry_args):
for k, v in RETRY_DEFAULTS.items():
retry_args.setdefault(k, v)
BACKOFF_MAX = retry_args.pop("BACKOFF_MAX")
retry_conf = retry.Retry(**retry_args)
retry_conf.BACKOFF_MAX = BACKOFF_MAX
http_adapter = requests.adapters.HTTPAdapter(max_retries=retry_conf)
session = requests.Session()
for prefix in ("http://", "https://"):
session.mount(prefix, http_adapter)
return session