From 2ec96b6672a9282a0dd12980bd7ce50b58da6d42 Mon Sep 17 00:00:00 2001 From: Federico Ceratto Date: Thu, 2 Jul 2015 15:59:39 +0100 Subject: [PATCH] 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 --- designateclient/tests/base.py | 4 +- designateclient/tests/test_v1/test_client.py | 24 +++- designateclient/tests/v2/test_client.py | 32 ++++++ designateclient/tests/v2/test_timeout.py | 109 +++++++++++++++++++ designateclient/utils.py | 20 ++++ designateclient/v1/__init__.py | 11 +- designateclient/v2/client.py | 25 ++++- doc/examples/zone_list_paging.py | 2 +- 8 files changed, 214 insertions(+), 13 deletions(-) create mode 100644 designateclient/tests/v2/test_client.py create mode 100644 designateclient/tests/v2/test_timeout.py 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 +# +# 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 +# +# 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)