From 5f50a147cff292656a29ee2c0ecfa4f2fd3f5ce1 Mon Sep 17 00:00:00 2001 From: Frank Smit Date: Wed, 21 Oct 2015 13:03:17 +0200 Subject: [PATCH 01/11] Fix typo in readme. --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 17ff8a7..ba204ed 100644 --- a/README.rst +++ b/README.rst @@ -12,7 +12,7 @@ Momoko Momoko wraps Psycopg2_'s functionality for use in Tornado_. Have a look at tutorial_ or full documentation_. -**Important:** This is the 2.x version of Momoko. It requres Tornado >= 4.0, uses futures instead of calllbacks +**Important:** This is the 2.x version of Momoko. It requires Tornado >= 4.0, uses futures instead of calllbacks and introduces a slightly different API compared to 1.x version. While transition is very straightforward, the API is not backward compatible with 1.x! From de98f2f20d7f98002fd54eee514afc22ea63d344 Mon Sep 17 00:00:00 2001 From: Zaar Hai Date: Sun, 25 Oct 2015 17:15:34 +0200 Subject: [PATCH 02/11] Catching IOError. Fixes #127 Under heavy load we get "No such file or directory" error when updating callbacks. I've failed to reproduce it with the tcpproxy setup, so this bugfix is currently not covered with unittest. --- momoko/connection.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/momoko/connection.py b/momoko/connection.py index 9e7607c..79d9ab1 100644 --- a/momoko/connection.py +++ b/momoko/connection.py @@ -703,15 +703,21 @@ def _io_callback(self, future, result, fd=None, events=None): self.ioloop.remove_handler(self.fileno) future.set_exc_info(sys.exc_info()) else: - if state == POLL_OK: + try: + if state == POLL_OK: + self.ioloop.remove_handler(self.fileno) + future.set_result(result) + elif state == POLL_READ: + self.ioloop.update_handler(self.fileno, IOLoop.READ) + elif state == POLL_WRITE: + self.ioloop.update_handler(self.fileno, IOLoop.WRITE) + else: + future.set_exception(psycopg2.OperationalError("poll() returned %s" % state)) + except IOError: + # Can happen when there are quite a lof of outstanding + # requests. See https://github.com/FSX/momoko/issues/127 self.ioloop.remove_handler(self.fileno) - future.set_result(result) - elif state == POLL_READ: - self.ioloop.update_handler(self.fileno, IOLoop.READ) - elif state == POLL_WRITE: - self.ioloop.update_handler(self.fileno, IOLoop.WRITE) - else: - future.set_exception(psycopg2.OperationalError("poll() returned %s" % state)) + future.set_exception(psycopg2.OperationalError("IOError on socker")) def ping(self): """ From 100bc5aefcb899489a27564a86b686d9939e53fb Mon Sep 17 00:00:00 2001 From: Greg Ward Date: Mon, 2 Nov 2015 14:44:43 -0500 Subject: [PATCH 03/11] docs: clarify an ambiguous paragraph --- docs/tutorial.rst | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/tutorial.rst b/docs/tutorial.rst index 3bcb84c..3efc189 100644 --- a/docs/tutorial.rst +++ b/docs/tutorial.rst @@ -11,9 +11,10 @@ wraps Psycopg2, the `Psycopg2 documentation`_ must be used alongside Momoko's. The principle ------------- -All of the :py:meth:`~momoko.Pool` and :py:meth:`~momoko.Connection` methods return -_futures. There are some notable exceptions like :py:meth:`~momoko.Pool.close` - make sure -to consule API documentation for the details. +Almost every method of :py:meth:`~momoko.Pool` and :py:meth:`~momoko.Connection` +returns a `future`_. There are some notable exceptions, like +:py:meth:`~momoko.Pool.close`; be sure to consult API documentation for the +details. These future objects can be simply ``yield``-ed in Tornado methods decorated with ``gen.coroutine``. For SQL execution related methods these futures resolve to corresponding cursor objects. @@ -226,4 +227,4 @@ Here is the server-side cursor example (based on the code in momoko unittests):: .. _Wait: http://tornado.readthedocs.org/en/stable/gen.html#tornado.gen.Wait .. _WaitAll: http://tornado.readthedocs.org/en/stable/gen.html#tornado.gen.WaitAll .. _exceptions: http://initd.org/psycopg/docs/module.html#exceptions -.. _futures: http://tornado.readthedocs.org/en/latest/concurrent.html +.. _future: http://tornado.readthedocs.org/en/latest/concurrent.html From d97af27306153c058c6e4f3b1c37ac40ee20c86a Mon Sep 17 00:00:00 2001 From: Greg Ward Date: Mon, 2 Nov 2015 14:45:13 -0500 Subject: [PATCH 04/11] Grammar/typo fixes in a docstring. --- momoko/connection.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/momoko/connection.py b/momoko/connection.py index 79d9ab1..6c8abcb 100644 --- a/momoko/connection.py +++ b/momoko/connection.py @@ -286,13 +286,13 @@ def getconn(self, ping=True): """ Acquire connection from the pool. - You can then use this connection for subsequest queries. + You can then use this connection for subsequent queries. Just use ``connection.execute`` instead of ``Pool.execute``. Make sure to return connection to the pool by calling :py:meth:`momoko.Pool.putconn`, - otherwise the connection will remain forever-busy and you'll starvate your pool quickly. + otherwise the connection will remain forever busy and you'll starve your pool. - Returns future that resolves to the acquired connection object. + Returns a future that resolves to the acquired connection object. :param boolean ping: Whether to ping the connection before returning it by executing :py:meth:`momoko.Connection.ping`. From d31975a330206ee4ade97f1e63997bd4097b9df5 Mon Sep 17 00:00:00 2001 From: Greg Ward Date: Mon, 2 Nov 2015 14:56:40 -0500 Subject: [PATCH 05/11] Fix another docstring typo. --- tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests.py b/tests.py index 26e2e3b..4ccae9d 100644 --- a/tests.py +++ b/tests.py @@ -667,7 +667,7 @@ def test_request_queueing(self): self.run_parallel_queries(self.pool_size*2) def test_parallel_queries_after_reconnect_all(self): - """Testing that pool still queries database in parallel after ALL connections were closeded""" + """Testing that pool still queries database in parallel after ALL connections were closed""" self.shutter(self.db) self.run_parallel_queries() From 753fefe75bcee2824e653ac73749fd740c6176ca Mon Sep 17 00:00:00 2001 From: Greg Ward Date: Mon, 2 Nov 2015 14:57:05 -0500 Subject: [PATCH 06/11] Fix spelling error (renames an instance attr). --- momoko/connection.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/momoko/connection.py b/momoko/connection.py index 6c8abcb..dff4d17 100644 --- a/momoko/connection.py +++ b/momoko/connection.py @@ -175,7 +175,7 @@ class Pool(object): connect to database during :py:meth:`momoko.Pool.connect`. :param int reconnect_interval: - If database server becomes unavailble, the pool will try to reestablish + If database server becomes unavailable, the pool will try to reestablish the connection. The attempt frequency is ``reconnect_interval`` milliseconds. @@ -244,7 +244,7 @@ def __init__(self, self.conns = ConnectionContainer() self._last_connect_time = 0 - self._no_conn_availble_error = self.DatabaseNotAvailable("No database connection available") + self._no_conn_available_error = self.DatabaseNotAvailable("No database connection available") self.shrink_period = shrink_period self.shrink_delay = shrink_delay self.auto_shrink = auto_shrink @@ -307,7 +307,7 @@ def getconn(self, ping=True): def on_reanimate_done(fut): if self.conns.all_dead: - future.set_exception(self._no_conn_availble_error) + future.set_exception(self._no_conn_available_error) return f = self.conns.acquire() assert isinstance(f, Future) @@ -333,7 +333,7 @@ def putconn(self, connection): self.conns.release(connection) if self.conns.all_dead: - self.conns.abort_waiting_queue(self._no_conn_availble_error) + self.conns.abort_waiting_queue(self._no_conn_available_error) @contextmanager def manage(self, connection): @@ -495,7 +495,7 @@ def _retry(self, retry, what, conn, keep, future): self.ioloop.add_future(conn.connect(), what) return else: - future.set_exception(self._no_conn_availble_error) + future.set_exception(self._no_conn_available_error) else: future.set_exc_info(sys.exc_info()) if not keep: @@ -580,7 +580,7 @@ def on_ping_done(ping_fut): ping_fut.result() except psycopg2.Error as error: if conn.closed: - ping_future.set_exception(self._no_conn_availble_error) + ping_future.set_exception(self._no_conn_available_error) else: ping_future.set_exc_info(sys.exc_info()) self.putconn(conn) From 192b080bb2c8c00a23f9094fe5732004d61e63c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=91=A8=E7=90=AA?= Date: Fri, 6 Nov 2015 13:36:07 +0800 Subject: [PATCH 07/11] Update README.rst typo --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index ba204ed..73ee4aa 100644 --- a/README.rst +++ b/README.rst @@ -40,7 +40,7 @@ Testing Set the following environment variables with your own values before running the unit tests:: - make -C tcpproxy + make -C tcproxy export MOMOKO_TEST_DB='your_db' export MOMOKO_TEST_USER='your_user' export MOMOKO_TEST_PASSWORD='your_password' From 7cc96cde6b29bca26345a6c8d4c7fdcfeabea59a Mon Sep 17 00:00:00 2001 From: Zaar Hai Date: Mon, 30 Nov 2015 14:38:45 +0200 Subject: [PATCH 08/11] Prevening tornado from logging (falsy) errors --- tests.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/tests.py b/tests.py index 26e2e3b..ca924b8 100644 --- a/tests.py +++ b/tests.py @@ -667,7 +667,7 @@ def test_request_queueing(self): self.run_parallel_queries(self.pool_size*2) def test_parallel_queries_after_reconnect_all(self): - """Testing that pool still queries database in parallel after ALL connections were closeded""" + """Testing that pool still queries database in parallel after ALL connections were closed""" self.shutter(self.db) self.run_parallel_queries() @@ -815,14 +815,20 @@ def test_abort_waiting_queue(self): @gen_test def test_execute_can_start_before_connection_is_done(self): db = momoko.Pool(dsn=self.good_dsn, size=1, ioloop=self.io_loop) - db.connect() + f = db.connect() cursor = yield db.execute("SELECT 1") self.assertEqual(cursor.fetchone()[0], 1) + # This is to hide tornado warnings about unconsumed futures + try: + yield f + except: + pass + @gen_test def test_execute_before_connection_is_done_will_error(self): db = momoko.Pool(dsn=bad_dsn, size=1, ioloop=self.io_loop) - db.connect() + f = db.connect() try: yield db.execute("SELECT 1") @@ -833,6 +839,12 @@ def test_execute_before_connection_is_done_will_error(self): except psycopg2.DatabaseError: pass + # This is to hide tornado warnings about unconsumed futures + try: + yield f + except: + pass + class MomokoPoolVolatileDbTestProxy(ProxyMixIn, MomokoPoolVolatileDbTest): @@ -869,15 +881,14 @@ def test_execute_can_fail_after_disconnect_with_no_reconnect(self): # No start proxy here! yield gen.sleep(db.reconnect_interval) f2 = db.execute("SELECT 1") - f3 = db.execute("SELECT 1") try: - yield [f2, f3] + yield f2 self.fail("Exception should have been raised") except psycopg2.DatabaseError: pass - self.assertEqual(len(db.conns.waiting_queue), 0) + self.assertEqual(len(db.conns.waiting_queue), 0) class MomokoPoolPartiallyConnectedTest(PoolBaseTest): raise_connect_errors = True From 7dcc245ad090d9f02599acbd23c70127ddbce8a3 Mon Sep 17 00:00:00 2001 From: Zaar Hai Date: Mon, 30 Nov 2015 14:39:07 +0200 Subject: [PATCH 09/11] Catching all syncronous errors. Fixes #134. Things like `cursor.execute` can throw not only `psycopg2.Error` exeception and their derivatives, but also standard Python errors like `IndexError` bacause badly formatted SQL. In the latter case, after such exception the connection became stale (i.e. forever-busy) in the Pool. --- momoko/connection.py | 2 +- tests.py | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/momoko/connection.py b/momoko/connection.py index 79d9ab1..5959def 100644 --- a/momoko/connection.py +++ b/momoko/connection.py @@ -457,7 +457,7 @@ def when_available(fut): log.debug("Obtained connection: %s", conn.fileno) try: future_or_result = method(conn, *args, **kwargs) - except psycopg2.Error as error: + except Exception as error: log.debug("Method failed synchronously") return self._retry(retry, when_available, conn, keep, future) diff --git a/tests.py b/tests.py index ca924b8..ac02cd7 100644 --- a/tests.py +++ b/tests.py @@ -580,6 +580,15 @@ def test_getconn_manage_with_exception(self): pass self.assertEqual(len(self.db.conns.busy), 0, msg="Some connections were not recycled") + @gen_test + def test_non_psycopg2_errors(self): + """Testing that non-psycopg2 errors are catched properly""" + try: + sql = yield self.conn.execute("SELECT %s %s;", (1,)) + except IndexError: + pass + self.assertEqual(len(self.db.conns.busy), 0, msg="Some connections were not recycled") + class MomokoPoolDataTestProxy(ProxyMixIn, MomokoPoolDataTest): pass From 114bb6d5b68737cd6c0cdcca3657c74c795c729a Mon Sep 17 00:00:00 2001 From: Zaar Hai Date: Mon, 30 Nov 2015 17:46:52 +0200 Subject: [PATCH 10/11] PEP8 fix --- tests.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests.py b/tests.py index ac02cd7..e64342c 100644 --- a/tests.py +++ b/tests.py @@ -899,6 +899,7 @@ def test_execute_can_fail_after_disconnect_with_no_reconnect(self): self.assertEqual(len(db.conns.waiting_queue), 0) + class MomokoPoolPartiallyConnectedTest(PoolBaseTest): raise_connect_errors = True pool_size = 3 From ff4b2189f3a9f6127167678204f58a42cf93a9d7 Mon Sep 17 00:00:00 2001 From: Zaar Hai Date: Wed, 2 Dec 2015 10:55:30 +0200 Subject: [PATCH 11/11] Going 2.2.2 --- changelog.rst | 15 +++++++++++++++ docs/conf.py | 4 ++-- setup.py | 2 +- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/changelog.rst b/changelog.rst index 98eeeda..e79b4ab 100644 --- a/changelog.rst +++ b/changelog.rst @@ -1,6 +1,21 @@ Changelog ========= +2.2.2 (2015-12-02) +------------------ +* Doc fixes (`issue 131`_). Thanks to gward_. +* Makefile fix (`issue 132`_). Thanks to bitwolaiye_. +* Catching all syncrhonous exceptions (`issue 134`_). Thanks to m-messiah_. +* Catchin ``IOError``\ s in IOLoop handlers (`issue 127`_). + +.. _issue 127: https://github.com/FSX/momoko/issues/127 +.. _issue 131: https://github.com/FSX/momoko/issues/131 +.. _issue 132: https://github.com/FSX/momoko/issues/132 +.. _issue 134: https://github.com/FSX/momoko/issues/134 +.. _bitwolaiye: https://github.com/bitwolaiye +.. _gward: https://github.com/gward +.. _m-messiah: https://github.com/m-messiah + 2.2.1 (2015-10-13) ------------------ * Wait for pending connections during connection acquiring (`issue 122`_). Thanks to jbowes_. diff --git a/docs/conf.py b/docs/conf.py index 78dd77e..79534ab 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -49,9 +49,9 @@ # built documents. # # The short X.Y version. -version = '2.2.1' +version = '2.2.2' # The full version, including alpha/beta/rc tags. -release = '2.2.1' +release = '2.2.2' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/setup.py b/setup.py index ead7814..b19d617 100644 --- a/setup.py +++ b/setup.py @@ -34,7 +34,7 @@ setup( name='Momoko', - version='2.2.1', + version='2.2.2', description="Momoko wraps Psycopg2's functionality for use in Tornado.", long_description=open('README.rst').read(), author='Frank Smit & Zaar Hai',