From 90a4208390f22e797619ac33dc099b985e367cbd Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Fri, 22 Nov 2019 15:31:55 +0100 Subject: [PATCH] Get rid from requests module (#1296) It wasn't used for anything critical anyway, so it doesn't make a lot of sense to keep it as an explicit dependency. --- features/steps/patroni_api.py | 36 ++++++++++++----------------------- patroni/config.py | 3 +-- patroni/dcs/etcd.py | 11 +++++------ patroni/dcs/exhibitor.py | 12 ++++++------ patroni/postgresql/config.py | 27 +++++++++++++++++++++++++- patroni/request.py | 15 ++++++++++++--- patroni/scripts/aws.py | 12 ++++++------ requirements.txt | 1 - tests/__init__.py | 15 ++------------- tests/test_aws.py | 30 +++++++---------------------- tests/test_etcd.py | 6 +++--- tests/test_exhibitor.py | 6 ++++-- 12 files changed, 84 insertions(+), 90 deletions(-) diff --git a/features/steps/patroni_api.py b/features/steps/patroni_api.py index bc40eae1..c95878a7 100644 --- a/features/steps/patroni_api.py +++ b/features/steps/patroni_api.py @@ -1,8 +1,6 @@ -import base64 import json import os import parse -import requests import shlex import subprocess import sys @@ -12,8 +10,10 @@ import yaml from behave import register_type, step, then from dateutil import tz from datetime import datetime, timedelta +from patroni.request import PatroniRequest tzutc = tz.tzutc() +request_executor = PatroniRequest({'ctl': {'auth': 'username:password'}}) @parse.with_pattern(r'https?://(?:\w|\.|:|/)+') @@ -45,9 +45,9 @@ def sleep_for_n_seconds(context, value): def _set_response(context, response): - context.status_code = response.status_code - data = response.content.decode('utf-8') - ct = response.headers.get('content-type', '') + context.status_code = response.status + data = response.data.decode('utf-8') + ct = response.getheader('content-type', '') if ct.startswith('application/json') or\ ct.startswith('text/yaml') or\ ct.startswith('text/x-yaml') or\ @@ -63,13 +63,7 @@ def _set_response(context, response): @step('I issue a GET request to {url:url}') def do_get(context, url): - try: - r = requests.get(url) - except requests.exceptions.RequestException: - context.status_code = None - context.response = None - else: - _set_response(context, r) + do_request(context, 'GET', url, None) @step('I issue an empty POST request to {url:url}') @@ -79,17 +73,11 @@ def do_post_empty(context, url): @step('I issue a {request_method:w} request to {url:url} with {data}') def do_request(context, request_method, url, data): - data = data and json.loads(data) or {} - headers = {'Authorization': 'Basic ' + base64.b64encode('username:password'.encode('utf-8')).decode('utf-8'), - 'Content-Type': 'application/json'} + data = data and json.loads(data) try: - if request_method == 'PATCH': - r = requests.patch(url, headers=headers, json=data) - else: - r = requests.post(url, headers=headers, json=data) - except requests.exceptions.RequestException: - context.status_code = None - context.response = None + r = request_executor.request(request_method, url, data) + except Exception: + context.status_code = context.response = None else: _set_response(context, r) @@ -149,8 +137,8 @@ def add_tag_to_config(context, tag, value, pg_name): def check_http_response(context, url, value, timeout, negate=False): timeout *= context.timeout_multiplier for _ in range(int(timeout)): - r = requests.get(url) - if (value in r.content.decode('utf-8')) != negate: + r = request_executor.request('GET', url) + if (value in r.data.decode('utf-8')) != negate: break time.sleep(1) else: diff --git a/patroni/config.py b/patroni/config.py index ea4a6d7e..3f75fda7 100644 --- a/patroni/config.py +++ b/patroni/config.py @@ -9,9 +9,8 @@ import yaml from collections import defaultdict from copy import deepcopy from patroni.dcs import ClusterConfig -from patroni.postgresql.config import ConfigHandler +from patroni.postgresql.config import CaseInsensitiveDict, ConfigHandler from patroni.utils import deep_compare, parse_bool, parse_int, patch_config -from requests.structures import CaseInsensitiveDict logger = logging.getLogger(__name__) diff --git a/patroni/dcs/etcd.py b/patroni/dcs/etcd.py index e63c8aaf..52c95dcb 100644 --- a/patroni/dcs/etcd.py +++ b/patroni/dcs/etcd.py @@ -5,7 +5,6 @@ import logging import os import urllib3.util.connection import random -import requests import six import socket import time @@ -15,8 +14,8 @@ from dns import resolver from patroni.dcs import AbstractDCS, ClusterConfig, Cluster, Failover, Leader, Member, SyncState, TimelineHistory from patroni.exceptions import DCSError from patroni.utils import Retry, RetryFailedError, split_host_port, uri +from patroni.request import get as requests_get from urllib3.exceptions import HTTPError, ReadTimeoutError, ProtocolError -from requests.exceptions import RequestException from six.moves.queue import Queue from six.moves.http_client import HTTPException from six.moves.urllib_parse import urlparse @@ -284,12 +283,12 @@ class Client(etcd.Client): url = uri(protocol, (host, port), endpoint) if endpoint: try: - response = requests.get(url, timeout=self.read_timeout, verify=False) - if response.ok: - for member in response.json(): + response = requests_get(url, timeout=self.read_timeout, verify=False) + if response.status < 400: + for member in json.loads(response.data.decode('utf-8')): ret.extend(member['clientURLs']) break - except RequestException: + except Exception: logger.exception('GET %s', url) else: ret.append(url) diff --git a/patroni/dcs/exhibitor.py b/patroni/dcs/exhibitor.py index 16568daf..70066d65 100644 --- a/patroni/dcs/exhibitor.py +++ b/patroni/dcs/exhibitor.py @@ -1,11 +1,11 @@ +import json import logging import random -import requests import time from patroni.dcs.zookeeper import ZooKeeper +from patroni.request import get as requests_get from patroni.utils import uri -from requests.exceptions import RequestException logger = logging.getLogger(__name__) @@ -48,10 +48,10 @@ class ExhibitorEnsembleProvider(object): random.shuffle(exhibitors) for host in exhibitors: try: - response = requests.get(uri('http', (host, self._exhibitor_port), self._uri_path), timeout=self.TIMEOUT) - return response.json() - except RequestException: - pass + response = requests_get(uri('http', (host, self._exhibitor_port), self._uri_path), timeout=self.TIMEOUT) + return json.loads(response.data.decode('utf-8')) + except Exception: + logging.debug('Request to %s failed', host) return None @property diff --git a/patroni/postgresql/config.py b/patroni/postgresql/config.py index c73f3bb1..473b0ca3 100644 --- a/patroni/postgresql/config.py +++ b/patroni/postgresql/config.py @@ -6,7 +6,7 @@ import socket import stat import time -from requests.structures import CaseInsensitiveDict +from collections import MutableMapping, OrderedDict from six.moves.urllib_parse import urlparse, parse_qsl, unquote from ..dcs import slot_name_from_member_name, RemoteMember @@ -250,6 +250,31 @@ class ConfigWriter(object): self.writeline("{0} = '{1}'".format(param, self.escape(value))) +class CaseInsensitiveDict(MutableMapping): + + def __init__(self, data): + self._store = OrderedDict() + self.update(data) + + def __setitem__(self, key, value): + self._store[key.lower()] = (key, value) + + def __getitem__(self, key): + return self._store[key.lower()][1] + + def __delitem__(self, key): + del self._store[key.lower()] + + def __iter__(self): + return (casedkey for casedkey, mappedvalue in self._store.values()) + + def __len__(self): + return len(self._store) + + def copy(self): + return CaseInsensitiveDict(self._store.values()) + + class ConfigHandler(object): # List of parameters which must be always passed to postmaster as command line options diff --git a/patroni/request.py b/patroni/request.py index 3276e2e3..f16b7089 100644 --- a/patroni/request.py +++ b/patroni/request.py @@ -1,5 +1,6 @@ import json import urllib3 +import six from six.moves.urllib_parse import urlparse, urlunparse @@ -37,11 +38,19 @@ class PatroniRequest(object): cacert = config.get('ctl', {}).get('cacert') or config.get('restapi', {}).get('cafile') self._apply_pool_param('ca_certs', cacert) + def request(self, method, url, body=None, **kwargs): + if body is not None and not isinstance(body, six.string_types): + body = json.dumps(body) + return self._pool.request(method.upper(), url, body=body, **kwargs) + def __call__(self, member, method='GET', endpoint=None, data=None, **kwargs): url = member.api_url if endpoint: scheme, netloc, _, _, _, _ = urlparse(url) url = urlunparse((scheme, netloc, endpoint, '', '', '')) - if data is not None: - kwargs['body'] = json.dumps(data) - return self._pool.request(method.upper(), url, **kwargs) + return self.request(method, url, data, **kwargs) + + +def get(url, verify=True, **kwargs): + http = PatroniRequest({}, not verify) + return http.request('GET', url, **kwargs) diff --git a/patroni/scripts/aws.py b/patroni/scripts/aws.py index ae2b4d09..ee7e0412 100755 --- a/patroni/scripts/aws.py +++ b/patroni/scripts/aws.py @@ -1,12 +1,12 @@ #!/usr/bin/env python +import json import logging -import requests -from requests.exceptions import RequestException import sys import boto.ec2 from patroni.utils import Retry, RetryFailedError +from patroni.request import get as requests_get logger = logging.getLogger(__name__) @@ -19,14 +19,14 @@ class AWSConnection(object): self._retry = Retry(deadline=300, max_delay=30, max_tries=-1, retry_exceptions=(boto.exception.StandardError,)) try: # get the instance id - r = requests.get('http://169.254.169.254/latest/dynamic/instance-identity/document', timeout=2.1) - except RequestException: + r = requests_get('http://169.254.169.254/latest/dynamic/instance-identity/document', timeout=2.1) + except Exception: logger.error('cannot query AWS meta-data') return - if r.ok: + if r.status < 400: try: - content = r.json() + content = json.loads(r.data.decode('utf-8')) self.instance_id = content['instanceId'] self.region = content['region'] except Exception: diff --git a/requirements.txt b/requirements.txt index 02f97f64..9ab94465 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,6 @@ urllib3>=1.19.1,!=1.21 boto PyYAML -requests six >= 1.7 kazoo>=1.3.1 python-etcd>=0.4.3,<0.5 diff --git a/tests/__init__.py b/tests/__init__.py index 319bc0ee..84d2b772 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1,5 +1,4 @@ import datetime -import json import os import shutil import unittest @@ -7,7 +6,7 @@ import unittest from mock import Mock, patch import psycopg2 -import requests +import urllib3 from patroni.dcs import Leader, Member from patroni.postgresql import Postgresql @@ -24,19 +23,11 @@ class MockResponse(object): def __init__(self, status_code=200): self.status_code = status_code self.content = '{}' - self.ok = True - - def json(self): - return json.loads(self.content) @property def data(self): return self.content.encode('utf-8') - @property - def text(self): - return self.content - @property def status(self): return self.status_code @@ -51,7 +42,7 @@ def requests_get(url, **kwargs): '"name":"default","clientURLs":["http://localhost:2379","http://localhost:4001"]}]' response = MockResponse() if url.startswith('http://local'): - raise requests.exceptions.RequestException() + raise urllib3.exceptions.HTTPError() elif ':8011/patroni' in url: response.content = '{"role": "replica", "xlog": {"received_location": 0}, "tags": {}}' elif url.endswith('/members'): @@ -62,11 +53,9 @@ def requests_get(url, **kwargs): data = kwargs.get('data', '') if ' false}' in data: response.status_code = 503 - response.ok = False response.content = 'restarting after failure already in progress' else: response.status_code = 404 - response.ok = False return response diff --git a/tests/test_aws.py b/tests/test_aws.py index dc297f08..63b6fa9b 100644 --- a/tests/test_aws.py +++ b/tests/test_aws.py @@ -1,11 +1,11 @@ import boto.ec2 import sys import unittest +import urllib3 from mock import Mock, patch from collections import namedtuple from patroni.scripts.aws import AWSConnection, main as _main -from requests.exceptions import RequestException class MockEc2Connection(object): @@ -22,28 +22,11 @@ class MockEc2Connection(object): return True -class MockResponse(object): - ok = True - - def __init__(self, content): - self.content = content - - def json(self): - return self.content - - -def requests_get(url, **kwargs): - if url.split('/')[-1] == 'document': - result = {"instanceId": "012345", "region": "eu-west-1"} - else: - result = 'foo' - return MockResponse(result) - - @patch('boto.ec2.connect_to_region', Mock(return_value=MockEc2Connection())) class TestAWSConnection(unittest.TestCase): - @patch('requests.get', requests_get) + @patch('patroni.scripts.aws.requests_get', Mock(return_value=urllib3.HTTPResponse( + status=200, body=b'{"instanceId": "012345", "region": "eu-west-1"}'))) def setUp(self): self.conn = AWSConnection('test') @@ -53,17 +36,18 @@ class TestAWSConnection(unittest.TestCase): self.conn._retry.max_tries = 1 self.assertFalse(self.conn.on_role_change('master')) - @patch('requests.get', Mock(side_effect=RequestException('foo'))) + @patch('patroni.scripts.aws.requests_get', Mock(side_effect=Exception('foo'))) def test_non_aws(self): conn = AWSConnection('test') self.assertFalse(conn.on_role_change("master")) - @patch('requests.get', Mock(return_value=MockResponse('foo'))) + @patch('patroni.scripts.aws.requests_get', Mock(return_value=urllib3.HTTPResponse(status=200, body=b'foo'))) def test_aws_bizare_response(self): conn = AWSConnection('test') self.assertFalse(conn.aws_available()) - @patch('requests.get', requests_get) + @patch('patroni.scripts.aws.requests_get', Mock(return_value=urllib3.HTTPResponse( + status=200, body=b'{"instanceId": "012345", "region": "eu-west-1"}'))) @patch('sys.exit', Mock()) def test_main(self): self.assertIsNone(_main()) diff --git a/tests/test_etcd.py b/tests/test_etcd.py index fcd1d22b..8432fad9 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -116,12 +116,12 @@ class TestDnsCachingResolver(unittest.TestCase): @patch('dns.resolver.query', dns_query) @patch('socket.getaddrinfo', socket_getaddrinfo) -@patch('requests.get', requests_get) +@patch('patroni.dcs.etcd.requests_get', requests_get) class TestClient(unittest.TestCase): @patch('dns.resolver.query', dns_query) @patch('socket.getaddrinfo', socket_getaddrinfo) - @patch('requests.get', requests_get) + @patch('patroni.dcs.etcd.requests_get', requests_get) def setUp(self): with patch.object(Client, 'machines') as mock_machines: mock_machines.__get__ = Mock(return_value=['http://localhost:2379', 'http://localhost:4001']) @@ -199,7 +199,7 @@ class TestClient(unittest.TestCase): socket_options=[(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)]) -@patch('requests.get', requests_get) +@patch('patroni.dcs.etcd.requests_get', requests_get) @patch('socket.getaddrinfo', socket_getaddrinfo) @patch.object(etcd.Client, 'write', etcd_write) @patch.object(etcd.Client, 'read', etcd_read) diff --git a/tests/test_exhibitor.py b/tests/test_exhibitor.py index b0d51196..9d4d460d 100644 --- a/tests/test_exhibitor.py +++ b/tests/test_exhibitor.py @@ -1,4 +1,5 @@ import unittest +import urllib3 from mock import Mock, patch from patroni.dcs.exhibitor import ExhibitorEnsembleProvider, Exhibitor @@ -8,7 +9,7 @@ from . import SleepException, requests_get from .test_zookeeper import MockKazooClient -@patch('requests.get', requests_get) +@patch('patroni.dcs.exhibitor.requests_get', requests_get) @patch('time.sleep', Mock(side_effect=SleepException)) class TestExhibitorEnsembleProvider(unittest.TestCase): @@ -21,7 +22,8 @@ class TestExhibitorEnsembleProvider(unittest.TestCase): class TestExhibitor(unittest.TestCase): - @patch('requests.get', requests_get) + @patch('urllib3.PoolManager.request', Mock(return_value=urllib3.HTTPResponse( + status=200, body=b'{"servers":["127.0.0.1","127.0.0.2","127.0.0.3"],"port":2181}'))) @patch('patroni.dcs.zookeeper.KazooClient', MockKazooClient) def setUp(self): self.e = Exhibitor({'hosts': ['localhost', 'exhibitor'], 'port': 8181, 'scope': 'test',