sadpandajoe commented on code in PR #40679:
URL: https://github.com/apache/superset/pull/40679#discussion_r3996158866
##########
superset/models/slice.py:
##########
@@ -347,7 +363,11 @@ def data(self) -> dict[str, Any]:
"extra_editors": get_extra_editor_subject_ids(self),
"viewers": [s.id for s in self.viewers],
"slice_id": self.id,
+ # ``slice_name`` stays canonical: the dashboard layout seeds
+ # ``meta.sliceName`` from it and persists it on save. The localized
+ # value is exposed separately for display only.
"slice_name": self.slice_name,
+ "localized_name": self.localized_name,
Review Comment:
`Slice.data` also feeds `/api/v1/explore`, but
`ExploreContextSchema.SliceSchema` still omits this field and
`resolveSnapshotCharts` fails to copy it when reconstructing charts absent from
the live dashboard. That leaves generated Explore clients out of sync and
version previews show canonical names for exactly those charts; could the
Explore schema and rehydration path carry `localized_name` through?
##########
examples/asset_metadata_translation/hook.py:
##########
@@ -0,0 +1,110 @@
+# 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.
+"""Reference translation hooks backed by the ``AssetTranslation`` table.
+
+NOT part of Superset core -- see this directory's README. Assign
+``translation_hook`` to ``TRANSLATION_HOOK`` in ``superset_config.py``, or
+``translation_batch_hook`` to ``TRANSLATION_BATCH_HOOK`` to resolve a whole
+collection in a single query (preferred for a table-backed store).
+"""
+
+from __future__ import annotations
+
+import logging
+from collections.abc import Sequence
+
+logger = logging.getLogger(__name__)
+
+
+def translation_hook(
+ default_text: str,
+ locale: str,
+ **kwargs: object,
+) -> str | None:
+ """Look up a stored translation for the active locale.
+
+ Matches on the source text plus the ``model_name``/``field_name`` context
+ Superset passes, so the same string can be translated differently per
field.
+ Returns ``None`` when there is no match (Superset falls back to the
canonical
+ text). Any failure is swallowed so rendering never breaks on a lookup
error.
+ """
+ # Local imports: these are only importable inside the running app context.
+ from superset import db
+
+ from .model import AssetTranslation
+
+ try:
+ row = (
+ db.session.query(AssetTranslation.translated_text)
+ .filter(
+ AssetTranslation.language_code == locale,
+ AssetTranslation.default_text == default_text,
+ AssetTranslation.model_name == kwargs.get("model_name", ""),
+ AssetTranslation.field_name == kwargs.get("field_name", ""),
+ )
+ .first()
+ )
+ except Exception: # pylint: disable=broad-except
+ logger.exception("asset translation lookup failed for %r",
default_text)
+ return None
+
+ return row[0] if row else None
+
+
+def translation_batch_hook(
+ default_texts: Sequence[str],
+ locale: str,
+ **kwargs: object,
+) -> dict[str, str]:
+ """Look up many stored translations in one query.
+
+ The batch counterpart to :func:`translation_hook`: Superset passes every
+ string it is about to render for one context -- all of a dashboard's chart
+ names, say -- so a table-backed store answers with a single ``IN`` query
+ instead of one per string. Strings with no stored translation are simply
+ absent from the result; Superset falls back to the canonical text.
+ """
+ # Local imports: these are only importable inside the running app context.
+ from superset import db
+
+ from .model import AssetTranslation
+
+ if not default_texts:
+ return {}
+
+ try:
+ rows = (
+ db.session.query(
+ AssetTranslation.default_text,
+ AssetTranslation.translated_text,
+ )
+ .filter(
+ AssetTranslation.language_code == locale,
+ AssetTranslation.default_text.in_(default_texts),
+ AssetTranslation.model_name == kwargs.get("model_name", ""),
+ AssetTranslation.field_name == kwargs.get("field_name", ""),
+ )
+ .all()
+ )
+ except Exception: # pylint: disable=broad-except
Review Comment:
This catches a failed lookup on Superset's shared scoped session without
ending PostgreSQL's failed transaction. If the example table is missing during
rollout, the hook returns `{}` but later chart serialization or access checks
hit `InFailedSqlTransaction`, so the promised canonical fallback still becomes
a 500; could the reference hook isolate its query transaction/session before
swallowing the error?
##########
superset/utils/i18n.py:
##########
@@ -0,0 +1,273 @@
+# 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.
+"""Localization of user-defined asset metadata (chart names, dashboard titles).
+
+This is distinct from UI-chrome translation (Flask-Babel / gettext), which
+covers static strings baked into the application. Here we resolve *data* that
+users author -- a chart called "Sales" should be able to display as "Ventes"
+for a French viewer -- by delegating to a deployment-provided
``TRANSLATION_HOOK``.
+
+Superset core intentionally does not store these translations itself; the hook
+abstracts where they live (a database table, an external translation service,
+a static mapping, ...), keeping core minimal and the feature pluggable.
+"""
+
+from __future__ import annotations
+
+import logging
+from collections.abc import Iterable, Mapping
+from typing import Any, Callable
+
+from flask import current_app as app, g, has_request_context
+from flask_babel import get_locale
+
+from superset.extensions import feature_flag_manager
+
+logger = logging.getLogger(__name__)
+
+#: Feature flag gating asset-metadata translation.
+FEATURE_FLAG = "ENABLE_I18N_ASSET_TRANSLATIONS"
+
+#: Attribute on ``flask.g`` holding the per-request resolution memo.
+_CACHE_ATTR = "_asset_translation_cache"
+
+#: Key identifying one resolution: locale + source text + hook context. The
+#: context is part of the key because the same string may resolve differently
+#: per field (a chart named "Sales" and a dashboard titled "Sales").
+_CacheKey = tuple[str, str, tuple[tuple[str, str], ...]]
+
+
+def is_asset_translation_enabled() -> bool:
+ """Whether asset-metadata translation should be attempted at all.
+
+ Gated on *both* conditions, mirroring the SIP-161 design:
+ 1. the ``ENABLE_I18N_ASSET_TRANSLATIONS`` feature flag is on, and
+ 2. more than one language is configured in ``LANGUAGES``.
+
+ The second condition means single-language deployments (the default) pay
+ zero cost: ``translate`` short-circuits before resolving the locale or
+ invoking the hook.
+ """
+ if not feature_flag_manager.is_feature_enabled(FEATURE_FLAG):
+ return False
+ return len(app.config.get("LANGUAGES") or {}) > 1
+
+
+def _target_locale() -> str | None:
+ """The locale to translate into, or ``None`` when there is nothing to do.
+
+ ``None`` means the active locale is unresolved or already the default the
+ canonical text is authored in, so the stored text is the correct answer.
+ """
+ locale = get_locale()
+ if locale is None:
+ return None
+
+ locale_str = str(locale)
Review Comment:
Babel canonicalizes the configured `zh_TW` locale to `zh_Hant_TW` here, so a
hook or translation table keyed by Superset's `LANGUAGES` entry misses every
Traditional Chinese translation and falls back to the canonical title. Could
this preserve the configured locale identifier, or explicitly normalize the
hook contract before invoking it?
##########
docs/admin_docs/configuration/asset-metadata-translation.mdx:
##########
@@ -0,0 +1,230 @@
+---
+title: Asset Metadata Translation
+hide_title: true
+sidebar_position: 16
+version: 1
+---
+
+# Asset Metadata Translation
+
+Superset's built-in internationalization (Flask-Babel / gettext) translates the
+application's **UI chrome** — buttons, menus, labels, error messages. It does
+**not** translate user-authored content such as chart names, dashboard titles,
+or axis/metric labels, because those are stored as data rather than as
+translatable source strings.
+
+This feature lets a deployment localize that user-authored metadata, so a chart
+named "Sales" can display as "Ventes" to a French viewer and "Hokohoko" to a
+Māori viewer — while the canonical stored name stays unchanged.
+
+:::info Background
+This implements the read path discussed in
+[SIP-161](https://github.com/apache/superset/issues/32854). Superset core does
+**not** store these translations. It calls a deployment-provided
+`TRANSLATION_HOOK` at render time; where the translations live and how they are
+authored is entirely up to the deployment (a static map, an external machine
+translation service, a database table, gettext `.po` catalogs, …). This keeps
+core minimal and the storage/authoring strategy pluggable.
+:::
+
+## Enabling
+
+Two conditions must both be true, otherwise translation is skipped entirely
+(single-language deployments pay zero cost):
+
+1. The `ENABLE_I18N_ASSET_TRANSLATIONS` [feature
flag](/admin-docs/configuration/configuring-superset#feature-flags)
+ is enabled.
+2. More than one language is configured in `LANGUAGES`.
+
+```python
+# superset_config.py
+FEATURE_FLAGS = {
+ "ENABLE_I18N_ASSET_TRANSLATIONS": True,
+}
+
+BABEL_DEFAULT_LOCALE = "en"
+LANGUAGES = {
+ "en": {"flag": "us", "name": "English"},
+ "fr": {"flag": "fr", "name": "French"},
+}
+```
+
+When enabled, the canonical text is always returned unchanged if:
+
+- the active locale is the default locale (`BABEL_DEFAULT_LOCALE`), or
+- neither `TRANSLATION_HOOK` nor `TRANSLATION_BATCH_HOOK` is configured, or
+- the hook returns a falsy value (no translation found), or
+- the hook raises (the error is logged; the original text is preserved).
+
+The canonical text is **always** a safe fallback, so a missing or broken
+translation never blanks out a chart or dashboard name.
+
+## The `TRANSLATION_HOOK`
+
+Define a callable in `superset_config.py`. It receives the default (canonical)
+text and the target locale, plus keyword context to disambiguate identical
+strings, and returns the translated text — or a falsy value to fall back.
+
+```python
+def TRANSLATION_HOOK(
+ default_text: str,
+ locale: str,
+ **kwargs, # e.g. model_name="Dashboard", field_name="dashboard_title"
+) -> str | None:
+ ...
+```
+
+:::tip
+Keep `**kwargs` last so future context can be added without breaking your
+implementation. Treat the hook as a pure, deterministic lookup where possible.
+:::
+
+## The `TRANSLATION_BATCH_HOOK`
+
+`TRANSLATION_HOOK` is called once per string, which is fine for an in-memory
+lookup but costly if every call is a query or a network round trip. Rendering a
+dashboard resolves every chart name on it; a page of recent activity resolves
+every entry.
+
+For those stores, define `TRANSLATION_BATCH_HOOK` instead. Superset hands it
+every string it is about to render for one context and expects a mapping back:
+
+```python
+def TRANSLATION_BATCH_HOOK(
+ default_texts: list[str],
+ locale: str,
+ **kwargs, # e.g. model_name="Slice", field_name="slice_name"
+) -> dict[str, str | None]:
+ ...
+```
+
+Keys that are missing from the returned mapping — or that map to a falsy value
+— fall back to the canonical text, so returning only the strings you actually
+have a translation for is fine.
+
+When set, this **fully replaces** `TRANSLATION_HOOK`: individual lookups are
+routed through it as a one-element batch, so there is no need to configure
+both. Superset also memoizes resolutions for the duration of a request, so a
+repeated string costs one lookup no matter how many times it is rendered.
+
+:::note
+Batching applies where Superset holds a whole collection before rendering it —
+the charts on a dashboard and the "Recents" list. On the chart and dashboard
+**list** pages each row is serialized independently, so those still resolve per
+row; back your hook with a cache or an in-process map if that path matters to
+you.
+:::
+
+## The `i18n` Jinja macro
+
+Templated fields (for example a chart axis label or a native filter name, when
Review Comment:
The only runtime binding added for `i18n()` is the server-side SQL
`JinjaTemplateProcessor`; chart axis labels and native-filter names are
rendered directly by the frontend, so following this example displays the
literal `{{ i18n(...) }}` text instead of a translation. Could this be
documented as SQL-template-only, or add the missing metadata rendering path?
--
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]
