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.

Author: dhermes

core/google/cloud/_http.py

@@ -15,7 +15,9 @@
 """Shared implementation of connections to API servers."""
 
 import json
+import platform
 from pkg_resources import get_distribution
+
 import six
 from six.moves.urllib.parse import urlencode
 
@@ -29,6 +31,10 @@
     get_distribution('google-cloud-core').version)
 """The user agent for google-cloud-python requests."""
 
+CLIENT_INFO_HEADER = 'X-Goog-API-Client'
+CLIENT_INFO_TEMPLATE = (
+    'gl-python/' + platform.python_version() + ' gccl/{}')
+
 
 class Connection(object):
     """A generic connection to Google Cloud Platform.
@@ -38,6 +44,11 @@ class Connection(object):
     """
 
     USER_AGENT = DEFAULT_USER_AGENT
+    _EXTRA_HEADERS = {}
+    """Headers to be sent with every request.
+
+    Intended to be over-ridden by subclasses.
+    """
 
     def __init__(self, client):
         self._client = client
@@ -147,7 +158,9 @@ def _make_request(self, method, url, data=None, content_type=None,
         :param content_type: The proper MIME type of the data provided.
 
         :type headers: dict
-        :param headers: A dictionary of HTTP headers to send with the request.
+        :param headers: (Optional) A dictionary of HTTP headers to send with
+                        the request. If passed, will be modified directly
+                        here with added headers.
 
         :type target_object: object
         :param target_object:
@@ -161,6 +174,7 @@ def _make_request(self, method, url, data=None, content_type=None,
                   returned by :meth:`_do_request`.
         """
         headers = headers or {}
+        headers.update(self._EXTRA_HEADERS)
         headers['Accept-Encoding'] = 'gzip'
 
         if data:

core/unit_tests/test__http.py

@@ -287,6 +287,44 @@ def test_api_request_w_headers(self):
         }
         self.assertEqual(http._called_with['headers'], expected_headers)
 
+    def test_api_request_w_extra_headers(self):
+        from six.moves.urllib.parse import urlsplit
+
+        http = _Http(
+            {'status': '200', 'content-type': 'application/json'},
+            b'{}',
+        )
+        client = mock.Mock(_http=http, spec=['_http'])
+        conn = self._make_mock_one(client)
+        conn._EXTRA_HEADERS = {
+            'X-Baz': 'dax-quux',
+            'X-Foo': 'not-bar',  # Collision with ``headers``.
+        }
+        self.assertEqual(
+            conn.api_request('GET', '/', headers={'X-Foo': 'bar'}), {})
+        self.assertEqual(http._called_with['method'], 'GET')
+        uri = http._called_with['uri']
+        scheme, netloc, path, qs, _ = urlsplit(uri)
+        self.assertEqual('%s://%s' % (scheme, netloc), conn.API_BASE_URL)
+        # Intended to emulate self.mock_template
+        PATH = '/'.join([
+            '',
+            'mock',
+            conn.API_VERSION,
+            '',
+        ])
+        self.assertEqual(path, PATH)
+        self.assertEqual(qs, '')
+        self.assertIsNone(http._called_with['body'])
+        expected_headers = {
+            'Accept-Encoding': 'gzip',
+            'Content-Length': '0',
+            'User-Agent': conn.USER_AGENT,
+            'X-Foo': 'not-bar',  # The one passed-in is overridden.
+            'X-Baz': 'dax-quux',
+        }
+        self.assertEqual(http._called_with['headers'], expected_headers)
+
     def test_api_request_w_data(self):
         import json
 

datastore/google/cloud/datastore/_http.py

@@ -16,6 +16,7 @@
 
 import contextlib
 import os
+from pkg_resources import get_distribution
 
 from google.rpc import status_pb2
 
@@ -61,6 +62,9 @@
 
 _DISABLE_GRPC = os.getenv(DISABLE_GRPC, False)
 _USE_GRPC = _HAVE_GRPC and not _DISABLE_GRPC
+_DATASTORE_DIST = get_distribution('google-cloud-datastore')
+_CLIENT_INFO = connection_module.CLIENT_INFO_TEMPLATE.format(
+    _DATASTORE_DIST.version)
 
 
 class _DatastoreAPIOverHttp(object):
@@ -102,6 +106,7 @@ def _request(self, project, method, data):
             'Content-Type': 'application/x-protobuf',
             'Content-Length': str(len(data)),
             'User-Agent': self.connection.USER_AGENT,
+            connection_module.CLIENT_INFO_HEADER: _CLIENT_INFO,
         }
         headers, content = self.connection.http.request(
             uri=self.connection.build_api_url(project=project, method=method),

datastore/unit_tests/test__http.py

@@ -31,6 +31,9 @@ def _make_one(self, *args, **kw):
         return self._get_target_class()(*args, **kw)
 
     def test__rpc(self):
+        from google.cloud import _http as connection_module
+        from google.cloud.datastore._http import _CLIENT_INFO
+
         class ReqPB(object):
 
             def SerializeToString(self):
@@ -56,17 +59,24 @@ def FromString(cls, pb):
         self.assertIsInstance(response, RspPB)
         self.assertEqual(response._pb, 'CONTENT')
         called_with = http._called_with
+        self.assertEqual(len(called_with), 4)
         self.assertEqual(called_with['uri'], URI)
         self.assertEqual(called_with['method'], 'POST')
-        self.assertEqual(called_with['headers']['Content-Type'],
-                         'application/x-protobuf')
-        self.assertEqual(called_with['headers']['User-Agent'],
-                         conn.USER_AGENT)
+        expected_headers = {
+            'Content-Type': 'application/x-protobuf',
+            'User-Agent': conn.USER_AGENT,
+            'Content-Length': '5',
+            connection_module.CLIENT_INFO_HEADER: _CLIENT_INFO,
+        }
+        self.assertEqual(called_with['headers'], expected_headers)
         self.assertEqual(called_with['body'], REQPB)
         self.assertEqual(conn.build_kwargs,
                          [{'method': METHOD, 'project': PROJECT}])
 
     def test__request_w_200(self):
+        from google.cloud import _http as connection_module
+        from google.cloud.datastore._http import _CLIENT_INFO
+
         PROJECT = 'PROJECT'
         METHOD = 'METHOD'
         DATA = b'DATA'
@@ -77,12 +87,16 @@ def test__request_w_200(self):
         self.assertEqual(datastore_api._request(PROJECT, METHOD, DATA),
                          'CONTENT')
         called_with = http._called_with
+        self.assertEqual(len(called_with), 4)
         self.assertEqual(called_with['uri'], URI)
         self.assertEqual(called_with['method'], 'POST')
-        self.assertEqual(called_with['headers']['Content-Type'],
-                         'application/x-protobuf')
-        self.assertEqual(called_with['headers']['User-Agent'],
-                         conn.USER_AGENT)
+        expected_headers = {
+            'Content-Type': 'application/x-protobuf',
+            'User-Agent': conn.USER_AGENT,
+            'Content-Length': '4',
+            connection_module.CLIENT_INFO_HEADER: _CLIENT_INFO,
+        }
+        self.assertEqual(called_with['headers'], expected_headers)
         self.assertEqual(called_with['body'], DATA)
         self.assertEqual(conn.build_kwargs,
                          [{'method': METHOD, 'project': PROJECT}])
@@ -386,12 +400,18 @@ def _make_one(self, client, use_grpc=False):
             return self._get_target_class()(client)
 
     def _verifyProtobufCall(self, called_with, URI, conn):
+        from google.cloud import _http as connection_module
+        from google.cloud.datastore._http import _CLIENT_INFO
+
         self.assertEqual(called_with['uri'], URI)
         self.assertEqual(called_with['method'], 'POST')
-        self.assertEqual(called_with['headers']['Content-Type'],
-                         'application/x-protobuf')
-        self.assertEqual(called_with['headers']['User-Agent'],
-                         conn.USER_AGENT)
+        expected_headers = {
+            'Content-Type': 'application/x-protobuf',
+            'User-Agent': conn.USER_AGENT,
+            'Content-Length': str(len(called_with['body'])),
+            connection_module.CLIENT_INFO_HEADER: _CLIENT_INFO,
+        }
+        self.assertEqual(called_with['headers'], expected_headers)
 
     def test_default_url(self):
         klass = self._get_target_class()
@@ -681,6 +701,9 @@ def test_lookup_multiple_keys_w_missing(self):
     def test_lookup_multiple_keys_w_deferred(self):
         from google.cloud.grpc.datastore.v1 import datastore_pb2
 
+        from google.cloud import _http as connection_module
+        from google.cloud.datastore._http import _CLIENT_INFO
+
         PROJECT = 'PROJECT'
         key_pb1 = self._make_key_pb(PROJECT)
         key_pb2 = self._make_key_pb(PROJECT, id_=2345)
@@ -704,9 +727,13 @@ def test_lookup_multiple_keys_w_deferred(self):
         self._verifyProtobufCall(cw, URI, conn)
         self.assertEqual(cw['uri'], URI)
         self.assertEqual(cw['method'], 'POST')
-        self.assertEqual(cw['headers']['Content-Type'],
-                         'application/x-protobuf')
-        self.assertEqual(cw['headers']['User-Agent'], conn.USER_AGENT)
+        expected_headers = {
+            'Content-Type': 'application/x-protobuf',
+            'User-Agent': conn.USER_AGENT,
+            'Content-Length': '48',
+            connection_module.CLIENT_INFO_HEADER: _CLIENT_INFO,
+        }
+        self.assertEqual(cw['headers'], expected_headers)
         rq_class = datastore_pb2.LookupRequest
         request = rq_class()
         request.ParseFromString(cw['body'])

storage/google/cloud/storage/_http.py

@@ -14,9 +14,15 @@
 
 """Create / interact with Google Cloud Storage connections."""
 
+from pkg_resources import get_distribution
+
 from google.cloud import _http
 
 
+_STORAGE_DIST = get_distribution('google-cloud-storage')
+_CLIENT_INFO = _http.CLIENT_INFO_TEMPLATE.format(_STORAGE_DIST.version)
+
+
 class Connection(_http.JSONConnection):
     """A connection to Google Cloud Storage via the JSON REST API.
 
@@ -32,3 +38,7 @@ class Connection(_http.JSONConnection):
 
     API_URL_TEMPLATE = '{api_base_url}/storage/{api_version}{path}'
     """A template for the URL of a particular API call."""
+
+    _EXTRA_HEADERS = {
+        _http.CLIENT_INFO_HEADER: _CLIENT_INFO,
+    }

storage/unit_tests/test__http.py

@@ -14,6 +14,8 @@
 
 import unittest
 
+import mock
+
 
 class TestConnection(unittest.TestCase):
 
@@ -26,6 +28,36 @@ def _get_target_class():
     def _make_one(self, *args, **kw):
         return self._get_target_class()(*args, **kw)
 
+    def test_extra_headers(self):
+        from google.cloud import _http as base_http
+        from google.cloud.storage import _http as MUT
+
+        http = mock.Mock(spec=['request'])
+        response = mock.Mock(status=200, spec=['status'])
+        data = b'brent-spiner'
+        http.request.return_value = response, data
+        client = mock.Mock(_http=http, spec=['_http'])
+
+        conn = self._make_one(client)
+        req_data = 'hey-yoooouuuuu-guuuuuyyssss'
+        result = conn.api_request(
+            'GET', '/rainbow', data=req_data, expect_json=False)
+        self.assertEqual(result, data)
+
+        expected_headers = {
+            'Content-Length': str(len(req_data)),
+            'Accept-Encoding': 'gzip',
+            base_http.CLIENT_INFO_HEADER: MUT._CLIENT_INFO,
+            'User-Agent': conn.USER_AGENT,
+        }
+        expected_uri = conn.build_api_url('/rainbow')
+        http.request.assert_called_once_with(
+            body=req_data,
+            headers=expected_headers,
+            method='GET',
+            uri=expected_uri,
+        )
+
     def test_build_api_url_no_extra_query_params(self):
         conn = self._make_one(object())
         URI = '/'.join([