Merge pull request #36 from zopefoundation/dataflake/mysqlclient_ping…

…_fix

Attempt to fix the ``ping`` behavior change

.meta.toml

@@ -21,6 +21,7 @@ known_third_party = "six, MySQLdb, pkg_resources"
 [check-manifest]
 additional-ignores = [
     "docs/_build/html/_static/*",
+    "docs/_build/html/_static/scripts/*",
     ]
 
 [manifest]

CHANGES.rst

@@ -4,6 +4,9 @@ Products.ZMySQLDA change log
 5.1 (unreleased)
 ----------------
 
+- Adjust for ``mysqlclient`` behavior change after version 2.2.0
+  (`#34 <https://github.com/zopefoundation/Products.ZMySQLDA/issues/34>`_)
+
 - Add support for Python 3.12 and 3.13.
 
 - Drop support for Python 3.7.

setup.cfg

@@ -12,6 +12,7 @@ ignore =
     .meta.toml
     docs/_build/html/_sources/*
     docs/_build/html/_static/*
+    docs/_build/html/_static/scripts/*
 
 [isort]
 force_single_line = True

src/Products/ZMySQLDA/db.py

@@ -323,16 +323,32 @@ def close(self):
 
     __del__ = close
 
-    def _forceReconnection(self):
+    def _forceReconnection(self, reason=''):
         """ (Re)Connect to database.
+
+        Kwargs:
+            reason (str): A reason for the reconnection, which will be logged.
         """
+        if reason:
+            if reason in hosed_connection:
+                LOG.error('Forcing reconnection: %s' % hosed_connection[
+                    reason])
+            else:
+                LOG.debug('Forcing reconnection, reason: %s.' % reason)
+
         try:  # try to clean up first
             self.db.close()
         except Exception:
             pass
+
         self.db = MySQLdb.connect(**self._kw_args)
-        # Newer mysqldb requires ping argument to attmept a reconnect.
-        # This setting is persistent, so only needed once per connection.
+        # Calling ``ping`` to verify that the connection works and passing
+        # ``True`` to enable the automatic reconnection feature.
+        # The MySQL/MariaDB client library supports automatic reconnections if
+        # the connection is down and a statement is sent to the server. This
+        # option (``MYSQL_OPT_RECONNECT``) is deprecated starting with MySQL
+        # 8.0.34/8.1 and will cause a warning to be written to STDERR.
+        # Future versions of this package will disable it.
         self.db.ping(True)
 
     @classmethod
@@ -533,16 +549,11 @@ def _query(self, query, force_reconnect=False):
                 raise
 
             # Hm. maybe the db is hosed.  Let's restart it.
-            if exc.args[0] in hosed_connection:
-                msg = '%s Forcing a reconnect.' % hosed_connection[exc.args[0]]
-                LOG.error(msg)
-            self._forceReconnection()
+            self._forceReconnection(reason=exc.args[0])
             self.db.query(query)
         except ProgrammingError as exc:
             if exc.args[0] in hosed_connection:
-                self._forceReconnection()
-                msg = '%s Forcing a reconnect.' % hosed_connection[exc.args[0]]
-                LOG.error(msg)
+                self._forceReconnection(reason=exc.args[0])
             else:
                 if len(query) > 2000:
                     msg = '%s... (truncated at 2000 chars)' % query[:2000]
@@ -640,15 +651,26 @@ def _begin(self, *ignored):
         Also called from _register() upon first query.
         """
         try:
+            try:
+                # Calling ``ping`` to verify that the connection is working.
+                self.db.ping()
+            except OperationalError as exc:
+                # Before mysqlclient version 2.2.1 the ``ping`` method seemed
+                # to never raise exceptions, now it does. Attempt to reconnect
+                # if the exception type implies a stale connection.
+                if exc.args[0] in hosed_connection:
+                    self._forceReconnection(reason=exc.args[0])
+                else:
+                    raise
+
             self._transaction_begun = True
-            self.db.ping()
             if self._transactions:
                 self._query('BEGIN')
             if self._mysql_lock:
                 self._query("SELECT GET_LOCK('%s',0)" % self._mysql_lock)
-        except Exception:
-            LOG.error('exception during _begin', exc_info=True)
-            raise ConflictError
+        except Exception as exc:
+            LOG.error('Exception during _begin', exc_info=True)
+            raise ConflictError('Database error %s' % exc.args[0])
 
     def _finish(self, *ignored):
         """ Commit a transaction, if transactions are enabled and the

src/Products/ZMySQLDA/tests/dummy.py

@@ -12,6 +12,9 @@
 ##############################################################################
 """ Dummy fixtures for testing
 """
+from MySQLdb import OperationalError
+
+
 RESULTS = {'show table status': [['table1', 'engine1', None, None, 5, None,
                                   None, None, None, None, None, None, None,
                                   None, 'my_collation']],
@@ -50,12 +53,14 @@ def __init__(self, **kw):
         self.last_query = None
         self.string_literal_called = False
         self.unicode_literal_called = False
+        self.ping_raises = False
 
         for k, v in kw.items():
             setattr(self, k, v)
 
     def ping(self, *args):
-        pass
+        if self.ping_raises:
+            raise OperationalError(self.ping_raises)
 
     def query(self, sql):
         self.last_query = sql

src/Products/ZMySQLDA/tests/test_db.py

@@ -15,6 +15,11 @@
 import unittest
 from _thread import get_ident
 
+from MySQLdb.constants.CR import SERVER_GONE_ERROR
+from MySQLdb.constants.CR import SERVER_HANDSHAKE_ERR
+
+from ZODB.POSException import ConflictError
+
 from .base import DB_CONN_STRING
 from .base import DB_PASSWORD
 from .base import DB_USER
@@ -374,6 +379,23 @@ def test__begin_transactions(self):
         self.assertTrue(db._transaction_begun)
         self.assertEqual(db.db.last_query, 'BEGIN')
 
+    def test__begin_raises(self):
+        # Starting with mysqlclient version 2.2.1 the connection object's
+        # ``ping`` method behavior changed and it may raise an exception.
+        db = self._makeOne(kw_args={})
+        db._transactions = True
+
+        # Use a type of exception that may be recoverable
+        db.db.ping_raises = SERVER_GONE_ERROR
+        db._begin()
+        self.assertTrue(db._transaction_begun)
+        self.assertEqual(db.db.last_query, 'BEGIN')
+
+        # Unrecoverable exception will be changed to a Zope transaction
+        # ConflictError
+        db.db.ping_raises = SERVER_HANDSHAKE_ERR
+        self.assertRaises(ConflictError, db._begin)
+
     def test__begin_mysql_lock(self):
         db = self._makeOne(kw_args={})
         db._mysql_lock = 'foo_lock'