Instead of having a custom retry code just for query_update(), make
use of the internal .retries attribute that applies to all Bodhi
requests. It also simplifies our code.
Our custom retry code was introduced in b1d4de312. At that time, we
didn't know about internal retry code (it's hidden inside
fedora.client.ProxyClient). I discovered it recently, it makes sense
to use it instead.
test suite adjusted. Tested manually in ipython and with upgradepath. In ipython:
import fedora.client
b = fedora.client.BodhiClient(base_url='http://httpstat.us')
b.retries = 3
import logging
logging.basicConfig()
b.log.setLevel(0logging.getLogger().setLevel(0)
b.send_request('503')
DEBUG:fedora.client.baseclient:No session cached for "None"
DEBUG:fedora.client.baseclient:proxyclient.send_request: entered
DEBUG:fedora.client.baseclient:Creating request http://httpstat.us/503
DEBUG:fedora.client.baseclient:Headers: {'Accept': 'application/json', 'User-agent': 'Fedora Bodhi Client/0.5.1'}
DEBUG:fedora.client.baseclient:Attempt #1 failed
DEBUG:fedora.client.baseclient:Attempt #2 failed
DEBUG:fedora.client.baseclient:Attempt #3 failed
---------------------------------------------------------------------------
ServerError Traceback (most recent call last)
<ipython-input-13-3d4c15b0c892> in <module>()
----> 1 b.send_request('503')
/home/kparal/devel/taskotron/env_taskotron/lib/python2.7/site-packages/fedora/client/baseclient.pyc in send_request(self, method, req_params, file_params, auth, retries, timeout, **kwargs)
367 session_id, data = super(BaseClient, self).send_request(
368 method, req_params=req_params, file_params=file_params,
--> 369 auth_params=auth_params, retries=retries, timeout=timeout)
370 # In case the server returned a new session id to us
371 if self.session_id != session_id:
/home/kparal/devel/taskotron/env_taskotron/lib/python2.7/site-packages/fedora/client/proxyclient.pyc in send_request(self, method, req_params, auth_params, file_params, retries, timeout)
486 except (KeyError, AttributeError):
487 msg = 'Unknown HTTP Server Response'
--> 488 raise ServerError(url, http_status, msg)
489 # Successfully returned data
490 break
ServerError: ServerError(http://httpstat.us/503, 503, Service Unavailable)| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
| Path | Packages | |||
|---|---|---|---|---|
| M | conf/taskotron.yaml.example (4 lines) | |||
| M | libtaskotron/bodhi_utils.py (46 lines) | |||
| M | libtaskotron/config_defaults.py (1 line) | |||
| M | testing/test_bodhi_utils.py (82 lines) |
| Commit | Tree | Parents | Author | Summary | Date |
|---|---|---|---|---|---|
| a573ce9f5df5 | 4bcb09e1814e | a843192daf66 | Kamil Páral | bodhi_utils: use internal retry logic (Show More…) | Jul 23 2015, 7:47 AM |
| Show First 20 Lines • Show All 53 Lines • ▼ Show 20 Line(s) | |||||
| 54 | ## URL of repository of all the RPM packages built in Koji | 54 | ## URL of repository of all the RPM packages built in Koji | ||
| 55 | #pkg_url: http://kojipkgs.fedoraproject.org/packages | 55 | #pkg_url: http://kojipkgs.fedoraproject.org/packages | ||
| 56 | 56 | | |||
| 57 | ## URL of Bodhi instance used for communication about package updates | 57 | ## URL of Bodhi instance used for communication about package updates | ||
| 58 | ## Please make sure the URL doesn't have a trailing slash. | 58 | ## Please make sure the URL doesn't have a trailing slash. | ||
| 59 | #bodhi_server: https://admin.fedoraproject.org/updates | 59 | #bodhi_server: https://admin.fedoraproject.org/updates | ||
| 60 | #bodhi_server: http://localhost:5000/bodhi | 60 | #bodhi_server: http://localhost:5000/bodhi | ||
| 61 | 61 | | |||
| 62 | ## Number of maximum retries to Bodhi server if the query request | | |||
| 63 | ## fails with 5xx HTTP error | | |||
| 64 | #bodhi_request_max_retries: 3 | | |||
| 65 | | ||||
| 66 | ## URL of ResultsDB server API interface, which can store all test results. | 62 | ## URL of ResultsDB server API interface, which can store all test results. | ||
| 67 | ## Please make sure the URL doesn't have a trailing slash. | 63 | ## Please make sure the URL doesn't have a trailing slash. | ||
| 68 | #resultsdb_server: http://localhost:5001/api/v1.0 | 64 | #resultsdb_server: http://localhost:5001/api/v1.0 | ||
| 69 | 65 | | |||
| 70 | ## URL of ResultsDB frontend, which displays results from ResultsDB. | 66 | ## URL of ResultsDB frontend, which displays results from ResultsDB. | ||
| 71 | ## Please make sure the URL doesn't have a trailing slash. | 67 | ## Please make sure the URL doesn't have a trailing slash. | ||
| 72 | #resultsdb_frontend: http://localhost:5002 | 68 | #resultsdb_frontend: http://localhost:5002 | ||
| 73 | 69 | | |||
| ▲ Show 20 Lines • Show All 72 Lines • Show Last 20 Lines | |||||
| Show All 29 Lines | 18 | class BodhiUtils(object): | |||
|---|---|---|---|---|---|
| 30 | 30 | | |||
| 31 | def __init__(self, client=None): | 31 | def __init__(self, client=None): | ||
| 32 | '''Create a new BodhiUtils instance. | 32 | '''Create a new BodhiUtils instance. | ||
| 33 | 33 | | |||
| 34 | :param client: custom :class:`BodhiClient` instance. If ``None``, a | 34 | :param client: custom :class:`BodhiClient` instance. If ``None``, a | ||
| 35 | default BodhiClient instance is used. | 35 | default BodhiClient instance is used. | ||
| 36 | ''' | 36 | ''' | ||
| 37 | 37 | | |||
| 38 | self.taskotron_config = config.get_config() | 38 | self.config = config.get_config() | ||
| 39 | username = self.taskotron_config.fas_username | | |||
| 40 | password = self.taskotron_config.fas_password | | |||
| 41 | bodhi_url = self.taskotron_config.bodhi_server | | |||
| 42 | 39 | | |||
| 43 | if not client: | 40 | if not client: | ||
| 44 | log.debug('Creating Bodhi client to %s' % bodhi_url) | 41 | log.debug('Creating Bodhi client to %s' % self.config.bodhi_server) | ||
| 45 | self.client = fedora.client.bodhi.BodhiClient(username=username, | 42 | self.client = fedora.client.bodhi.BodhiClient(base_url=self.config.bodhi_server, | ||
| 46 | password=password, | 43 | username=self.config.fas_username, | ||
| 47 | base_url=bodhi_url) | 44 | password=self.config.fas_password) | ||
| 45 | # automatically retry failed requests (HTTP 5xx and similar) | ||||
| 46 | self.client.retries = 10 | ||||
| 48 | else: | 47 | else: | ||
| 49 | self.client = client | 48 | self.client = client | ||
| 50 | 49 | | |||
| 51 | | ||||
| 52 | def query_update(self, package): | 50 | def query_update(self, package): | ||
| 53 | '''Get the last Bodhi update matching criteria. Retry | 51 | '''Get the last Bodhi update matching criteria. | ||
| 54 | ``bodhi_request_max_retries``-times when there is a network error or | | |||
| 55 | the server returns HTTP ``5xx`` response. | | |||
| 56 | 52 | | |||
| 57 | :param str package: package NVR or package name or update title or | 53 | :param str package: package NVR or package name or update title or | ||
| 58 | update ID | 54 | update ID | ||
| 59 | 55 | | |||
| 60 | .. note:: | 56 | .. note:: | ||
| 61 | Only NVR allowed, not NEVR. See | 57 | Only NVR allowed, not NEVR. See | ||
| 62 | https://fedorahosted.org/bodhi/ticket/592. | 58 | https://fedorahosted.org/bodhi/ticket/592. | ||
| 63 | :return: most recent Bodhi update object matching criteria, or ``None`` | 59 | :return: most recent Bodhi update object matching criteria, or ``None`` | ||
| 64 | when no update is found | 60 | when no update is found | ||
| 65 | :rtype: :class:`bunch.Bunch` | 61 | :rtype: :class:`bunch.Bunch` | ||
| 66 | ''' | 62 | ''' | ||
| 67 | log.debug('Searching Bodhi updates for: %s', package) | 63 | log.debug('Searching Bodhi updates for: %s', package) | ||
| 68 | max_retries = self.taskotron_config.bodhi_request_max_retries | 64 | res = self.client.query(package=package, limit=1) | ||
| 69 | for retry_num in xrange(1, max_retries + 1): | 65 | if res['updates']: | ||
| 70 | try: | 66 | return res['updates'][0] | ||
| 71 | res = self.client.query(package=package, limit=1) | 67 | else: | ||
| 72 | if res['updates']: | 68 | return None | ||
| 73 | return res['updates'][0] | | |||
| 74 | else: | | |||
| 75 | return None | | |||
| 76 | except fedora.client.ServerError as e: | | |||
| 77 | if retry_num >= max_retries: | | |||
| 78 | log.critical('Maximum Bodhi server retries exceeded') | | |||
| 79 | raise | | |||
| 80 | elif not (e.code == -1 or 500 <= e.code <= 599): | | |||
| 81 | # -1 is a network error, anything positive is an HTTP | | |||
| 82 | # response code | | |||
| 83 | raise | | |||
| 84 | else: | | |||
| 85 | log.warning('Received the following error from Bodhi, ' | | |||
| 86 | 'retrying (%d retries remaining): %s', | | |||
| 87 | max_retries - retry_num, e) | | |||
| 88 | | ||||
| 89 | 69 | | |||
| 90 | def build2update(self, builds, strict=False): | 70 | def build2update(self, builds, strict=False): | ||
| 91 | '''Find matching Bodhi updates for provided builds. | 71 | '''Find matching Bodhi updates for provided builds. | ||
| 92 | 72 | | |||
| 93 | :param builds: builds to search for in N(E)VR format (``foo-1.2-3.fc20`` | 73 | :param builds: builds to search for in N(E)VR format (``foo-1.2-3.fc20`` | ||
| 94 | or ``foo-4:1.2-3.fc20``) | 74 | or ``foo-4:1.2-3.fc20``) | ||
| 95 | :type builds: iterable of str | 75 | :type builds: iterable of str | ||
| 96 | :param bool strict: if ``False``, incomplete Bodhi updates are allowed. | 76 | :param bool strict: if ``False``, incomplete Bodhi updates are allowed. | ||
| ▲ Show 20 Lines • Show All 100 Lines • Show Last 20 Lines | |||||
| Show First 20 Lines • Show All 57 Lines • ▼ Show 20 Line(s) | 23 | class Config(object): | |||
|---|---|---|---|---|---|
| 58 | resultsdb_frontend = 'http://localhost:5002' #: | 58 | resultsdb_frontend = 'http://localhost:5002' #: | ||
| 59 | taskotron_master = 'http://localhost/taskmaster' #: | 59 | taskotron_master = 'http://localhost/taskmaster' #: | ||
| 60 | artifacts_baseurl = 'http://localhost/artifacts' #: | 60 | artifacts_baseurl = 'http://localhost/artifacts' #: | ||
| 61 | buildbot_task_step = 'runtask' #: | 61 | buildbot_task_step = 'runtask' #: | ||
| 62 | download_cache_enabled = True #: | 62 | download_cache_enabled = True #: | ||
| 63 | 63 | | |||
| 64 | bodhi_posting_comments_span = 4320 #: | 64 | bodhi_posting_comments_span = 4320 #: | ||
| 65 | # 3 days (3*24*60 = 4320) | 65 | # 3 days (3*24*60 = 4320) | ||
| 66 | bodhi_request_max_retries = 3 #: | | |||
| 67 | 66 | | |||
| 68 | tmpdir = '/var/tmp/taskotron' #: | 67 | tmpdir = '/var/tmp/taskotron' #: | ||
| 69 | logdir = '/var/log/taskotron' #: | 68 | logdir = '/var/log/taskotron' #: | ||
| 70 | log_name = 'taskotron.log' | 69 | log_name = 'taskotron.log' | ||
| 71 | '''name of the main log file in :attr:`logdir`''' | 70 | '''name of the main log file in :attr:`logdir`''' | ||
| 72 | artifactsdir = '/var/lib/taskotron/artifacts' #: | 71 | artifactsdir = '/var/lib/taskotron/artifacts' #: | ||
| 73 | cachedir = '/var/cache/taskotron' #: | 72 | cachedir = '/var/cache/taskotron' #: | ||
| 74 | 73 | | |||
| ▲ Show 20 Lines • Show All 43 Lines • Show Last 20 Lines | |||||
| 1 | # -*- coding: utf-8 -*- | 1 | # -*- coding: utf-8 -*- | ||
|---|---|---|---|---|---|
| 2 | # Copyright 2009-2014, Red Hat, Inc. | 2 | # Copyright 2009-2014, Red Hat, Inc. | ||
| 3 | # License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+> | 3 | # License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+> | ||
| 4 | # See the LICENSE file for more details on Licensing | 4 | # See the LICENSE file for more details on Licensing | ||
| 5 | 5 | | |||
| 6 | '''Unit tests for libtaskotron/bodhi_utils.py''' | 6 | '''Unit tests for libtaskotron/bodhi_utils.py''' | ||
| 7 | 7 | | |||
| 8 | import pytest | 8 | import pytest | ||
| 9 | from dingus import Dingus | 9 | from dingus import Dingus | ||
| 10 | from bunch import Bunch | 10 | from bunch import Bunch | ||
| 11 | from fedora.client import ServerError | | |||
| 12 | 11 | | |||
| 13 | from libtaskotron import bodhi_utils | 12 | from libtaskotron import bodhi_utils | ||
| 14 | from libtaskotron import exceptions as exc | 13 | from libtaskotron import exceptions as exc | ||
| 15 | 14 | | |||
| 16 | 15 | | |||
| 17 | # http://stackoverflow.com/questions/3190706/nonlocal-keyword-in-python-2-x | | |||
| 18 | def create_failing_server(error_code, response): | | |||
| 19 | first_call = {"value": True} | | |||
| 20 | | ||||
| 21 | def multicall(*args, **kwargs): | | |||
| 22 | if first_call["value"]: | | |||
| 23 | first_call["value"] = False | | |||
| 24 | raise ServerError("nonexist", error_code, "Server or network error") | | |||
| 25 | else: | | |||
| 26 | return response | | |||
| 27 | | ||||
| 28 | return multicall | | |||
| 29 | | ||||
| 30 | | ||||
| 31 | def raise_500_server_error(*args, **kwargs): | | |||
| 32 | raise ServerError("nonexist", 500, "Internal Server Error") | | |||
| 33 | | ||||
| 34 | def raise_404_not_found(*args, **kwargs): | | |||
| 35 | raise ServerError("nonexist", 404, "Not found") | | |||
| 36 | | ||||
| 37 | def raise_minus1_network_error(*args, **kwargs): | | |||
| 38 | raise ServerError("nonexist", -1, "Timed out") | | |||
| 39 | | ||||
| 40 | | ||||
| 41 | class TestQueryUpdate(): | 16 | class TestQueryUpdate(): | ||
| 42 | '''Test query_update()''' | 17 | '''Test query_update()''' | ||
| 43 | 18 | | |||
| 44 | def setup_method(self, method): | 19 | def setup_method(self, method): | ||
| 45 | '''Run this before every test invocation''' | 20 | '''Run this before every test invocation''' | ||
| 46 | self.ref_bodhi_id = 'FEDORA-1234-56789' | 21 | self.ref_bodhi_id = 'FEDORA-1234-56789' | ||
| 47 | self.ref_update = {'title': 'Random update', | 22 | self.ref_update = {'title': 'Random update', | ||
| 48 | 'builds': [{'nvr': 'foo-1.2-3.fc99', | 23 | 'builds': [{'nvr': 'foo-1.2-3.fc99', | ||
| 49 | 'package': {}, | 24 | 'package': {} | ||
| 50 | }], | 25 | }], | ||
| 51 | 'other_keys': {}, | 26 | 'other_keys': {}, | ||
| 52 | } | 27 | } | ||
| 53 | 28 | | |||
| 54 | def test_query_existing_update(self): | 29 | def test_query_existing_update(self): | ||
| 55 | '''Test query for existing update''' | 30 | '''Test query for existing update''' | ||
| 56 | ref_query_answer = {'tg_flash': None, | 31 | ref_query_answer = {'tg_flash': None, | ||
| 57 | 'num_items': 1, | 32 | 'num_items': 1, | ||
| 58 | 'title': '1 update found', | 33 | 'title': '1 update found', | ||
| 59 | 'updates': [self.ref_update], | 34 | 'updates': [self.ref_update], | ||
| 60 | } | 35 | } | ||
| 61 | stub_bodhi = Dingus(query__returns=ref_query_answer) | 36 | stub_bodhi = Dingus(query__returns=ref_query_answer) | ||
| 62 | bodhi = bodhi_utils.BodhiUtils(client=stub_bodhi) | 37 | bodhi = bodhi_utils.BodhiUtils(client=stub_bodhi) | ||
| 63 | update = bodhi.query_update(self.ref_bodhi_id) | 38 | update = bodhi.query_update(self.ref_bodhi_id) | ||
| 64 | 39 | | |||
| 65 | assert update == self.ref_update | 40 | assert update == self.ref_update | ||
| 66 | 41 | | |||
| 67 | def test_query_non_existing_update(self, monkeypatch): | 42 | def test_query_non_existing_update(self, monkeypatch): | ||
| 68 | '''Test query for non-existing update''' | 43 | '''Test query for non-existing update''' | ||
| 69 | ref_query_answer = {'tg_flash': None, | 44 | ref_query_answer = {'tg_flash': None, | ||
| 70 | 'num_items': 0, | 45 | 'num_items': 0, | ||
| 71 | 'title': '0 updates found', | 46 | 'title': '0 updates found', | ||
| 72 | 'updates': [], | 47 | 'updates': [], | ||
| 73 | } | 48 | } | ||
| 74 | stub_bodhi = Dingus(query__returns=ref_query_answer) | 49 | stub_bodhi = Dingus(query__returns=ref_query_answer) | ||
| 75 | bodhi = bodhi_utils.BodhiUtils(client=stub_bodhi) | 50 | bodhi = bodhi_utils.BodhiUtils(client=stub_bodhi) | ||
| 76 | update = bodhi.query_update(self.ref_bodhi_id) | 51 | update = bodhi.query_update(self.ref_bodhi_id) | ||
| 77 | 52 | | |||
| 78 | assert update is None | 53 | assert update is None | ||
| 79 | 54 | | |||
| 80 | def test_query_retry_500(self): | | |||
| 81 | '''Test whether query can retry if server returns HTTP 500''' | | |||
| 82 | ref_query_answer = create_failing_server(500, | | |||
| 83 | {'tg_flash': None, | | |||
| 84 | 'num_items': 1, | | |||
| 85 | 'title': '1 update found', | | |||
| 86 | 'updates': [self.ref_update]}) | | |||
| 87 | stub_bodhi = Dingus(query=ref_query_answer) | | |||
| 88 | bodhi = bodhi_utils.BodhiUtils(client=stub_bodhi) | | |||
| 89 | update = bodhi.query_update(self.ref_bodhi_id) | | |||
| 90 | | ||||
| 91 | assert update == self.ref_update | | |||
| 92 | | ||||
| 93 | def test_query_retry_minus1(self): | | |||
| 94 | '''Test whether query can retry if there's a network error (e.g. timed | | |||
| 95 | out)''' | | |||
| 96 | ref_query_answer = create_failing_server(-1, | | |||
| 97 | {'tg_flash': None, | | |||
| 98 | 'num_items': 1, | | |||
| 99 | 'title': '1 update found', | | |||
| 100 | 'updates': [self.ref_update]}) | | |||
| 101 | stub_bodhi = Dingus(query=ref_query_answer) | | |||
| 102 | bodhi = bodhi_utils.BodhiUtils(client=stub_bodhi) | | |||
| 103 | update = bodhi.query_update(self.ref_bodhi_id) | | |||
| 104 | | ||||
| 105 | assert update == self.ref_update | | |||
| 106 | | ||||
| 107 | def test_query_retry_fail(self): | | |||
| 108 | '''Test query on failing server''' | | |||
| 109 | stub_bodhi = Dingus(query=raise_500_server_error) | | |||
| 110 | bodhi = bodhi_utils.BodhiUtils(client=stub_bodhi) | | |||
| 111 | | ||||
| 112 | with pytest.raises(ServerError): | | |||
| 113 | bodhi.query_update(self.ref_bodhi_id) | | |||
| 114 | | ||||
| 115 | stub_bodhi = Dingus(query=raise_404_not_found) | | |||
| 116 | bodhi = bodhi_utils.BodhiUtils(client=stub_bodhi) | | |||
| 117 | | ||||
| 118 | with pytest.raises(ServerError): | | |||
| 119 | bodhi.query_update(self.ref_bodhi_id) | | |||
| 120 | | ||||
| 121 | stub_bodhi = Dingus(query=raise_minus1_network_error) | | |||
| 122 | bodhi = bodhi_utils.BodhiUtils(client=stub_bodhi) | | |||
| 123 | | ||||
| 124 | with pytest.raises(ServerError): | | |||
| 125 | bodhi.query_update(self.ref_bodhi_id) | | |||
| 126 | | ||||
| 127 | 55 | | |||
| 128 | @pytest.mark.usefixtures('setup') | 56 | @pytest.mark.usefixtures('setup') | ||
| 129 | class TestBuild2Update(): | 57 | class TestBuild2Update(): | ||
| 130 | '''Test build2update()''' | 58 | '''Test build2update()''' | ||
| 131 | 59 | | |||
| 132 | # fake requests and responses for query_update() | 60 | # fake requests and responses for query_update() | ||
| 133 | requests = { | 61 | requests = { | ||
| 134 | 'foo-ok-1.2-3.fc20': Bunch(builds=[Bunch(nvr=u'foo-ok-1.2-3.fc20')]), | 62 | 'foo-ok-1.2-3.fc20': Bunch(builds=[Bunch(nvr=u'foo-ok-1.2-3.fc20')]), | ||
| ▲ Show 20 Lines • Show All 162 Lines • Show Last 20 Lines | |||||