You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by el...@apache.org on 2026/07/26 19:19:20 UTC

(superset) branch fix/pkg-resources-redshift-warning-suppression created (now 5ae1bf4dd2e)

This is an automated email from the ASF dual-hosted git repository.

eschutho pushed a change to branch fix/pkg-resources-redshift-warning-suppression
in repository https://gitbox.apache.org/repos/asf/superset.git


      at 5ae1bf4dd2e fix: scope pkg_resources warning filters and add regression coverage

This branch includes the following new commits:

     new 5ae1bf4dd2e fix: scope pkg_resources warning filters and add regression coverage

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(superset) 01/01: fix: scope pkg_resources warning filters and add regression coverage

Posted by el...@apache.org.
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 <es...@gmail.com>
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"