launchpad-reviewers team mailing list archive

[Colin Watson <mp+469246@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/python-pgbouncer/pgbouncer-1.23.0 into lp:python-pgbouncer.

Commit message:
Deal with signal handling change in pgbouncer 1.23.0.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/python-pgbouncer/pgbouncer-1.23.0/+merge/469246

pgbouncer 1.23.0 (https://www.pgbouncer.org/changelog.html#pgbouncer-123x) changed its handling of SIGTERM in a way that causes Storm's test suite to fail, and this fixture now needs to use SIGQUIT instead; but it needs to keep using SIGTERM for older versions.  Detect the pgbouncer version and use whichever strategy is appropriate.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/python-pgbouncer/pgbouncer-1.23.0 into lp:python-pgbouncer.

=== modified file 'NEWS.txt'
--- NEWS.txt	2019-11-03 00:47:02 +0000
+++ NEWS.txt	2024-07-11 21:16:41 +0000
@@ -2,6 +2,7 @@
 ======
 
 - Officially add support for Python 3.6, 3.7, and 3.8.
+- Deal with signal handling change in pgbouncer 1.23.0.
 
 0.0.9 (2019-06-11)
 ==================

=== modified file 'pgbouncer/fixture.py'
--- pgbouncer/fixture.py	2019-06-07 15:29:03 +0000
+++ pgbouncer/fixture.py	2024-07-11 21:16:41 +0000
@@ -20,11 +20,13 @@
 
 import itertools
 import os.path
+import signal
 import socket
 import subprocess
 import time
 
 from fixtures import Fixture, TempDir
+import psycopg2
 from testtools.content import content_from_file
 
 
@@ -86,13 +88,18 @@
         self.databases = {}
         # username -> details
         self.users = {}
-        # list of usernames that can all console queries
+        # list of usernames that can run all console queries
         self.admin_users = []
         # list of usernames that can run readonly console queries
         self.stats_users = []
         self.pool_mode = 'session'
         self.unix_socket_dir = None
         self.process = None
+        # Username and password that the fixture itself uses to run console
+        # queries.  Since this fixture is only for testing, there's no harm
+        # in hardcoding these.
+        self.fixture_admin_username = '_pgbouncerfixture'
+        self.fixture_admin_password = 'trusted'
 
     def setUp(self):
         super(PGBouncerFixture, self).setUp()
@@ -126,11 +133,15 @@
             inifile.write('auth_file = %s\n' % (self.authpath,))
             inifile.write('logfile = %s\n' % (self.logpath,))
             inifile.write('pidfile = %s\n' % (self.pidpath,))
-            adminusers = ','.join(self.admin_users)
+            adminusers = ','.join(
+                [self.fixture_admin_username] + self.admin_users)
             inifile.write('admin_users = %s\n' % (adminusers,))
             statsusers = ','.join(self.stats_users)
             inifile.write('stats_users = %s\n' % (statsusers,))
         with open(self.authpath, 'wt') as authfile:
+            authfile.write(
+                '"%s" "%s"\n' %
+                (self.fixture_admin_username, self.fixture_admin_password))
             for user_creds in self.users.items():
                 authfile.write('"%s" "%s"\n' % user_creds)
 
@@ -142,10 +153,24 @@
             # pgbouncer has not yet exited.
             self.process.poll() is None)
 
+    def get_pgbouncer_version(self):
+        assert self.is_running
+        with psycopg2.connect(
+            host=self.host,
+            port=self.port,
+            database="pgbouncer",
+            user=self.fixture_admin_username,
+            password=self.fixture_admin_password,
+        ) as con:
+            return con.server_version
+
     def stop(self):
         if not self.is_running:
             return
-        self.process.terminate()
+        if self.get_pgbouncer_version() >= 12300:
+            self.process.send_signal(signal.SIGQUIT)
+        else:
+            self.process.terminate()
         for iteration in countdown():
             if self.process.poll() is not None:
                 break

=== modified file 'pgbouncer/tests.py'
--- pgbouncer/tests.py	2019-06-07 14:55:16 +0000
+++ pgbouncer/tests.py	2024-07-11 21:16:41 +0000
@@ -68,6 +68,15 @@
         self.bouncer.start()
         self.connect().close()
 
+    def test_stop_with_client_connected(self):
+        # Stopping the fixture stops pgbouncer even if clients are connected.
+        self.useFixture(self.bouncer)
+        connection = self.connect()
+        try:
+            self.bouncer.stop()
+        finally:
+            connection.close()
+
     def test_unix_sockets(self):
         unix_socket_dir = self.useFixture(fixtures.TempDir()).path
         self.bouncer.unix_socket_dir = unix_socket_dir