sushu0 commented on code in PR #45790: URL: https://github.com/apache/superset/pull/45790#discussion_r4408415848
########## superset/migrations/versions/2026-10-27_00-00_9670ab7f6bc6_normalize_metadata_cache_timeouts.py: ########## @@ -0,0 +1,115 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Normalize legacy database metadata cache timeouts. + +Older database forms stored metadata cache timeouts as strings. Convert only +ASCII digit strings, preserving malformed configurations +and unrelated fields instead of guessing at their intended values. + +Revision ID: 9670ab7f6bc6 +Revises: f72b6e9c104a +Create Date: 2026-10-27 00:00:00.000000 +""" + +import logging +from decimal import InvalidOperation + +from alembic import op +from sqlalchemy import column, Integer, select, table, Text + +from superset.utils import json + +revision = "9670ab7f6bc6" +down_revision = "f72b6e9c104a" + +logger = logging.getLogger(__name__) + +_DATABASES = table("dbs", column("id", Integer), column("extra", Text)) +_TIMEOUT_KEYS = ( + "catalog_cache_timeout", + "schema_cache_timeout", + "table_cache_timeout", +) + + +def upgrade() -> None: + """Convert legacy ASCII digit strings to integer timeouts.""" + connection = op.get_bind() + rows = connection.execute( + select(_DATABASES.c.id, _DATABASES.c.extra).with_for_update() + ).all() + for database_id, raw_extra in rows: + try: + extra = json.loads(raw_extra, use_decimal=True) + except (TypeError, ValueError, InvalidOperation): + logger.warning( + "Database %s: skipping metadata cache timeouts; " + "invalid or unrepresentable JSON", + database_id, + ) + continue + if not isinstance(extra, dict) or not isinstance( + extra.get("metadata_cache_timeout"), dict + ): + continue + timeouts = extra["metadata_cache_timeout"] + + changed = False + for key in _TIMEOUT_KEYS: + value = timeouts.get(key) + if not isinstance(value, str): + continue + if not (value.isascii() and value.isdigit()): + logger.warning( + "Database %s: leaving metadata_cache_timeout.%s unchanged; " + "expected an ASCII digit string", + database_id, + key, + ) + continue + try: + timeout = int(value) + except ValueError: + logger.warning( + "Database %s: leaving metadata_cache_timeout.%s unchanged; " + "cannot parse integer string", + database_id, + key, + ) + continue + timeouts[key] = timeout + changed = True + + if changed: + result = connection.execute( + _DATABASES.update() + .where(_DATABASES.c.id == database_id, _DATABASES.c.extra == raw_extra) + .values(extra=json.dumps(extra)) Review Comment: Addressed in b17d34f299a7418e958473a632b2999301bcd3ca. Changed rows now use compact separators and ASCII escapes. The actual new ASCII byte count is checked against the original UTF-8 byte count and a stricter original-character budget. The second bound is conservative capacity protection when the column charset is unknown; it is not used as a substitute for measuring bytes. Unsafe encoding or expansion leaves the entire original row unchanged with an ID-only static warning. I kept `ensure_ascii=True` rather than introducing literal Unicode with a fallback. This preserves escaped astral/surrogate values on narrower column charsets, at the cost of conservatively skipping some literal-Unicode rows that could fit a known UTF-8 column. The 14 added SQLite cases cover original 65,535-byte ASCII/Unicode documents, zero/nonzero timeouts, exact Decimal/unrelated-value round trips, escaped astral/surrogate preservation, Unicode/Decimal expansion skips and blank rows. Before the fix they had 12 failures / 2 passes, including an ASCII result growing to 72,345 bytes. The final migration/single-head/cache/JSON suite passed 113 tests; CAS and two-row rollback remain covered. These are explicit byte assertions over actual SQLite rows, not MySQL strict/non-strict upgrade execution. ########## superset/migrations/versions/2026-10-27_00-00_9670ab7f6bc6_normalize_metadata_cache_timeouts.py: ########## @@ -0,0 +1,115 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Normalize legacy database metadata cache timeouts. + +Older database forms stored metadata cache timeouts as strings. Convert only +ASCII digit strings, preserving malformed configurations +and unrelated fields instead of guessing at their intended values. + +Revision ID: 9670ab7f6bc6 +Revises: f72b6e9c104a +Create Date: 2026-10-27 00:00:00.000000 +""" + +import logging +from decimal import InvalidOperation + +from alembic import op +from sqlalchemy import column, Integer, select, table, Text + +from superset.utils import json + +revision = "9670ab7f6bc6" +down_revision = "f72b6e9c104a" + +logger = logging.getLogger(__name__) + +_DATABASES = table("dbs", column("id", Integer), column("extra", Text)) +_TIMEOUT_KEYS = ( + "catalog_cache_timeout", + "schema_cache_timeout", + "table_cache_timeout", +) + + +def upgrade() -> None: + """Convert legacy ASCII digit strings to integer timeouts.""" + connection = op.get_bind() + rows = connection.execute( + select(_DATABASES.c.id, _DATABASES.c.extra).with_for_update() + ).all() + for database_id, raw_extra in rows: + try: + extra = json.loads(raw_extra, use_decimal=True) Review Comment: Addressed in b17d34f299a7418e958473a632b2999301bcd3ca. SQL NULL and empty-string `extra` return before `json.loads`, without logging. I extracted the existing parsing/shape checks into a small helper to keep the migration within its complexity limit; nonblank invalid JSON keeps the original redacted warning. The two new caplog cases verify exact NULL/empty preservation and no migration warning. The genuine invalid/unrepresentable-JSON warning and redaction cases remain passing in the final 113-test migration/single-head/cache/JSON suite. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
