From 5a6cf0511d385b8108a797339baab4bd613e8900 Mon Sep 17 00:00:00 2001 From: Marrony Neris Date: Wed, 2 Nov 2016 17:23:56 -0200 Subject: [PATCH 1/4] adds new_session_client method --- faunadb/client.py | 91 +++++++++++++++++++++++++++++-------- tests/helpers.py | 15 +++--- tests/test_client.py | 15 +++--- tests/test_client_logger.py | 3 +- tests/test_errors.py | 4 +- tests/test_query.py | 4 +- 6 files changed, 92 insertions(+), 40 deletions(-) diff --git a/faunadb/client.py b/faunadb/client.py index f06d8f1b..3e640b2c 100644 --- a/faunadb/client.py +++ b/faunadb/client.py @@ -1,14 +1,37 @@ from time import time # pylint: disable=redefined-builtin from builtins import object +import threading from requests import Request, Session +from requests.auth import HTTPBasicAuth from faunadb.errors import _get_or_raise, FaunaError, UnexpectedError from faunadb.query import _wrap from faunadb.request_result import RequestResult from faunadb._json import parse_json_or_none, to_json +class _Counter(object): + def __init__(self): + self.lock = threading.Lock() + self.counter = 0 + + def __str__(self): + return "Counter(%s)" % self.counter + + def increment(self): + with self.lock: + self.counter += 1 + return self.counter + + def decrement(self): + with self.lock: + self.counter -= 1 + return self.counter + + def get(self): + with self.lock: + return self.counter class FaunaClient(object): """ @@ -27,13 +50,16 @@ class FaunaClient(object): # pylint: disable=too-many-arguments, too-many-instance-attributes def __init__( self, + secret, domain="rest.faunadb.com", scheme="https", port=None, timeout=60, - secret=None, - observer=None): + observer=None, + **kwargs): """ + :param secret: + Auth token for the FaunaDB server. :param domain: Base URL for the FaunaDB server. :param scheme: @@ -42,8 +68,6 @@ def __init__( Port of the FaunaDB server. :param timeout: Read timeout in seconds. - :param secret: - Auth token for the FaunaDB server. :param observer: Callback that will be passed a :any:`RequestResult` after every completed request. """ @@ -52,26 +76,32 @@ def __init__( self.scheme = scheme self.port = (443 if scheme == "https" else 80) if port is None else port - self.session = Session() - if secret is not None: - self.session.auth = (secret, "") + self.auth = HTTPBasicAuth(secret, "") + self.base_url = "%s://%s:%s" % (self.scheme, self.domain, self.port) + self.observer = observer - self.session.headers.update({ - "Accept-Encoding": "gzip", - "Content-Type": "application/json;charset=utf-8" - }) - self.session.timeout = timeout + if ('session' not in kwargs) or ('counter' not in kwargs): + self.session = Session() + self.counter = _Counter() - self.base_url = "%s://%s:%s" % (self.scheme, self.domain, self.port) + self.session.headers.update({ + "Accept-Encoding": "gzip", + "Content-Type": "application/json;charset=utf-8" + }) + self.session.timeout = timeout + else: + self.session = kwargs['session'] + self.counter = kwargs['counter'] - self.observer = observer + self.counter.increment() def __del__(self): # pylint: disable=bare-except - try: - self.session.close() - except: - pass + if self.counter.decrement() == 0: + try: + self.session.close() + except: + pass def query(self, expression): """ @@ -89,6 +119,29 @@ def ping(self, scope=None, timeout=None): """ return self._execute("GET", "ping", query={"scope": scope, "timeout": timeout}) + def new_session_client(self, secret, observer=None): + """ + Create a new client from the existing config with a given secret. + The returned client share its parent underlying resources. + + :param secret: + Credentials to use when sending requests. + :param observer: + Callback that will be passed a :any:`RequestResult` after every completed request. + :return: + """ + if self.counter.get() > 0: + return FaunaClient(secret=secret, + domain=self.domain, + scheme=self.scheme, + port=self.port, + timeout=self.session.timeout, + observer=observer or self.observer, + session=self.session, + counter=self.counter) + else: + raise UnexpectedError("Cannnot create a session client from a closed session", None) + def _execute(self, action, path, data=None, query=None): """Performs an HTTP action, logs it, and looks for errors.""" if query is not None: @@ -118,5 +171,5 @@ def _execute(self, action, path, data=None, query=None): def _perform_request(self, action, path, data, query): """Performs an HTTP action.""" url = self.base_url + "/" + path - req = Request(action, url, params=query, data=to_json(data)) + req = Request(action, url, params=query, data=to_json(data), auth=self.auth) return self.session.send(self.session.prepare_request(req)) diff --git a/tests/helpers.py b/tests/helpers.py index 95cc9575..d74b2e1b 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -28,7 +28,7 @@ def setUpClass(cls): # Turn off annoying logging about reset connections. getLogger("requests").setLevel(WARNING) - cls.root_client = cls.get_client(secret=_FAUNA_ROOT_KEY) + cls.root_client = cls._get_client() rnd = ''.join(random.choice(string.ascii_lowercase) for _ in range(10)) db_name = "faunadb-python-test" + rnd @@ -41,11 +41,11 @@ def setUpClass(cls): cls.server_key = cls.root_client.query( query.create(Ref("keys"), {"database": cls.db_ref, "role": "server"}))["secret"] - cls.client = cls.get_client() + cls.client = cls.root_client.new_session_client(secret=cls.server_key) cls.admin_key = cls.root_client.query( query.create(Ref("keys"), {"database": cls.db_ref, "role": "admin"}))["secret"] - cls.admin_client = cls.get_client(secret=cls.admin_key) + cls.admin_client = cls.root_client.new_session_client(secret=cls.admin_key) @classmethod def tearDownClass(cls): @@ -76,14 +76,11 @@ def assertRaisesRegexCompat(self, exception, regexp, callable, *args, **kwds): self.assertRaisesRegexp(exception, regexp, callable, *args, **kwds) @classmethod - def get_client(cls, secret=None, observer=None): - if secret is None: - secret = cls.server_key - + def _get_client(cls): args = {"domain": _FAUNA_DOMAIN, "scheme": _FAUNA_SCHEME, "port": _FAUNA_PORT} # If None, use default instead non_null_args = {k: v for k, v in args.items() if v is not None} - return FaunaClient(secret=secret, observer=observer, **non_null_args) + return FaunaClient(secret=_FAUNA_ROOT_KEY, **non_null_args) def assert_raises(self, exception_class, action): """Like self.assertRaises and returns the exception too.""" @@ -93,7 +90,7 @@ def assert_raises(self, exception_class, action): def mock_client(response_text, status_code=codes.ok): - c = FaunaClient() + c = FaunaClient(secret=None) c.session = _MockSession(response_text, status_code) return c diff --git a/tests/test_client.py b/tests/test_client.py index d62ae87e..f7871eae 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1,5 +1,5 @@ -from faunadb.objects import Ref -from faunadb.query import create +from faunadb.client import FaunaClient +from faunadb.errors import UnexpectedError from tests.helpers import FaunaTestCase class ClientTest(FaunaTestCase): @@ -7,8 +7,9 @@ class ClientTest(FaunaTestCase): def test_ping(self): self.assertEqual(self.client.ping("all"), "Scope all is OK") - def _create_class(self): - return self.client.query(create(Ref("classes"), {"name": "my_class"})) - - def _create_instance(self): - return self.client.query(create(Ref("classes/my_class"), {})) + def test_error_on_closed_client(self): + client = FaunaClient(secret="secret") + client.__del__() + self.assertRaisesRegexCompat(UnexpectedError, + "Cannnot create a session client from a closed session", + lambda: client.new_session_client(secret="new_secret")) diff --git a/tests/test_client_logger.py b/tests/test_client_logger.py index 114cf976..36206940 100644 --- a/tests/test_client_logger.py +++ b/tests/test_client_logger.py @@ -55,6 +55,7 @@ def test_url_query(self): def get_logged(self, client_action): logged_box = [] - client = self.get_client(observer=logger(logged_box.append)) + client = self.root_client.new_session_client(secret=self.server_key, + observer=logger(logged_box.append)) client_action(client) return logged_box[0] diff --git a/tests/test_errors.py b/tests/test_errors.py index 63d27ca6..7d7fa484 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -36,13 +36,13 @@ def test_unexpected_error_code(self): #region FaunaError def test_unauthorized(self): - client = self.get_client(secret="bad_key") + client = self.root_client.new_session_client(secret="bad_key") self._assert_http_error( lambda: client.query(get(self.db_ref)), Unauthorized, "unauthorized") def test_permission_denied(self): # Create client with client key - client = self.get_client( + client = self.root_client.new_session_client( secret=self.root_client.query( create(Ref("keys"), {"database": self.db_ref, "role": "client"}))["secret"] ) diff --git a/tests/test_query.py b/tests/test_query.py index a9ea31a0..570d2bb2 100644 --- a/tests/test_query.py +++ b/tests/test_query.py @@ -261,7 +261,7 @@ def test_create_key(self): "database": Ref("databases/database_for_key_test"), "role": "server"})) - new_client = self.get_client(secret=resource["secret"]) + new_client = self.admin_client.new_session_client(secret=resource["secret"]) new_client.query(query.create_class({"name": "class_for_test"})) @@ -345,7 +345,7 @@ def test_login_logout(self): query.create(self.class_ref, {"credentials": {"password": "sekrit"}}))["ref"] secret = self.client.query( query.login(instance_ref, {"password": "sekrit"}))["secret"] - instance_client = self.get_client(secret=secret) + instance_client = self.client.new_session_client(secret=secret) self.assertEqual(instance_client.query( query.select("ref", query.get(Ref("classes/widgets/self")))), instance_ref) From 3d5c403a6ce68d2b54674752dc4ff142584aada6 Mon Sep 17 00:00:00 2001 From: Marrony Neris Date: Thu, 3 Nov 2016 15:08:12 -0200 Subject: [PATCH 2/4] fix possible thread racing when creating new session clients --- faunadb/client.py | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/faunadb/client.py b/faunadb/client.py index 3e640b2c..8f64a4c4 100644 --- a/faunadb/client.py +++ b/faunadb/client.py @@ -12,27 +12,24 @@ from faunadb._json import parse_json_or_none, to_json class _Counter(object): - def __init__(self): + def __init__(self, init_value=0): self.lock = threading.Lock() - self.counter = 0 + self.counter = init_value def __str__(self): return "Counter(%s)" % self.counter - def increment(self): + def get_and_increment(self): with self.lock: + counter = self.counter self.counter += 1 - return self.counter + return counter def decrement(self): with self.lock: self.counter -= 1 return self.counter - def get(self): - with self.lock: - return self.counter - class FaunaClient(object): """ Directly communicates with FaunaDB via JSON. @@ -82,7 +79,7 @@ def __init__( if ('session' not in kwargs) or ('counter' not in kwargs): self.session = Session() - self.counter = _Counter() + self.counter = _Counter(1) self.session.headers.update({ "Accept-Encoding": "gzip", @@ -93,8 +90,6 @@ def __init__( self.session = kwargs['session'] self.counter = kwargs['counter'] - self.counter.increment() - def __del__(self): # pylint: disable=bare-except if self.counter.decrement() == 0: @@ -130,7 +125,7 @@ def new_session_client(self, secret, observer=None): Callback that will be passed a :any:`RequestResult` after every completed request. :return: """ - if self.counter.get() > 0: + if self.counter.get_and_increment() > 0: return FaunaClient(secret=secret, domain=self.domain, scheme=self.scheme, From a9ffffe17bdecb6aea57618a036eb9e2feeecfac Mon Sep 17 00:00:00 2001 From: Marrony Neris Date: Thu, 3 Nov 2016 15:47:19 -0200 Subject: [PATCH 3/4] adds parameters to configure number of connections to pool --- faunadb/client.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/faunadb/client.py b/faunadb/client.py index 8f64a4c4..e073bc6b 100644 --- a/faunadb/client.py +++ b/faunadb/client.py @@ -5,6 +5,7 @@ from requests import Request, Session from requests.auth import HTTPBasicAuth +from requests.adapters import HTTPAdapter from faunadb.errors import _get_or_raise, FaunaError, UnexpectedError from faunadb.query import _wrap @@ -53,6 +54,8 @@ def __init__( port=None, timeout=60, observer=None, + pool_connections=10, + pool_maxsize=10, **kwargs): """ :param secret: @@ -67,6 +70,10 @@ def __init__( Read timeout in seconds. :param observer: Callback that will be passed a :any:`RequestResult` after every completed request. + :param pool_connections: + The number of connection pools to cache. + :param pool_maxsize: + The maximum number of connections to save in the pool. """ self.domain = domain @@ -77,8 +84,15 @@ def __init__( self.base_url = "%s://%s:%s" % (self.scheme, self.domain, self.port) self.observer = observer + self.pool_connections = pool_connections + self.pool_maxsize = pool_maxsize + if ('session' not in kwargs) or ('counter' not in kwargs): self.session = Session() + self.session.mount('https://', HTTPAdapter(pool_connections=pool_connections, + pool_maxsize=pool_maxsize)) + self.session.mount('http://', HTTPAdapter(pool_connections=pool_connections, + pool_maxsize=pool_maxsize)) self.counter = _Counter(1) self.session.headers.update({ @@ -133,7 +147,9 @@ def new_session_client(self, secret, observer=None): timeout=self.session.timeout, observer=observer or self.observer, session=self.session, - counter=self.counter) + counter=self.counter, + pool_connections=self.pool_connections, + pool_maxsize=self.pool_maxsize) else: raise UnexpectedError("Cannnot create a session client from a closed session", None) From a6c77bca23d55aceaa09ab219900ffb9d411734f Mon Sep 17 00:00:00 2001 From: Marrony Neris Date: Thu, 3 Nov 2016 19:55:57 -0200 Subject: [PATCH 4/4] remove unnecessary try/catch --- faunadb/client.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/faunadb/client.py b/faunadb/client.py index e073bc6b..a644e9a7 100644 --- a/faunadb/client.py +++ b/faunadb/client.py @@ -105,12 +105,8 @@ def __init__( self.counter = kwargs['counter'] def __del__(self): - # pylint: disable=bare-except if self.counter.decrement() == 0: - try: - self.session.close() - except: - pass + self.session.close() def query(self, expression): """