Implement socket timeout in v1

Add endpoint_override keyword to v2.Client() and check for the
session argument in DesignateAdapter()
Add unit test, fix minor typos

Change-Id: I038ec7b0d1feadc9642bd47285e397b3fe84c13c
Closes-Bug: 1469739

8 files changed, 214 insertions, 13 deletions

diff --git a/designateclient/tests/base.py b/designateclient/tests/base.py
index 8ebf5aa..0e44984 100644
--- a/designateclient/tests/base.py
+++ b/designateclient/tests/base.py
@@ -18,7 +18,6 @@ import json as json_
 import os
 
 import fixtures
-from keystoneclient import adapter
 from keystoneclient import session as keystone_session
 from oslotest import base as test
 from requests_mock.contrib import fixture as req_fixture
@@ -26,6 +25,7 @@ import six
 from six.moves.urllib import parse as urlparse
 
 from designateclient import client
+from designateclient.utils import AdapterWithTimeout
 
 _TRUE_VALUES = ('True', 'true', '1', 'yes')
 
@@ -98,7 +98,7 @@ class APITestCase(TestCase):
     def get_client(self, version=None, session=None):
         version = version or self.VERSION
         session = session or keystone_session.Session()
-        adapted = adapter.Adapter(
+        adapted = AdapterWithTimeout(
             session=session, endpoint_override=self.get_base())
         return client.Client(version, session=adapted)
 
diff --git a/designateclient/tests/test_v1/test_client.py b/designateclient/tests/test_v1/test_client.py
index e195191..015e56b 100644
--- a/designateclient/tests/test_v1/test_client.py
+++ b/designateclient/tests/test_v1/test_client.py
@@ -13,10 +13,13 @@
 # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 # License for the specific language governing permissions and limitations
 # under the License.
+
 from designateclient.tests import test_v1
 from designateclient import utils
 from designateclient import v1
 
+from keystoneclient import session as keystone_session
+
 
 class TestClient(test_v1.APIV1TestCase):
     def test_all_tenants(self):
@@ -26,7 +29,7 @@ class TestClient(test_v1.APIV1TestCase):
         # Verify this has been picked up
         self.assertTrue(client.all_tenants)
 
-    def test_all_tenants_not_suplied(self):
+    def test_all_tenants_not_supplied(self):
         # Create a client without supplying any all_tenants flag
         client = v1.Client()
 
@@ -68,7 +71,7 @@ class TestClient(test_v1.APIV1TestCase):
         # Verify this has been picked up
         self.assertTrue(client.edit_managed)
 
-    def test_edit_managed_not_suplied(self):
+    def test_edit_managed_not_supplied(self):
         # Create a client without supplying any edit_managed flag
         client = v1.Client()
 
@@ -102,3 +105,20 @@ class TestClient(test_v1.APIV1TestCase):
 
         # Verify the edit_managed flag has been picked up
         self.assertTrue(client.edit_managed)
+
+    def test_timeout_new_session(self):
+        client = v1.Client(
+            auth_url="http://127.0.0.1:22/",
+            timeout=1,
+        )
+        assert client.session.timeout == 1
+
+    def test_timeout_override_session_timeout(self):
+        # The adapter timeout should override the session timeout
+        session = keystone_session.Session(timeout=10)
+        client = v1.Client(
+            auth_url="http://127.0.0.1:22/",
+            session=session,
+            timeout=2,
+        )
+        self.assertEqual(client.session.timeout, 2)
diff --git a/designateclient/tests/v2/test_client.py b/designateclient/tests/v2/test_client.py
new file mode 100644
index 0000000..3b44d4c
--- /dev/null
+++ b/designateclient/tests/v2/test_client.py
@@ -0,0 +1,32 @@
+# Copyright 2015 Hewlett-Packard Development Company, L.P.
+#
+# Author: Federico Ceratto <federico.ceratto@hp.com>
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from keystoneclient import adapter
+from keystoneclient import session as keystone_session
+
+from designateclient.tests.base import TestCase
+from designateclient.v2.client import Client
+
+
+class TestClient(TestCase):
+    def test_init(self):
+        self.assertRaises(ValueError, Client)
+
+    def test_init_with_session(self):
+        session = keystone_session.Session()
+        adapted = adapter.Adapter(session=session)
+        client = Client(session=adapted)
+        assert client.session
diff --git a/designateclient/tests/v2/test_timeout.py b/designateclient/tests/v2/test_timeout.py
new file mode 100644
index 0000000..3bdcb1b
--- /dev/null
+++ b/designateclient/tests/v2/test_timeout.py
@@ -0,0 +1,109 @@
+# Copyright 2015 Hewlett-Packard Development Company, L.P.
+#
+# Author: Federico Ceratto <federico.ceratto@hp.com>
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from keystoneclient.auth.identity import generic
+from keystoneclient import session as keystone_session
+from mock import Mock
+
+from designateclient.tests import v2
+from designateclient.v2.client import Client
+
+
+def create_session(timeout=None):
+    auth = generic.Password(auth_url='', username='', password='',
+                            tenant_name='')
+    return keystone_session.Session(auth=auth, timeout=timeout)
+
+
+class TestTimeout(v2.APIV2TestCase, v2.CrudMixin):
+
+    def setUp(self):
+        super(TestTimeout, self).setUp()
+
+        # Mock methods in KeyStone's Session
+        self._saved_methods = (
+            keystone_session.Session.get_auth_headers,
+            keystone_session.Session.get_endpoint,
+            keystone_session.Session._send_request,
+        )
+
+        resp = Mock()
+        resp.text = ''
+        resp.status_code = 200
+
+        keystone_session.Session.get_auth_headers = Mock(
+            return_value=[]
+        )
+        keystone_session.Session.get_endpoint = Mock(
+            return_value='foo'
+        )
+        keystone_session.Session._send_request = Mock(
+            return_value=resp,
+        )
+        self.mock_send_request = keystone_session.Session._send_request
+
+    def tearDown(self):
+        super(TestTimeout, self).tearDown()
+        (
+            keystone_session.Session.get_auth_headers,
+            keystone_session.Session.get_endpoint,
+            keystone_session.Session._send_request,
+        ) = self._saved_methods
+
+    def _call_request_and_check_timeout(self, client, timeout):
+        """call the mocked _send_request() and check if the timeout was set
+        """
+        client.limits.get()
+        self.assertTrue(self.mock_send_request.called)
+        kw = self.mock_send_request.call_args[1]
+        if timeout is None:
+            self.assertFalse('timeout' in kw)
+        else:
+            self.assertEqual(kw['timeout'], timeout)
+
+    def test_no_timeout(self):
+        session = create_session(timeout=None)
+        client = Client(session=session)
+        self.assertEqual(session.timeout, None)
+        self.assertEqual(client.session.timeout, None)
+        self._call_request_and_check_timeout(client, None)
+
+    def test_timeout_in_session(self):
+        session = create_session(timeout=1)
+        client = Client(session=session)
+        self.assertEqual(session.timeout, 1)
+        self.assertEqual(client.session.timeout, None)
+        self._call_request_and_check_timeout(client, 1)
+
+    def test_timeout_override_session_timeout(self):
+        # The adapter timeout should override the session timeout
+        session = create_session(timeout=10)
+        self.assertEqual(session.timeout, 10)
+        client = Client(session=session, timeout=2)
+        self.assertEqual(client.session.timeout, 2)
+        self._call_request_and_check_timeout(client, 2)
+
+    def test_timeout_update(self):
+        session = create_session(timeout=1)
+        client = Client(session=session)
+        self.assertEqual(session.timeout, 1)
+        self.assertEqual(client.session.timeout, None)
+        self._call_request_and_check_timeout(client, 1)
+
+        session.timeout = 2
+        self.assertEqual(session.timeout, 2)
+
+        self._call_request_and_check_timeout(client, 2)
diff --git a/designateclient/utils.py b/designateclient/utils.py
index ea5d48b..1067463 100644
--- a/designateclient/utils.py
+++ b/designateclient/utils.py
@@ -19,6 +19,7 @@ import os
 import uuid
 
 from debtcollector import removals
+from keystoneclient import adapter
 from keystoneclient.auth.identity import generic
 from keystoneclient.auth import token_endpoint
 from keystoneclient import session as ks_session
@@ -172,3 +173,22 @@ def find_resourceid_by_name_or_id(resource_client, name_or_id):
         raise exceptions.NoUniqueMatch(
             'Multiple resources with name "%s": %s' % (name_or_id, str_ids))
     return candidate_ids[0]
+
+
+class AdapterWithTimeout(adapter.Adapter):
+    """adapter.Adapter wraps around a Session.
+    The user can pass a timeout keyword that will apply only to
+    the Designate Client, in order:
+        - timeout keyword passed to request()
+        - timeout keyword passed to AdapterWithTimeout()
+        - timeout attribute on keystone session
+    """
+    def __init__(self, *args, **kw):
+        self.timeout = kw.pop('timeout', None)
+        super(self.__class__, self).__init__(*args, **kw)
+
+    def request(self, *args, **kwargs):
+        if self.timeout is not None:
+            kwargs.setdefault('timeout', self.timeout)
+
+        return super(AdapterWithTimeout, self).request(*args, **kwargs)
diff --git a/designateclient/v1/__init__.py b/designateclient/v1/__init__.py
index 5c216dc..d38303d 100644
--- a/designateclient/v1/__init__.py
+++ b/designateclient/v1/__init__.py
@@ -13,7 +13,6 @@
 # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 # License for the specific language governing permissions and limitations
 # under the License.
-from keystoneclient import adapter
 from stevedore import extension
 
 from designateclient import exceptions
@@ -32,7 +31,8 @@ class Client(object):
                  project_domain_id=None, auth_url=None, token=None,
                  endpoint_type='publicURL', region_name=None,
                  service_type='dns', insecure=False, session=None,
-                 cacert=None, all_tenants=None, edit_managed=None):
+                 cacert=None, all_tenants=None, edit_managed=None,
+                 timeout=None):
         """
         :param endpoint: Endpoint URL
         :param token: A token instead of username / password
@@ -62,7 +62,7 @@ class Client(object):
                 user_domain_name=user_domain_name,
                 token=token,
                 insecure=insecure,
-                cacert=cacert
+                cacert=cacert,
             )
 
         # NOTE: all_tenants and edit_managed are pulled from the session for
@@ -82,7 +82,7 @@ class Client(object):
         # an adapter around the session to not modify it's state.
         interface = endpoint_type.rstrip('URL')
 
-        self.session = adapter.Adapter(
+        self.session = utils.AdapterWithTimeout(
             session,
             auth=session.auth,
             endpoint_override=endpoint,
@@ -90,7 +90,8 @@ class Client(object):
             service_type=service_type,
             interface=interface,
             user_agent='python-designateclient-%s' % version.version_info,
-            version='1'
+            version='1',
+            timeout=timeout,
         )
 
         def _load_controller(ext):
diff --git a/designateclient/v2/client.py b/designateclient/v2/client.py
index 1dbac3a..daeb48e 100644
--- a/designateclient/v2/client.py
+++ b/designateclient/v2/client.py
@@ -30,13 +30,25 @@ from designateclient import version
 class DesignateAdapter(adapter.LegacyJsonAdapter):
     """
     Adapter around LegacyJsonAdapter.
+    The user can pass a timeout keyword that will apply only to
+    the Designate Client, in order:
+        - timeout keyword passed to request()
+        - timeout attribute on keystone session
     """
+    def __init__(self, *args, **kwargs):
+        self.timeout = kwargs.pop('timeout', None)
+        super(self.__class__, self).__init__(*args, **kwargs)
+
     def request(self, *args, **kwargs):
         kwargs.setdefault('raise_exc', False)
 
+        if self.timeout is not None:
+            kwargs.setdefault('timeout', self.timeout)
+
         kwargs.setdefault('headers', {}).setdefault(
             'Content-Type', 'application/json')
-        response, body = super(DesignateAdapter, self).request(*args, **kwargs)
+
+        response, body = super(self.__class__, self).request(*args, **kwargs)
 
         # Decode is response, if possible
         try:
@@ -60,7 +72,11 @@ class DesignateAdapter(adapter.LegacyJsonAdapter):
 class Client(object):
     def __init__(self, region_name=None, endpoint_type='publicURL',
                  extensions=None, service_type='dns', service_name=None,
-                 http_log_debug=False, session=None, auth=None):
+                 http_log_debug=False, session=None, auth=None, timeout=None,
+                 endpoint_override=None):
+        if session is None:
+            raise ValueError("A session instance is required")
+
         self.session = DesignateAdapter(
             session,
             auth=auth,
@@ -68,7 +84,10 @@ class Client(object):
             service_type=service_type,
             interface=endpoint_type.rstrip('URL'),
             user_agent='python-designateclient-%s' % version.version_info,
-            version=('2'))
+            version=('2'),
+            endpoint_override=endpoint_override,
+            timeout=timeout
+        )
 
         self.blacklists = BlacklistController(self)
         self.floatingips = FloatingIPController(self)
diff --git a/doc/examples/zone_list_paging.py b/doc/examples/zone_list_paging.py
index 47aba29..ca8ab3a 100644
--- a/doc/examples/zone_list_paging.py
+++ b/doc/examples/zone_list_paging.py
@@ -14,7 +14,7 @@ auth = generic.Password(
     password=shell.env('OS_PASSWORD'),
     tenant_name=shell.env('OS_TENANT_NAME'))
 
-session = keystone_session.Session(auth=auth)
+session = keystone_session.Session(auth=auth, timeout=10)
 
 client = client.Client(session=session)