Merge lp:~james-w/requestsplus/first-stab into lp:requestsplus

[James Westby]

First stab at a requests helper library.

Hi,

This is my first attempt at a requests wrapper library, as described in Malta.

It's not a lot of code.

It has a great name.

Thanks,

James

[Natalia Bidart (nataliabidart)]

Overall looks good. I'm not sure the name is that great :-)

[James Westby (james-w)]

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

[Natalia Bidart (nataliabidart)]

review: Approve

=== added file '.bzrignore'
--- .bzrignore	1970-01-01 00:00:00 +0000
+++ .bzrignore	2014-10-22 22:01:51 +0000
@@ -0,0 +1,1 @@
+virtualenv

=== added file 'Makefile'
--- Makefile	1970-01-01 00:00:00 +0000
+++ Makefile	2014-10-22 22:01:51 +0000
@@ -0,0 +1,19 @@
+ENV=virtualenv
+BIN=$(ENV)/bin
+PIP=$(BIN)/pip
+PYTHON=$(BIN)/python
+
+$(ENV):
+	virtualenv $(ENV)
+
+deps: $(ENV)
+	$(PIP) install -r requirements.txt
+
+test:
+	$(PYTHON) -m testtools.run discover
+
+lint:
+	$(BIN)/pyflakes requestsplus
+	$(BIN)/pep8 requestsplus
+
+.PHONY: deps test

=== added file 'README'
--- README	1970-01-01 00:00:00 +0000
+++ README	2014-10-22 22:01:51 +0000
@@ -0,0 +1,35 @@
+requestsplus
+============
+
+To make full use of the features for an internal request
+made during a django request you would use:
+
+import logging
+from django_statsd.clients import statsd
+from timeline_django.wsgi import timeline_factory
+from requestsplus import (
+    get_request_id,
+    Session,
+)
+
+
+logger = logging.getLogger(__name__)
+
+
+def some_view(request):
+    request_id = get_request_id(request)
+    session = Session(
+        logger=logger,
+        request_id=request_id,
+        timeline_factory=timeline_factory,
+        statsd=statsd,
+        statsd_prefix='request.sso.validate_request',
+    )
+    response = session.get('https://...')
+    django_response = HttpResponse()
+    forward_oops_ids(response, django_response)
+
+
+If you are making an external request (i.e. to a service we
+don't control) then omit the request_id, and don't bother
+forwarding oops ids in the response.

=== added directory 'requestsplus'
=== added file 'requestsplus/__init__.py'
--- requestsplus/__init__.py	1970-01-01 00:00:00 +0000
+++ requestsplus/__init__.py	2014-10-22 22:01:51 +0000
@@ -0,0 +1,114 @@
+from contextlib import contextmanager
+
+from requests import Session as _Session
+
+
+OOPS_HEADER = 'X-Oops-Id'
+REQUEST_ID_HEADER = 'X-Request-Id'
+
+
+def get_request_id(request):
+    return request.META.get('HTTP_X_REQUEST_ID')
+
+
+def get_oops_ids(response):
+    return response.headers.get(OOPS_HEADER, None)
+
+
+def forward_oops_ids(response, django_response):
+    oops_ids = get_oops_ids(response)
+    if oops_ids is not None:
+        django_response[OOPS_HEADER] = oops_ids
+
+
+class Session(_Session):
+
+    def __init__(self, *args, **kwargs):
+        self.logger = kwargs.pop('logger', None)
+        self.request_id = kwargs.pop('request_id', None)
+        self.timeline_factory = kwargs.pop('timeline_factory', None)
+        self.statsd = kwargs.pop('statsd', None)
+        self.statsd_prefix = kwargs.pop('statsd_prefix', None)
+        super(Session, self).__init__(*args, **kwargs)
+
+    def request(self, method, url, **kwargs):
+        request_id = kwargs.pop('request_id', self.request_id)
+        kwargs.setdefault('headers', {})
+        kwargs['headers'][REQUEST_ID_HEADER] = request_id
+        with maybe_timeline(self.timeline_factory, method, url) as action:
+            resp = super(Session, self).request(method, url, **kwargs)
+            if action is not None:
+                action.detail += " {}".format(resp.status_code)
+        if self.logger:
+            log_response(self.logger, resp)
+        if self.statsd and self.statsd_prefix:
+            record_stats(self.statsd, self.statsd_prefix, resp)
+        return resp
+
+
+def record_stats(statsd, prefix, response):
+    duration = response.elapsed.total_seconds() * 1000
+    statsd.timing(prefix, duration)
+    statsd.timing(prefix + '.' + str(response.status_code), duration)
+    statsd.timing(prefix + '.' + response.request.method, duration)
+    statsd.timing(
+        prefix + '.' + response.request.method
+        + '.' + str(response.status_code), duration)
+
+
+def get_extra_details(response):
+    # TODO: Probably log If-* headers if present in request
+    extra = dict()
+
+    def get_response_header(name, fallback_to_request=False):
+        value = response.headers.get(name, None)
+        if fallback_to_request and not value:
+            value = response.request.headers.get(name, None)
+        if value:
+            extra[name] = value
+
+    response_headers = [
+        'Age',
+        'Allow',
+        'Content-Length',
+        'Content-Type',
+        'ETag',
+        'Last-Modified',
+        'Location',
+        OOPS_HEADER,
+    ]
+
+    map(get_response_header, response_headers)
+    get_response_header(REQUEST_ID_HEADER, fallback_to_request=True)
+
+    return extra
+
+
+def log_response(logger, response):
+    request = response.request
+    extra = get_extra_details(response)
+
+    logger.info(
+        "%(method)s %(url)s returned %(status_code)s "
+        "in %(duration)s %(extra)s",
+        dict(method=request.method,
+             url=request.url,
+             status_code=response.status_code,
+             duration=str(response.elapsed),
+             extra=["{}={}".format(key, value)
+                    for key, value in sorted(extra.items())],
+             )
+    )
+
+
+@contextmanager
+def maybe_timeline(factory, category, detail):
+    if factory is not None:
+        timeline = factory()
+        action = timeline.start(category, detail)
+        try:
+            yield action
+        finally:
+            action.finish()
+    else:
+        yield

=== added directory 'requestsplus/tests'
=== added file 'requestsplus/tests/__init__.py'
--- requestsplus/tests/__init__.py	1970-01-01 00:00:00 +0000
+++ requestsplus/tests/__init__.py	2014-10-22 22:01:51 +0000
@@ -0,0 +1,282 @@
+from datetime import timedelta
+import logging
+
+import fixtures
+from testtools import TestCase
+from responses import RequestsMock
+from requests import (
+    Request,
+    Response,
+)
+from timeline import Timeline
+
+from .. import (
+    forward_oops_ids,
+    get_extra_details,
+    get_oops_ids,
+    get_request_id,
+    record_stats,
+    Session,
+)
+
+
+class MockStatsd(object):
+
+    def __init__(self):
+        self.timings = {}
+
+    def timing(self, key, value):
+        self.timings[key] = value
+
+
+class RecordStatsTests(TestCase):
+
+    def get_response(self):
+        response = Response()
+        response.request = Request(
+            method="GET", url="http://localhost/").prepare()
+        response.elapsed = timedelta(seconds=2)
+        response.status_code = 400
+        return response
+
+    def test_records_global(self):
+        statsd = MockStatsd()
+        response = self.get_response()
+        prefix = 'a'
+        record_stats(statsd, prefix, response)
+        self.assertEqual(2000, statsd.timings[prefix])
+
+    def test_records_status_code(self):
+        statsd = MockStatsd()
+        response = self.get_response()
+        prefix = 'a'
+        record_stats(statsd, prefix, response)
+        key = prefix + '.' + str(response.status_code)
+        self.assertEqual(2000, statsd.timings[key])
+
+    def test_records_method(self):
+        statsd = MockStatsd()
+        response = self.get_response()
+        prefix = 'a'
+        record_stats(statsd, prefix, response)
+        key = prefix + '.' + response.request.method
+        self.assertEqual(2000, statsd.timings[key])
+
+    def test_records_method_and_status_code(self):
+        statsd = MockStatsd()
+        response = self.get_response()
+        prefix = 'a'
+        record_stats(statsd, prefix, response)
+        key = (prefix + '.' + response.request.method
+               + '.' + str(response.status_code))
+        self.assertEqual(2000, statsd.timings[key])
+
+
+class GetExtraDetailsTests(TestCase):
+
+    def get_response(self):
+        response = Response()
+        response.request = Request(
+            method="GET", url="http://localhost/").prepare()
+        return response
+
+    def test_empty(self):
+        response = self.get_response()
+        self.assertEqual({}, get_extra_details(response))
+
+    def assert_header_used(self, name, value, response=None):
+        if response is None:
+            response = self.get_response()
+        response.headers[name] = value
+        self.assertEqual({name: value}, get_extra_details(response))
+
+    def test_age_header(self):
+        self.assert_header_used('Age', '2')
+
+    def test_allow_header(self):
+        self.assert_header_used('Allow', 'POST')
+
+    def test_content_length_header(self):
+        self.assert_header_used('Content-Length', '26')
+
+    def test_content_type_header(self):
+        self.assert_header_used('Content-Type', 'text/plain')
+
+    def test_etag_header(self):
+        self.assert_header_used('ETag', '12345')
+
+    def test_last_modified_header(self):
+        self.assert_header_used('Last-Modified', 'yesterday')
+
+    def test_location_header(self):
+        self.assert_header_used('Location', 'here')
+
+    def test_oops_header(self):
+        self.assert_header_used('X-Oops-Id', 'OOPS-12345')
+
+    def test_request_id_header(self):
+        response = self.get_response()
+        response.request.headers['X-Request-Id'] = '98765'
+        self.assert_header_used('X-Request-Id', '12345', response=response)
+
+    def test_request_id_request_header_fallback(self):
+        response = self.get_response()
+        response.request.headers['X-Request-Id'] = '98765'
+        self.assertEqual(
+            {'X-Request-Id': '98765'}, get_extra_details(response))
+
+
+class GetOopsTests(TestCase):
+
+    def get_response(self):
+        response = Response()
+        response.request = Request(
+            method="GET", url="http://localhost/").prepare()
+        return response
+
+    def test_no_oops_header(self):
+        response = self.get_response()
+        self.assertEqual(None, get_oops_ids(response))
+
+    def test_none_oops_header(self):
+        response = self.get_response()
+        response.headers['X-Oops-Id'] = None
+        self.assertEqual(None, get_oops_ids(response))
+
+    def test_oops_header(self):
+        oopses = "['OOPS-12345', 'OOPS-67890']"
+        response = self.get_response()
+        response.headers['X-Oops-Id'] = oopses
+        self.assertEqual(oopses, get_oops_ids(response))
+
+
+class ForwardOopsIdsTests(TestCase):
+
+    def get_response(self):
+        response = Response()
+        response.request = Request(
+            method="GET", url="http://localhost/").prepare()
+        return response
+
+    def test_no_oops_ids(self):
+        response = self.get_response()
+        django_response = dict()
+        forward_oops_ids(response, django_response)
+        self.assertNotIn('X-Oops-Id', django_response)
+
+    def test_oops_ids(self):
+        response = self.get_response()
+        oops_ids = 'OOPS-1'
+        response.headers['X-Oops-Id'] = oops_ids
+        django_response = dict()
+        forward_oops_ids(response, django_response)
+        self.assertEqual(oops_ids, django_response['X-Oops-Id'])
+
+
+class FakeDjangoRequest(object):
+
+    def __init__(self):
+        self.META = dict()
+
+
+class GetRequestIdTests(TestCase):
+
+    def test_no_header(self):
+        request = FakeDjangoRequest()
+        self.assertEqual(None, get_request_id(request))
+
+    def test_header(self):
+        request = FakeDjangoRequest()
+        request_id = 1
+        request.META['HTTP_X_REQUEST_ID'] = request_id
+        self.assertEqual(request_id, get_request_id(request))
+
+
+class SessionTests(TestCase):
+
+    def test_sends_request_id_header(self):
+        s = Session(request_id='b')
+        request_id = 'a'
+        responses = RequestsMock()
+        responses.add('GET', 'http://localhost')
+        with responses:
+            s.get('http://localhost', request_id=request_id)
+            self.assertEqual(1, len(responses.calls))
+            self.assertEqual(
+                request_id,
+                responses.calls[0].request.headers['X-Request-Id'])
+
+    def test_sends_request_id_header_with_other_headers(self):
+        s = Session()
+        request_id = 'a'
+        responses = RequestsMock()
+        responses.add('GET', 'http://localhost')
+        with responses:
+            s.get('http://localhost', request_id=request_id,
+                  headers=dict(a='b'))
+            self.assertEqual(1, len(responses.calls))
+            self.assertEqual(
+                request_id,
+                responses.calls[0].request.headers['X-Request-Id'])
+            self.assertEqual(
+                'b', responses.calls[0].request.headers['a'])
+
+    def test_uses_per_session_request_id(self):
+        request_id = 'b'
+        s = Session(request_id=request_id)
+        responses = RequestsMock()
+        responses.add('GET', 'http://localhost')
+        with responses:
+            s.get('http://localhost')
+            self.assertEqual(1, len(responses.calls))
+            self.assertEqual(
+                request_id,
+                responses.calls[0].request.headers['X-Request-Id'])
+
+    def test_no_request_id(self):
+        s = Session()
+        responses = RequestsMock()
+        responses.add('GET', 'http://localhost')
+        with responses:
+            s.get('http://localhost')
+            self.assertEqual(1, len(responses.calls))
+            self.assertNotIn(
+                'X-Request-Id', responses.calls[0].request.headers)
+
+    def test_logs_request(self):
+        log_capture = self.useFixture(fixtures.FakeLogger())
+        logger = logging.getLogger(__name__)
+        s = Session(logger=logger)
+        responses = RequestsMock()
+        responses.add('GET', 'http://localhost')
+        with responses:
+            s.get('http://localhost')
+        self.assertEqual(
+            "GET http://localhost/ returned 200 "
+            "in 0:00:00 ['Content-Type=text/plain']\n",
+            log_capture.output)
+
+    def test_uses_timeline_factory(self):
+        timeline = Timeline()
+        timeline_factory = lambda: timeline
+        s = Session(timeline_factory=timeline_factory)
+        responses = RequestsMock()
+        url = 'http://localhost'
+        responses.add('GET', url)
+        with responses:
+            s.get(url)
+        self.assertEqual(1, len(timeline.actions))
+        action = timeline.actions[0]
+        self.assertIsInstance(action.duration, timedelta)
+        self.assertEqual('GET', action.category)
+        self.assertEqual(url + " 200", action.detail)
+
+    def test_records_stats(self):
+        statsd = MockStatsd()
+        s = Session(statsd=statsd, statsd_prefix='a')
+        responses = RequestsMock()
+        url = 'http://localhost'
+        responses.add('GET', url)
+        with responses:
+            s.get(url)
+        self.assertEqual(4, len(statsd.timings))

=== added file 'requirements.txt'
--- requirements.txt	1970-01-01 00:00:00 +0000
+++ requirements.txt	2014-10-22 22:01:51 +0000
@@ -0,0 +1,7 @@
+fixtures
+pyflakes
+pep8
+requests
+responses
+testtools
+timeline

=== added file 'tarmac_tests.sh'
--- tarmac_tests.sh	1970-01-01 00:00:00 +0000
+++ tarmac_tests.sh	2014-10-22 22:01:51 +0000
@@ -0,0 +1,7 @@
+#!/bin/bash
+
+set -e
+
+make deps
+make test
+make lint