From 1f590f7ed735ad3e948d39224c7ac2645ae9d68e Mon Sep 17 00:00:00 2001 From: Pavel Raiskup Date: Oct 01 2019 16:16:02 +0000 Subject: frontend, backend: add FrontendClient.{get,post,put} We shouldn't use FrontendClient's private/protected methods outside of frontend.py, same as we shouldn't use manual 'requests.*' methods when we have reliable 'frontend_client'. So enhance FrontendClient API and use it everywhere where we talk to `backend_ns`. Frontend's '/backend/chroots-prunerepo-status/' is naturally GET only, so change the methods= argument there (backend will use GET as well now). Merges: #1021 --- diff --git a/backend/backend/daemons/action_dispatcher.py b/backend/backend/daemons/action_dispatcher.py index 133667e..08cbf54 100644 --- a/backend/backend/daemons/action_dispatcher.py +++ b/backend/backend/daemons/action_dispatcher.py @@ -3,9 +3,9 @@ import time import multiprocessing from setproctitle import setproctitle -from requests import get, RequestException from backend.frontend import FrontendClient +from backend.exceptions import FrontendClientException from ..actions import ActionWorkerManager, ActionQueueTask from ..helpers import get_redis_logger, get_redis_connection @@ -23,6 +23,7 @@ class ActionDispatcher(multiprocessing.Process): self.opts = opts self.log = get_redis_logger(self.opts, "backend.action_dispatcher", "action_dispatcher") + self.frontend_client = FrontendClient(self.opts, self.log) def update_process_title(self, msg=None): proc_title = "Action dispatcher" @@ -36,10 +37,8 @@ class ActionDispatcher(multiprocessing.Process): """ try: - url = "{0}/backend/pending-actions/".format(self.opts.frontend_base_url) - request = get(url, auth=("user", self.opts.frontend_auth)) - raw_actions = request.json() - except (RequestException, ValueError) as error: + raw_actions = self.frontend_client.get('pending-actions').json() + except (FrontendClientException, ValueError) as error: self.log.exception( "Retrieving an action tasks failed with error: %s", error) diff --git a/backend/backend/daemons/build_dispatcher.py b/backend/backend/daemons/build_dispatcher.py index 740dd61..64e19e7 100644 --- a/backend/backend/daemons/build_dispatcher.py +++ b/backend/backend/daemons/build_dispatcher.py @@ -6,7 +6,6 @@ import multiprocessing from collections import defaultdict from setproctitle import setproctitle -from requests import get, RequestException from backend.frontend import FrontendClient @@ -81,10 +80,8 @@ class BuildDispatcher(multiprocessing.Process): self.update_process_title("Waiting for jobs from frontend for {} s" .format(int(time.time() - get_task_init_time))) try: - tasks = get("{0}/backend/pending-jobs/".format(self.opts.frontend_base_url), - auth=("user", self.opts.frontend_auth)).json() - - except (RequestException, ValueError) as error: + tasks = self.frontend_client.get('pending-jobs').json() + except (FrontendClientException, ValueError) as error: self.log.exception("Retrieving build jobs from %s failed with error: %s", self.opts.frontend_base_url, error) finally: diff --git a/backend/backend/frontend.py b/backend/backend/frontend.py index 4c6f61c..154969d 100644 --- a/backend/backend/frontend.py +++ b/backend/backend/frontend.py @@ -1,7 +1,7 @@ import json import time import logging -from requests import post, get, RequestException +from requests import get, post, put, RequestException from backend.exceptions import FrontendClientException @@ -38,15 +38,24 @@ class FrontendClient(object): self.logger.addHandler(logging.NullHandler()) return self.logger - def _frontend_request(self, url_path, data=None, authenticate=True): + def _frontend_request(self, url_path, data=None, authenticate=True, + method='post'): headers = {"content-type": "application/json"} url = "{}/{}/".format(self.frontend_url, url_path) auth = ("user", self.frontend_auth) if authenticate else None try: - # TODO: no data => use get() - response = post(url, data=json.dumps(data), auth=auth, - headers=headers) + kwargs = { + 'auth': auth, + 'headers': headers, + } + method = method.lower() + if method in ['post', 'put']: + kwargs['data'] = json.dumps(data) + method = post if method == 'post' else put + else: + method = get + response = method(url, **kwargs) except RequestException as ex: raise FrontendClientRetryError( "Requests error on {}: {}".format(url, str(ex))) @@ -68,28 +77,8 @@ class FrontendClient(object): # TODO: Success, but tighten the redirects etc. return response - def get_reliably(self, url_path): - """ - Get the URL response from frontend, try indefinitely till the server - gives us answer. - """ - url = "{}/{}/".format(self.frontend_url, url_path) - auth = ("user", self.frontend_auth) - - attempt = 0 - while True: - attempt += 1 - try: - response = get(url, auth=auth) - except RequestException as ex: - self.msg = "Get request {} failed: {}".format(attempt, ex) - time.sleep(RETRY_TIMEOUT) - continue - - return response - - - def _post_to_frontend_repeatedly(self, data, url_path): + def _frontend_request_repeatedly(self, url_path, method='post', data=None, + authenticate=True): """ Repeat the request until it succeeds, or timeout is reached. """ @@ -106,13 +95,35 @@ class FrontendClient(object): "(we gave it {} attempts)".format(i)) try: - return self._frontend_request(url_path, data=data) + return self._frontend_request(url_path, data=data, + authenticate=authenticate, + method=method) except FrontendClientRetryError as ex: self.log.warning("Retry request #%s on %s: %s", i, url_path, str(ex)) time.sleep(sleep) sleep += SLEEP_INCREMENT_TIME + + def _post_to_frontend_repeatedly(self, data, url_path): + """ + Repeat the request until it succeeds, or timeout is reached. + """ + return self._frontend_request_repeatedly(url_path, data=data) + + def get(self, url_path): + 'Issue relentless GET request to Frontend' + return self._frontend_request_repeatedly(url_path, method='get') + + def post(self, url_path, data): + 'Issue relentless POST request to Frontend' + return self._frontend_request_repeatedly(url_path, data=data) + + def put(self, url_path, data): + 'Issue relentless POST request to Frontend' + return self._frontend_request_repeatedly(url_path, data=data, + method='put') + def update(self, data): """ Send data to be updated in the frontend diff --git a/backend/run/copr-backend-process-action b/backend/run/copr-backend-process-action index 0067cbc..ffa3227 100755 --- a/backend/run/copr-backend-process-action +++ b/backend/run/copr-backend-process-action @@ -55,7 +55,7 @@ def handle_task(opts, args, log): redis.hset(args.worker_id, 'started', 1) redis.hset(args.worker_id, 'PID', os.getpid()) - resp = frontend_client.get_reliably('action/{}'.format(task_id)) + resp = frontend_client.get('action/{}'.format(task_id)) if resp.status_code != 200: log.error("failed to download task, apache code %s", resp.status_code) sys.exit(1) diff --git a/backend/run/copr_prune_results.py b/backend/run/copr_prune_results.py index b3f502b..755168f 100755 --- a/backend/run/copr_prune_results.py +++ b/backend/run/copr_prune_results.py @@ -79,7 +79,7 @@ class Pruner(object): self.mtime_optimization = not cmdline_opts.no_mtime_optimization def run(self): - response = self.frontend_client._post_to_frontend_repeatedly("", "chroots-prunerepo-status") + response = self.frontend_client.get("chroots-prunerepo-status") self.chroots = json.loads(response.content) results_dir = self.opts.destdir @@ -102,7 +102,7 @@ class Pruner(object): for chroot, active in self.chroots.items(): if not active: chroots_to_prune.append(chroot) - self.frontend_client._post_to_frontend_repeatedly(chroots_to_prune, "final-prunerepo-done") + self.frontend_client.post(chroots_to_prune, "final-prunerepo-done") loginfo("--------------------------------------------") loginfo("Pruning finished") diff --git a/backend/tests/test_frontend.py b/backend/tests/test_frontend.py index 3056667..84abbca 100644 --- a/backend/tests/test_frontend.py +++ b/backend/tests/test_frontend.py @@ -18,6 +18,12 @@ def post_req(): with mock.patch("backend.frontend.post") as obj: yield obj +@pytest.fixture(scope='function', params=['get', 'post', 'put']) +def f_request_method(request): + 'mock the requests.{get,post,put} method' + with mock.patch("backend.frontend.{}".format(request.param)) as ctx: + yield (request.param, ctx) + @pytest.yield_fixture def mc_time(): @@ -49,11 +55,23 @@ class TestFrontendClient(object): self.f_r = MagicMock() self.fc._frontend_request = self.f_r - def test_post_to_frontend(self, post_req): - post_req.return_value.status_code = 200 - self.fc._frontend_request(self.url_path, self.data) + def test_post_to_frontend(self, f_request_method): + name, method = f_request_method + method.return_value.status_code = 200 + self.fc._frontend_request(self.url_path, self.data, method=name) + assert method.called - assert post_req.called + def test_post_to_frontend_wrappers(self, f_request_method): + name, method = f_request_method + method.return_value.status_code = 200 + + call = getattr(self.fc, name) + if name == 'get': + call(self.url_path) + else: + call(self.url_path, self.data) + + assert method.called def test_post_to_frontend_not_200(self, post_req): post_req.return_value.status_code = 501 @@ -77,14 +95,21 @@ class TestFrontendClient(object): assert self.fc._post_to_frontend_repeatedly(self.data, self.url_path) == response assert not mc_time.sleep.called - def test_post_to_frontend_repeated_second_try_ok(self, mask_frontend_request, mc_time): + def test_post_to_frontend_repeated_second_try_ok(self, f_request_method, + mask_frontend_request, mc_time): + method_name, method = f_request_method + response = "ok\n" self.f_r.side_effect = [ FrontendClientRetryError(), response, ] mc_time.time.return_value = 0 - assert self.fc._post_to_frontend_repeatedly(self.data, self.url_path) == response + assert self.fc._frontend_request_repeatedly( + self.url_path, + data=self.data, + method=method_name + ) == response assert mc_time.sleep.called def test_post_to_frontend_err_400(self, post_req, mc_time): diff --git a/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py b/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py index d5b56ea..e7cd98f 100644 --- a/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py +++ b/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py @@ -369,7 +369,7 @@ def reschedule_build_chroot(): return flask.jsonify(response) -@backend_ns.route("/chroots-prunerepo-status/", methods=["POST", "PUT"]) +@backend_ns.route("/chroots-prunerepo-status/") def chroots_prunerepo_status(): return flask.jsonify(MockChrootsLogic.chroots_prunerepo_status())