Skip to content

Commit 7aa9b18

Browse files
dhermesdhermes
committed
Adding ability to send version info header on HTTP requests.
Added an "extra headers" feature to enable this. I am not a fan of changing `Connection()` so haphazardly, but I hope to completely re-factor / destory `Connection()` in the near-term so I am less worried. This only adds the storage and datastore header info, for the purposes of a simple review. Once we agree on the approach, I can add support in the other subpackages.
1 parent c79fc92 commit 7aa9b18

File tree

6 files changed

+142
-16
lines changed

6 files changed

+142
-16
lines changed

‎core/google/cloud/_http.py‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
import json
1818
from pkg_resources import get_distribution
19+
import platform
20+
1921
import six
2022
from six.moves.urllib.parse import urlencode
2123

@@ -29,6 +31,10 @@
2931
get_distribution('google-cloud-core').version)
3032
"""The user agent for google-cloud-python requests."""
3133

34+
CLIENT_INFO_HEADER = 'X-Goog-API-Client'
35+
CLIENT_INFO_TEMPLATE = (
36+
'gl-python/' + platform.python_version() + ' gccl/{}')
37+
3238

3339
class Connection(object):
3440
"""A generic connection to Google Cloud Platform.
@@ -38,6 +44,11 @@ class Connection(object):
3844
"""
3945

4046
USER_AGENT = DEFAULT_USER_AGENT
47+
_EXTRA_HEADERS = {}
48+
"""Headers to be sent with every request.
49+
50+
Intended to be over-ridden by subclasses.
51+
"""
4152

4253
def __init__(self, client):
4354
self._client = client
@@ -147,7 +158,9 @@ def _make_request(self, method, url, data=None, content_type=None,
147158
:param content_type: The proper MIME type of the data provided.
148159
149160
:type headers: dict
150-
:param headers: A dictionary of HTTP headers to send with the request.
161+
:param headers: (Optional) A dictionary of HTTP headers to send with
162+
the request. If passed, will be modified directly
163+
here with added headers.
151164
152165
:type target_object: object
153166
:param target_object:
@@ -161,6 +174,7 @@ def _make_request(self, method, url, data=None, content_type=None,
161174
returned by :meth:`_do_request`.
162175
"""
163176
headers = headers or {}
177+
headers.update(self._EXTRA_HEADERS)
164178
headers['Accept-Encoding'] = 'gzip'
165179

166180
if data:

‎core/unit_tests/test__http.py‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,44 @@ def test_api_request_w_headers(self):
287287
}
288288
self.assertEqual(http._called_with['headers'], expected_headers)
289289

290+
def test_api_request_w_extra_headers(self):
291+
from six.moves.urllib.parse import urlsplit
292+
293+
http = _Http(
294+
{'status': '200', 'content-type': 'application/json'},
295+
b'{}',
296+
)
297+
client = mock.Mock(_http=http, spec=['_http'])
298+
conn = self._make_mock_one(client)
299+
conn._EXTRA_HEADERS = {
300+
'X-Baz': 'dax-quux',
301+
'X-Foo': 'not-bar', # Collision with ``headers``.
302+
}
303+
self.assertEqual(
304+
conn.api_request('GET', '/', headers={'X-Foo': 'bar'}), {})
305+
self.assertEqual(http._called_with['method'], 'GET')
306+
uri = http._called_with['uri']
307+
scheme, netloc, path, qs, _ = urlsplit(uri)
308+
self.assertEqual('%s://%s' % (scheme, netloc), conn.API_BASE_URL)
309+
# Intended to emulate self.mock_template
310+
PATH = '/'.join([
311+
'',
312+
'mock',
313+
conn.API_VERSION,
314+
'',
315+
])
316+
self.assertEqual(path, PATH)
317+
self.assertEqual(qs, '')
318+
self.assertIsNone(http._called_with['body'])
319+
expected_headers = {
320+
'Accept-Encoding': 'gzip',
321+
'Content-Length': '0',
322+
'User-Agent': conn.USER_AGENT,
323+
'X-Foo': 'not-bar', # The one passed-in is overridden.
324+
'X-Baz': 'dax-quux',
325+
}
326+
self.assertEqual(http._called_with['headers'], expected_headers)
327+
290328
def test_api_request_w_data(self):
291329
import json
292330

‎datastore/google/cloud/datastore/_http.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import contextlib
1818
import os
19+
from pkg_resources import get_distribution
1920

2021
from google.rpc import status_pb2
2122

@@ -61,6 +62,9 @@
6162

6263
_DISABLE_GRPC = os.getenv(DISABLE_GRPC, False)
6364
_USE_GRPC = _HAVE_GRPC and not _DISABLE_GRPC
65+
_DATASTORE_DIST = get_distribution('google-cloud-datastore')
66+
_CLIENT_INFO = connection_module.CLIENT_INFO_TEMPLATE.format(
67+
_DATASTORE_DIST.version)
6468

6569

6670
class _DatastoreAPIOverHttp(object):
@@ -102,6 +106,7 @@ def _request(self, project, method, data):
102106
'Content-Type': 'application/x-protobuf',
103107
'Content-Length': str(len(data)),
104108
'User-Agent': self.connection.USER_AGENT,
109+
connection_module.CLIENT_INFO_HEADER: _CLIENT_INFO,
105110
}
106111
headers, content = self.connection.http.request(
107112
uri=self.connection.build_api_url(project=project, method=method),

‎datastore/unit_tests/test__http.py‎

Lines changed: 42 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ def _make_one(self, *args, **kw):
3131
return self._get_target_class()(*args, **kw)
3232

3333
def test__rpc(self):
34+
from google.cloud import _http as connection_module
35+
from google.cloud.datastore._http import _CLIENT_INFO
36+
3437
class ReqPB(object):
3538

3639
def SerializeToString(self):
@@ -56,17 +59,24 @@ def FromString(cls, pb):
5659
self.assertIsInstance(response, RspPB)
5760
self.assertEqual(response._pb, 'CONTENT')
5861
called_with = http._called_with
62+
self.assertEqual(len(called_with), 4)
5963
self.assertEqual(called_with['uri'], URI)
6064
self.assertEqual(called_with['method'], 'POST')
61-
self.assertEqual(called_with['headers']['Content-Type'],
62-
'application/x-protobuf')
63-
self.assertEqual(called_with['headers']['User-Agent'],
64-
conn.USER_AGENT)
65+
expected_headers = {
66+
'Content-Type': 'application/x-protobuf',
67+
'User-Agent': conn.USER_AGENT,
68+
'Content-Length': '5',
69+
connection_module.CLIENT_INFO_HEADER: _CLIENT_INFO,
70+
}
71+
self.assertEqual(called_with['headers'], expected_headers)
6572
self.assertEqual(called_with['body'], REQPB)
6673
self.assertEqual(conn.build_kwargs,
6774
[{'method': METHOD, 'project': PROJECT}])
6875

6976
def test__request_w_200(self):
77+
from google.cloud import _http as connection_module
78+
from google.cloud.datastore._http import _CLIENT_INFO
79+
7080
PROJECT = 'PROJECT'
7181
METHOD = 'METHOD'
7282
DATA = b'DATA'
@@ -77,12 +87,16 @@ def test__request_w_200(self):
7787
self.assertEqual(datastore_api._request(PROJECT, METHOD, DATA),
7888
'CONTENT')
7989
called_with = http._called_with
90+
self.assertEqual(len(called_with), 4)
8091
self.assertEqual(called_with['uri'], URI)
8192
self.assertEqual(called_with['method'], 'POST')
82-
self.assertEqual(called_with['headers']['Content-Type'],
83-
'application/x-protobuf')
84-
self.assertEqual(called_with['headers']['User-Agent'],
85-
conn.USER_AGENT)
93+
expected_headers = {
94+
'Content-Type': 'application/x-protobuf',
95+
'User-Agent': conn.USER_AGENT,
96+
'Content-Length': '4',
97+
connection_module.CLIENT_INFO_HEADER: _CLIENT_INFO,
98+
}
99+
self.assertEqual(called_with['headers'], expected_headers)
86100
self.assertEqual(called_with['body'], DATA)
87101
self.assertEqual(conn.build_kwargs,
88102
[{'method': METHOD, 'project': PROJECT}])
@@ -386,12 +400,18 @@ def _make_one(self, client, use_grpc=False):
386400
return self._get_target_class()(client)
387401

388402
def _verifyProtobufCall(self, called_with, URI, conn):
403+
from google.cloud import _http as connection_module
404+
from google.cloud.datastore._http import _CLIENT_INFO
405+
389406
self.assertEqual(called_with['uri'], URI)
390407
self.assertEqual(called_with['method'], 'POST')
391-
self.assertEqual(called_with['headers']['Content-Type'],
392-
'application/x-protobuf')
393-
self.assertEqual(called_with['headers']['User-Agent'],
394-
conn.USER_AGENT)
408+
expected_headers = {
409+
'Content-Type': 'application/x-protobuf',
410+
'User-Agent': conn.USER_AGENT,
411+
'Content-Length': str(len(called_with['body'])),
412+
connection_module.CLIENT_INFO_HEADER: _CLIENT_INFO,
413+
}
414+
self.assertEqual(called_with['headers'], expected_headers)
395415

396416
def test_default_url(self):
397417
klass = self._get_target_class()
@@ -681,6 +701,9 @@ def test_lookup_multiple_keys_w_missing(self):
681701
def test_lookup_multiple_keys_w_deferred(self):
682702
from google.cloud.grpc.datastore.v1 import datastore_pb2
683703

704+
from google.cloud import _http as connection_module
705+
from google.cloud.datastore._http import _CLIENT_INFO
706+
684707
PROJECT = 'PROJECT'
685708
key_pb1 = self._make_key_pb(PROJECT)
686709
key_pb2 = self._make_key_pb(PROJECT, id_=2345)
@@ -704,9 +727,13 @@ def test_lookup_multiple_keys_w_deferred(self):
704727
self._verifyProtobufCall(cw, URI, conn)
705728
self.assertEqual(cw['uri'], URI)
706729
self.assertEqual(cw['method'], 'POST')
707-
self.assertEqual(cw['headers']['Content-Type'],
708-
'application/x-protobuf')
709-
self.assertEqual(cw['headers']['User-Agent'], conn.USER_AGENT)
730+
expected_headers = {
731+
'Content-Type': 'application/x-protobuf',
732+
'User-Agent': conn.USER_AGENT,
733+
'Content-Length': '48',
734+
connection_module.CLIENT_INFO_HEADER: _CLIENT_INFO,
735+
}
736+
self.assertEqual(cw['headers'], expected_headers)
710737
rq_class = datastore_pb2.LookupRequest
711738
request = rq_class()
712739
request.ParseFromString(cw['body'])

‎storage/google/cloud/storage/_http.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,15 @@
1414

1515
"""Create / interact with Google Cloud Storage connections."""
1616

17+
from pkg_resources import get_distribution
18+
1719
from google.cloud import _http
1820

1921

22+
_STORAGE_DIST = get_distribution('google-cloud-storage')
23+
_CLIENT_INFO = _http.CLIENT_INFO_TEMPLATE.format(_STORAGE_DIST.version)
24+
25+
2026
class Connection(_http.JSONConnection):
2127
"""A connection to Google Cloud Storage via the JSON REST API.
2228
@@ -32,3 +38,7 @@ class Connection(_http.JSONConnection):
3238

3339
API_URL_TEMPLATE = '{api_base_url}/storage/{api_version}{path}'
3440
"""A template for the URL of a particular API call."""
41+
42+
_EXTRA_HEADERS = {
43+
_http.CLIENT_INFO_HEADER: _CLIENT_INFO,
44+
}

‎storage/unit_tests/test__http.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
import unittest
1616

17+
import mock
18+
1719

1820
class TestConnection(unittest.TestCase):
1921

@@ -26,6 +28,36 @@ def _get_target_class():
2628
def _make_one(self, *args, **kw):
2729
return self._get_target_class()(*args, **kw)
2830

31+
def test_extra_headers(self):
32+
from google.cloud import _http as base_http
33+
from google.cloud.storage import _http as MUT
34+
35+
http = mock.Mock(spec=['request'])
36+
response = mock.Mock(status=200, spec=['status'])
37+
data = b'brent-spiner'
38+
http.request.return_value = response, data
39+
client = mock.Mock(_http=http, spec=['_http'])
40+
41+
conn = self._make_one(client)
42+
req_data = 'hey-yoooouuuuu-guuuuuyyssss'
43+
result = conn.api_request(
44+
'GET', '/rainbow', data=req_data, expect_json=False)
45+
self.assertEqual(result, data)
46+
47+
expected_headers = {
48+
'Content-Length': str(len(req_data)),
49+
'Accept-Encoding': 'gzip',
50+
base_http.CLIENT_INFO_HEADER: MUT._CLIENT_INFO,
51+
'User-Agent': conn.USER_AGENT,
52+
}
53+
expected_uri = conn.build_api_url('/rainbow')
54+
http.request.assert_called_once_with(
55+
body=req_data,
56+
headers=expected_headers,
57+
method='GET',
58+
uri=expected_uri,
59+
)
60+
2961
def test_build_api_url_no_extra_query_params(self):
3062
conn = self._make_one(object())
3163
URI = '/'.join([

0 commit comments

Comments
 (0)