Compare commits

...

4 Commits

Author SHA1 Message Date
Elizabeth Thompson
5ae1bf4dd2 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.
2026-07-26 18:57:49 +00:00
Elizabeth Thompson
3fd3db5237 chore: apply ruff 0.9.7 formatting to test assertion
Aligns with CI's pinned ruff version; no behavior change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-22 00:33:54 +00:00
Elizabeth Thompson
d36a6801b2 fix: register pkg_resources filter ahead of LOGGING_CONFIGURATOR dispatch 2026-07-11 16:37:10 +00:00
Elizabeth Thompson
16db953533 fix: close gaps in pkg_resources/sqlalchemy-redshift warning suppression
The pkg_resources UserWarning from sqlalchemy-redshift's __init__.py
(#36082) was already suppressed in db_engine_specs/redshift.py, but
production logs still show ~76/day of it firing:

- mcp_service runs as a separate process and never imports
  db_engine_specs/redshift.py, so it had no filter for this warning at
  all (unlike its sibling authlib.jose filter, which is present in both
  __init__.py and server.py). Added it in both places, mirroring the
  existing authlib.jose pattern exactly.
- The main web/celery filter only registers the first time something
  accesses Database.db_engine_spec, which isn't guaranteed to be the
  first thing to run in every process. Registered the same filter
  unconditionally in DefaultLoggingConfigurator.configure_logging(),
  which runs first in SupersetAppInitializer.init_app(), as a more
  robust primary suppression point. The existing redshift.py filter is
  left in place as a fallback for standalone scripts.
2026-07-10 16:19:55 +00:00
6 changed files with 118 additions and 0 deletions

View File

@@ -46,9 +46,25 @@ from superset.utils import json
# Setuptools 80.x (pinned in requirements/base.txt) raises this as a plain
# 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)
# LOGGING_CONFIGURATOR. That's now the primary suppression point for the
# web app and celery workers; this one remains as a fallback for standalone
# scripts that import this module without going through create_app()
# (filterwarnings() calls are idempotent, so registering it twice is
# harmless).
warnings.filterwarnings(
"ignore",
message=r"pkg_resources is deprecated as an API",
category=UserWarning,
module=r"sqlalchemy_redshift(?:\..*)?",
)
logger = logging.getLogger()

View File

@@ -20,6 +20,7 @@ import contextlib
import logging
import os
import sys
import warnings
from typing import Any, Callable, TYPE_CHECKING
import wtforms_json
@@ -1301,6 +1302,20 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods
)
def configure_logging(self) -> None:
# sqlalchemy-redshift's own __init__ still imports pkg_resources (see
# superset/db_engine_specs/redshift.py for the full rationale). This
# filter used to live in DefaultLoggingConfigurator.configure_logging(),
# but LOGGING_CONFIGURATOR is a deployment-replaceable hook -- any
# custom configurator skipped it entirely and still hit the warning.
# Registering it here, before LOGGING_CONFIGURATOR runs, guarantees
# it's installed regardless of which configurator is configured.
warnings.filterwarnings(
"ignore",
message=r"pkg_resources is deprecated as an API",
category=UserWarning,
module=r"sqlalchemy_redshift(?:\..*)?",
)
self.config["LOGGING_CONFIGURATOR"].configure_logging(
self.config, self.superset_app.debug
)

View File

@@ -47,6 +47,18 @@ warnings.filterwarnings(
message=r"authlib\.jose module is deprecated",
)
# sqlalchemy-redshift's own __init__ still imports pkg_resources (see the
# equivalent filter and rationale in superset/db_engine_specs/redshift.py).
# MCP tools that touch a Redshift-backed database trigger this import via a
# separate process from the main web/worker app, so it needs its own filter
# here rather than relying on db_engine_specs/redshift.py having been loaded.
warnings.filterwarnings(
"ignore",
message=r"pkg_resources is deprecated as an API",
category=UserWarning,
module=r"sqlalchemy_redshift(?:\..*)?",
)
__version__ = "1.0.0"
# Tools are auto-registered when imported by the MCP service

View File

@@ -62,6 +62,8 @@ def _suppress_third_party_warnings() -> None:
- marshmallow ``RemovedInMarshmallow4Warning`` (triggered during
database engine schema instantiation)
- google.api_core ``FutureWarning`` (Python version support notices)
- sqlalchemy-redshift ``pkg_resources`` UserWarning (see
superset/db_engine_specs/redshift.py for details)
"""
import warnings
@@ -82,6 +84,19 @@ def _suppress_third_party_warnings() -> None:
"ignore",
message=r"authlib\.jose module is deprecated",
)
# Same treatment for the pkg_resources warning suppressed at package
# 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(?:\..*)?",
)
class FastMCPValidationFilter(logging.Filter):

View File

@@ -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()

View File

@@ -144,6 +144,24 @@ def test_suppress_third_party_warnings():
]
assert len(google_filters) >= 1, "Expected google FutureWarning filter"
# 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"
def test_create_event_store_returns_none_when_redis_store_fails():
"""EventStore returns None when Redis store creation fails."""