Consolidate SQLAlchemy connection pool option handling

Pool configuration parameters are now passed directly within the database 'options' block and parsed by get_engine() rather than store_db_parameters().

This streamlines internal processing and formalizes connection pool settings within the pygeoapi configuration schema. Updated tests reflect this shift in responsibility.

Author: KoalaGeo

pygeoapi/process/manager/postgresql.py

@@ -95,7 +95,6 @@ def __init__(self, manager_def: dict):
             self.db_user,
             self._db_password,
             self.db_conn,
-            self.db_pool_options,
             **self.db_options
         )
         self.table_output = self.output_dir is None
@@ -340,4 +339,4 @@ def get_table_model(
 
     Base.metadata.create_all(engine, tables=[Jobs], checkfirst=True)
 
-    return Jobs
+    return Jobs

pygeoapi/provider/sql.py

@@ -134,7 +134,6 @@ def __init__(
             self.db_user,
             self._db_password,
             self.db_conn,
-            self.db_pool_options,
             **self.db_options
         )
         self.table_model = get_table_model(
@@ -616,31 +615,32 @@ def store_db_parameters(
         connection_data.get('search_path') or
         options.pop('search_path', ['public'])
     )
-    # Connection-pool tuning. These are popped out of ``options`` so they
-    # are NOT passed to the DBAPI as connect_args, and are coerced to a
-    # hashable form so get_engine() can stay functools.cache()-able.
-    # Defaults keep SQLAlchemy's QueuePool sizing but, unlike SQLAlchemy's
-    # default of -1, recycle connections after an hour so that pooled
-    # connections cannot sit IDLE on the server indefinitely.
-    pool_defaults = {
-        'pool_size': 5,
-        'max_overflow': 10,
-        'pool_recycle': -1,   # SQLAlchemy default; preserves current behaviour
-        'pool_timeout': 30,
-        'pool_pre_ping': True,
-    }
-    self.db_pool_options = tuple(sorted(
-        (key, type(default)(options.pop(key, default)))
-        for key, default in pool_defaults.items()
-    ))
-
+    # Connection-pool tuning keys (pool_size, max_overflow, pool_recycle,
+    # pool_timeout, pool_pre_ping) are intentionally left in ``options`` and
+    # flow through ``db_options`` to get_engine(), which separates them from
+    # the DBAPI connect_args. Their types are validated by the config JSON
+    # Schema, so no coercion is performed here.
     self.db_options = {
         k: v
         for k, v in options.items()
         if not isinstance(v, dict)
     }
 
 
+#: Connection-pool tuning keys recognised by get_engine(). These configure
+#: SQLAlchemy's QueuePool rather than the DBAPI, so get_engine() separates
+#: them from connect_args. The defaults reproduce pygeoapi's previous
+#: behaviour exactly: SQLAlchemy's own QueuePool defaults, except for
+#: pool_pre_ping, which was previously hardcoded to True.
+POOL_OPTION_DEFAULTS = {
+    'pool_size': 5,
+    'max_overflow': 10,
+    'pool_recycle': -1,   # SQLAlchemy default; never recycles connections
+    'pool_timeout': 30,
+    'pool_pre_ping': True,
+}
+
+
 @functools.cache
 def get_engine(
     driver_name: str,
@@ -650,7 +650,6 @@ def get_engine(
     user: str,
     password: str,
     conn_str: Optional[str] = None,
-    pool_options: tuple[tuple[str, Any], ...] = (),
     **connect_args
 ) -> Engine:
     """
@@ -663,12 +662,11 @@ def get_engine(
     :param user: database user
     :param password: database password
     :param conn_str: optional connection URL
-    :param pool_options: hashable tuple of (key, value) pairs controlling
-                         the connection pool (pool_size, max_overflow,
-                         pool_recycle, pool_timeout, pool_pre_ping). Passed
-                         as a tuple rather than a dict so this function can
-                         remain functools.cache()-able.
-    :param connect_args: custom connection arguments to pass to create_engine()
+    :param connect_args: keyword arguments forwarded from the provider's
+                         ``options`` block. Connection-pool tuning keys (see
+                         POOL_OPTION_DEFAULTS) are extracted and applied to
+                         the engine's pool; any remaining keys are passed to
+                         the DBAPI as connect_args.
 
     :returns: SQL Alchemy engine
     """
@@ -682,15 +680,26 @@ def get_engine(
             database=database
         )
 
+    # Separate connection-pool tuning from DBAPI connect args. Pool keys are
+    # applied to create_engine() directly; everything left in connect_args is
+    # forwarded to the DBAPI. get_engine() stays functools.cache()-able
+    # because connect_args values are hashable scalars, so engine sharing per
+    # process is preserved; providers with differing pool config (or any
+    # other option) correctly get distinct engines.
+    pool_options = {
+        key: connect_args.pop(key, default)
+        for key, default in POOL_OPTION_DEFAULTS.items()
+    }
+
     engine = create_engine(
         conn_str,
         connect_args=connect_args,
-        **dict(pool_options)
+        **pool_options
     )
 
     LOGGER.debug(
         f'Created engine for {repr(engine.url)} '
-        f'with pool options {dict(pool_options)}.'
+        f'with pool options {pool_options}.'
     )
 
     return engine
@@ -927,4 +936,4 @@ def get(self, identifier, crs_transform_spec=None, **kwargs):
                 else feature_id
             )
 
-        return feature
+        return feature

pygeoapi/resources/schemas/config/pygeoapi-config-0.x.yml

@@ -734,8 +734,23 @@ definitions:
                                         type: integer
                                     keepalives_interval:
                                         type: integer
+                                    pool_size:
+                                        type: integer
+                                        description: persistent connections held open per worker process
+                                    max_overflow:
+                                        type: integer
+                                        description: extra short-lived connections allowed above pool_size
+                                    pool_recycle:
+                                        type: integer
+                                        description: recreate connections older than this many seconds (-1 never recycles)
+                                    pool_timeout:
+                                        type: integer
+                                        description: seconds to wait for a connection from the pool before erroring
+                                    pool_pre_ping:
+                                        type: boolean
+                                        description: test connections with a lightweight ping before use
 required:
     - server
     - logging
     - metadata
-    - resources
+    - resources

tests/provider/test_sql_pool_options.py

@@ -1,99 +1,29 @@
 # =================================================================
-# Tests for configurable SQLAlchemy connection-pool options on the
-# SQL provider. These exercise store_db_parameters() directly and do
-# not require a live database, so they run in standard CI.
+# Test that get_engine() separates SQLAlchemy connection-pool tuning
+# options from DBAPI connect_args. This is the contract introduced by
+# the configurable-pool change; it needs no live database.
 # =================================================================
 
-import pytest
+from unittest import mock
 
-from pygeoapi.provider.sql import store_db_parameters
+from pygeoapi.provider import sql
 
 
-class _Dummy:
-    """Minimal stand-in for a provider/manager instance."""
-    default_port = 5432
-
-
-CONN = {'host': 'h', 'dbname': 'd', 'user': 'u', 'password': 'p'}
-
-
-def test_pool_options_defaults_preserve_current_behaviour():
-    obj = _Dummy()
-    store_db_parameters(obj, dict(CONN), {})
-    pool = dict(obj.db_pool_options)
-    # Defaults must match pre-existing effective behaviour:
-    # pool_pre_ping was hardcoded True; pool_recycle was unset (-1).
-    assert pool['pool_size'] == 5
-    assert pool['max_overflow'] == 10
-    assert pool['pool_timeout'] == 30
-    assert pool['pool_pre_ping'] is True
-    assert pool['pool_recycle'] == -1
-
-
-def test_pool_options_are_overridable_and_typed():
-    obj = _Dummy()
-    store_db_parameters(
-        obj, dict(CONN),
-        {'pool_size': 2, 'max_overflow': 3, 'pool_recycle': 300},
+@mock.patch.object(sql, 'create_engine')
+def test_get_engine_separates_pool_options_from_connect_args(mock_create):
+    sql.get_engine.cache_clear()
+    sql.get_engine(
+        'postgresql+psycopg2', 'h', 5432, 'd', 'u', 'p', None,
+        pool_size=2, pool_recycle=300, connect_timeout=10,
     )
-    pool = dict(obj.db_pool_options)
-    assert pool['pool_size'] == 2 and isinstance(pool['pool_size'], int)
-    assert pool['max_overflow'] == 3
-    assert pool['pool_recycle'] == 300
-    # untouched keys keep defaults
-    assert pool['pool_timeout'] == 30
-    assert pool['pool_pre_ping'] is True
-
-
-def test_pool_options_not_leaked_to_dbapi_connect_args():
-    obj = _Dummy()
-    store_db_parameters(
-        obj, dict(CONN),
-        {'connect_timeout': 10, 'pool_size': 2, 'pool_recycle': 300},
-    )
-    for k in ('pool_size', 'max_overflow', 'pool_recycle',
-              'pool_timeout', 'pool_pre_ping'):
-        assert k not in obj.db_options
-    # genuine DBAPI connect args still pass through
-    assert obj.db_options['connect_timeout'] == 10
-
-
-def test_dict_valued_options_still_filtered():
-    obj = _Dummy()
-    store_db_parameters(
-        obj, dict(CONN),
-        {'pool_size': 2, 'zoom': {'min': 0, 'max': 22}},
-    )
-    assert 'zoom' not in obj.db_options
-    assert dict(obj.db_pool_options)['pool_size'] == 2
-
-
-def test_pool_options_hashable_and_deterministic():
-    a, b = _Dummy(), _Dummy()
-    store_db_parameters(a, dict(CONN), {'pool_size': 2})
-    store_db_parameters(b, dict(CONN), {'pool_size': 2})
-    # identical config -> identical key -> shared engine via functools.cache
-    assert a.db_pool_options == b.db_pool_options
-    assert hash(a.db_pool_options) == hash(b.db_pool_options)
-
-    c = _Dummy()
-    store_db_parameters(c, dict(CONN), {'pool_size': 9})
-    # differing pool config -> distinct key (separate engine, by design)
-    assert c.db_pool_options != a.db_pool_options
-
-
-def test_pool_options_coexist_with_search_path():
-    obj = _Dummy()
-    store_db_parameters(
-        obj, dict(CONN),
-        {'search_path': ['published', 'public'], 'pool_size': 4},
-    )
-    assert obj.db_search_path == ('published', 'public')
-    assert dict(obj.db_pool_options)['pool_size'] == 4
-
 
-@pytest.mark.parametrize('bad', [{'pool_size': 'two'}])
-def test_non_integer_pool_value_raises(bad):
-    # type coercion surfaces bad config loudly rather than silently
-    with pytest.raises(ValueError):
-        store_db_parameters(_Dummy(), dict(CONN), bad)
+    _, kwargs = mock_create.call_args
+    # pool keys are applied to the engine (QueuePool), with overrides
+    # honoured and unset pool keys falling back to the documented defaults
+    assert kwargs['pool_size'] == 2
+    assert kwargs['pool_recycle'] == 300
+    assert kwargs['max_overflow'] == 10
+    assert kwargs['pool_timeout'] == 30
+    assert kwargs['pool_pre_ping'] is True
+    # genuine DBAPI args are forwarded via connect_args; pool keys are not
+    assert kwargs['connect_args'] == {'connect_timeout': 10}