This is an automated email from the ASF dual-hosted git repository. eschutho pushed a commit to branch fix/pkg-resources-redshift-warning-suppression in repository https://gitbox.apache.org/repos/asf/superset.git
commit 5ae1bf4dd2e80ebba12e6a4d82100516620fe70f Author: Elizabeth Thompson <[email protected]> AuthorDate: Sun Jul 26 18:57:49 2026 +0000 fix: scope pkg_resources warning filters and add regression coverage Per richardfogaca's review: the unconditional filters matched only the warning message, so they suppressed the same setuptools pkg_resources warning from any dependency, not just sqlalchemy-redshift. Scopes each filter with category=UserWarning and module=sqlalchemy_redshift(?:\..*)? across all four registration points (db_engine_specs/redshift.py, initialization/__init__.py, mcp_service/__init__.py and server.py). Adds a regression test proving SupersetAppInitializer.configure_logging() installs the filter before LOGGING_CONFIGURATOR.configure_logging() dispatches, and tightens the existing mcp_service assertion to check the new category/module scoping. The server.py registration is intentionally kept (not removed as suggested) with a comment explaining why: pytest's warnings plugin resets warnings.filters around every test, and the existing test_suppress_third_party_warnings test fails without this line if removed, proving the reset scenario is real. --- superset/db_engine_specs/redshift.py | 7 +++++ superset/initialization/__init__.py | 2 ++ superset/mcp_service/__init__.py | 2 ++ superset/mcp_service/server.py | 10 ++++-- tests/unit_tests/initialization_test.py | 42 +++++++++++++++++++++++++ tests/unit_tests/mcp_service/test_mcp_server.py | 12 +++++-- 6 files changed, 70 insertions(+), 5 deletions(-) diff --git a/superset/db_engine_specs/redshift.py b/superset/db_engine_specs/redshift.py index fe11d8b1c21..c4c266ab6e8 100644 --- a/superset/db_engine_specs/redshift.py +++ b/superset/db_engine_specs/redshift.py @@ -47,6 +47,11 @@ from superset.utils import json # UserWarning, not DeprecationWarning -- don't add category=DeprecationWarning # here, it would silently stop matching. # +# Scoped to the sqlalchemy_redshift module (via stacklevel=2 in setuptools' +# own warn() call, the warning is attributed to whatever imports +# pkg_resources, i.e. sqlalchemy_redshift/__init__.py) so this doesn't also +# swallow the same deprecation warning from unrelated dependencies. +# # The same filter is also registered unconditionally in # SupersetAppInitializer.configure_logging() (superset/initialization/ # __init__.py), before it dispatches to the (deployment-replaceable) @@ -58,6 +63,8 @@ from superset.utils import json warnings.filterwarnings( "ignore", message=r"pkg_resources is deprecated as an API", + category=UserWarning, + module=r"sqlalchemy_redshift(?:\..*)?", ) logger = logging.getLogger() diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 91995c9ec67..b371db6bb04 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -1312,6 +1312,8 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods warnings.filterwarnings( "ignore", message=r"pkg_resources is deprecated as an API", + category=UserWarning, + module=r"sqlalchemy_redshift(?:\..*)?", ) self.config["LOGGING_CONFIGURATOR"].configure_logging( diff --git a/superset/mcp_service/__init__.py b/superset/mcp_service/__init__.py index db34fa04e52..419ed13fe49 100644 --- a/superset/mcp_service/__init__.py +++ b/superset/mcp_service/__init__.py @@ -55,6 +55,8 @@ warnings.filterwarnings( warnings.filterwarnings( "ignore", message=r"pkg_resources is deprecated as an API", + category=UserWarning, + module=r"sqlalchemy_redshift(?:\..*)?", ) __version__ = "1.0.0" diff --git a/superset/mcp_service/server.py b/superset/mcp_service/server.py index cf0b52f6c3e..22600634152 100644 --- a/superset/mcp_service/server.py +++ b/superset/mcp_service/server.py @@ -85,11 +85,17 @@ def _suppress_third_party_warnings() -> None: message=r"authlib\.jose module is deprecated", ) # Same treatment for the pkg_resources warning suppressed at package - # init time — covers late imports triggered by a Redshift-backed - # database connection opened after tool execution begins. + # init time. Confirmed non-redundant: warnings.filters can be reset + # between the package import and this call (e.g. pytest's warnings + # plugin resets it around every test -- test_suppress_third_party_warnings + # below fails without this line, proving the reset scenario is real, + # not hypothetical), so re-registering here is load-bearing, not + # belt-and-suspenders. warnings.filterwarnings( "ignore", message=r"pkg_resources is deprecated as an API", + category=UserWarning, + module=r"sqlalchemy_redshift(?:\..*)?", ) diff --git a/tests/unit_tests/initialization_test.py b/tests/unit_tests/initialization_test.py index e3e6d24c6e3..97e2e67910f 100644 --- a/tests/unit_tests/initialization_test.py +++ b/tests/unit_tests/initialization_test.py @@ -225,6 +225,48 @@ class TestSupersetAppInitializer: assert "secretpass" not in output assert "postgresql://user:***@localhost:5432/db" in output + @patch("superset.initialization.logger") + def test_configure_logging_installs_pkg_resources_filter_before_configurator( + self, mock_logger + ) -> None: + """The pkg_resources warning filter must be installed before + LOGGING_CONFIGURATOR.configure_logging() dispatches, so a deployment's + custom configurator (which may skip DefaultLoggingConfigurator's own + filter) still benefits from it.""" + import re + import warnings + + def has_pkg_resources_filter() -> bool: + return any( + f[0] == "ignore" + and isinstance(f[1], re.Pattern) + and f[1].pattern == r"pkg_resources is deprecated as an API" + for f in warnings.filters + ) + + seen_during_dispatch = [] + + class RecordingConfigurator: + def configure_logging(self, app_config, debug_mode): + seen_during_dispatch.append(has_pkg_resources_filter()) + + mock_app = MagicMock() + mock_app.config = {"LOGGING_CONFIGURATOR": RecordingConfigurator()} + mock_app.debug = False + app_initializer = SupersetAppInitializer(mock_app) + + with warnings.catch_warnings(): + # Isolate from filters registered by other tests/import side effects. + warnings.resetwarnings() + assert not has_pkg_resources_filter() + + app_initializer.configure_logging() + + assert seen_during_dispatch == [True], ( + "pkg_resources filter must already be installed by the time " + "LOGGING_CONFIGURATOR.configure_logging() runs" + ) + def test_check_and_warn_database_connection_invalid_uri(self) -> None: """Test that invalid URIs are handled safely without crashing.""" mock_app = MagicMock() diff --git a/tests/unit_tests/mcp_service/test_mcp_server.py b/tests/unit_tests/mcp_service/test_mcp_server.py index c2f8d81b938..6f30db5e41e 100644 --- a/tests/unit_tests/mcp_service/test_mcp_server.py +++ b/tests/unit_tests/mcp_service/test_mcp_server.py @@ -144,15 +144,21 @@ def test_suppress_third_party_warnings(): ] assert len(google_filters) >= 1, "Expected google FutureWarning filter" - # Verify pkg_resources UserWarning filter is installed (sqlalchemy-redshift - # triggers this via a late import on Redshift-backed connections; see - # superset/db_engine_specs/redshift.py for the full rationale) + # Verify pkg_resources UserWarning filter is installed, scoped to + # sqlalchemy_redshift (sqlalchemy-redshift triggers this via a late + # import on Redshift-backed connections; see + # superset/db_engine_specs/redshift.py for the full rationale). Scoping + # by category+module keeps this from also swallowing the same + # deprecation message from unrelated dependencies. pkg_resources_filters = [ f for f in warnings.filters if f[0] == "ignore" + and f[2] is UserWarning and isinstance(f[1], re.Pattern) and f[1].pattern == r"pkg_resources is deprecated as an API" + and isinstance(f[3], re.Pattern) + and f[3].pattern == r"sqlalchemy_redshift(?:\..*)?" ] assert len(pkg_resources_filters) >= 1, "Expected pkg_resources warning filter"
