alex-poor commented on code in PR #40679: URL: https://github.com/apache/superset/pull/40679#discussion_r4000594334
########## 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: Fixed in a53c03c — confirmed locally, `Locale.parse('zh_TW')` stringifies as `zh_Hant_TW` and `zh_CN` as `zh_Hans_CN`, both shipped Superset locales, so this silently broke every Traditional and Simplified Chinese lookup. Took the first option: the resolved locale is mapped back to the `LANGUAGES` key that names it, so the hook is handed the identifier the deployment configured and the language picker shows. Matching is by canonicalizing each configured key and comparing, rather than string munging, so it holds for any locale Babel rewrites. When nothing in `LANGUAGES` resolves to the active locale the canonical form is passed through unchanged, and a malformed key is skipped rather than failing the lookup. Two tests cover both directions. ########## 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: Fixed in a53c03c — both halves. `localized_name` is now declared on `ExploreContextSchema.SliceSchema` (marshmallow was dropping it on dump, so it never reached the response at all), and the spec is regenerated accordingly. `resolveSnapshotCharts` now carries it through the rebuild, with a test: the charts it reconstructs are precisely the ones missing from the live dashboard, so those were the only ones showing canonical names in a preview -- a difference that would have looked like the translation randomly not applying. ########## 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: Fixed in a53c03c — you're right, and the examples I chose were the misleading part. Verified the macro's only binding is `JinjaTemplateProcessor`, so it reaches strings Superset renders server-side as part of a query. Documented the real scope rather than adding the rendering path: the section now names virtual dataset SQL, calculated columns, metric expressions and SQL Lab queries, and carries a caution that it does **not** reach labels the frontend renders from stored metadata -- axis labels and native filter names specifically -- where the macro would display literally. The surfaces table says "SQL-templated content" rather than "templated fields". Rendering metadata through the hook on the frontend is a feature in its own right and seems worth its own change rather than being smuggled into this one. ########## 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: Fixed in a53c03c — good catch, and the failure mode is worse than a lost translation: the missing-table case is exactly the rollout window this example is most likely to be exercised in, and the canonical fallback it advertises would have turned into a 500 on the *next* query rather than a visible error here. Both hooks now wrap their query in `db.session.begin_nested()`, so a failure unwinds to the SAVEPOINT and leaves the shared session usable. Scoped that way rather than a bare `rollback()` on purpose -- a plain rollback would discard whatever the caller had pending on that session. -- 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]
