alex-poor commented on code in PR #40679: URL: https://github.com/apache/superset/pull/40679#discussion_r3825129470
########## docs/admin_docs/configuration/asset-metadata-translation.mdx: ########## @@ -0,0 +1,228 @@ +--- +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 +- no `TRANSLATION_HOOK` is configured, or Review Comment: Fixed in d182815 — the bullet now reads "neither `TRANSLATION_HOOK` nor `TRANSLATION_BATCH_HOOK` is configured", and the Jinja macro section says it resolves through the configured hook, batch or single, rather than naming only `TRANSLATION_HOOK`. The batching note lower down was stale in the same way and now says the charts on a dashboard rather than "dashboard loads", since the batch point moved to the charts endpoint. ########## examples/asset_metadata_translation/README.md: ########## @@ -0,0 +1,65 @@ +<!-- +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: database-backed asset metadata translation + +This is a **self-contained reference**, not part of Superset core. It shows one +concrete way to implement end-to-end authoring on top of the +`TRANSLATION_HOOK` read path documented at +[Asset Metadata Translation](https://superset.apache.org/docs/configuration/asset-metadata-translation). + +Superset core intentionally ships only the read path (the hook + the Jinja +`i18n` macro). It does **not** provide storage or an authoring UI — those are +left to deployments so core stays minimal (per SIP-161 / the maintainer +direction). This example fills that gap for the common case: "I want editors to +enter translations and have them stored in the metadata database." + +## What it provides + +- `model.py` — a single `AssetTranslation` table keyed by + `(model_name, field_name, default_text, language_code)`. +- `hook.py` — a `translation_hook(default_text, locale, **kwargs)` that reads + that table, suitable for assigning to `TRANSLATION_HOOK`. +- `seed.py` — a small helper to populate translations programmatically (stand + in for, or grow into, a real authoring UI / CSV import / Transifex sync). + +## Why it lives outside core + +A production-grade version of this would be a proper +[Superset extension](https://superset.apache.org/docs/contributing/development) +(its own migration, CRUD API, and React authoring surface). That is a separate +project. This example deliberately stays minimal so it reads as documentation: +enough to wire up and demonstrate, not a supported component. + +## Usage sketch + +```python +# superset_config.py +from asset_metadata_translation.hook import translation_hook Review Comment: Fixed in d182815 — you're right, the example was unrunnable as written. The README now leads with the fact that `examples/` is not packaged and shows both ways to make it importable (copying the directory into the pythonpath dir, or putting the checkout's `examples/` on `PYTHONPATH`) before the config snippet. I left it as a documented step rather than packaging the directory, since the point of keeping this outside core was that it stays a documentation-grade reference rather than a shipped component. Say the word if you'd rather it were packaged. The snippet also now wires up `TRANSLATION_BATCH_HOOK`, which is the better fit for the table-backed store the example demonstrates. ########## tests/unit_tests/utils/i18n_test.py: ########## @@ -0,0 +1,400 @@ +# 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. +from __future__ import annotations + +from collections.abc import Iterator + +import pytest +from flask import current_app +from pytest_mock import MockerFixture + +from superset.utils import i18n + +MULTI_LANG = { + "en": {"flag": "us", "name": "English"}, + "fr": {"flag": "fr", "name": "French"}, +} + + [email protected](autouse=True) +def reset_hooks() -> Iterator[None]: + """Keep hook config from leaking between tests.""" + yield + current_app.config["TRANSLATION_HOOK"] = None + current_app.config["TRANSLATION_BATCH_HOOK"] = None + + [email protected] +def fr_locale(mocker: MockerFixture) -> None: + """Active locale resolves to French (non-default).""" + + class _Locale: + def __str__(self) -> str: + return "fr" + + mocker.patch.object(i18n, "get_locale", return_value=_Locale()) + + +def _enable(mocker: MockerFixture, enabled: bool = True) -> None: + mocker.patch.object( + i18n.feature_flag_manager, "is_feature_enabled", return_value=enabled + ) + + +def test_translate_returns_original_when_feature_disabled( + mocker: MockerFixture, fr_locale: None +) -> None: + _enable(mocker, enabled=False) + current_app.config["LANGUAGES"] = MULTI_LANG + current_app.config["TRANSLATION_HOOK"] = lambda *a, **k: "Ventes" + + assert i18n.translate("Sales") == "Sales" + + +def test_translate_returns_original_with_single_language( + mocker: MockerFixture, fr_locale: None +) -> None: + _enable(mocker) + current_app.config["LANGUAGES"] = {"en": {"flag": "us", "name": "English"}} + current_app.config["TRANSLATION_HOOK"] = lambda *a, **k: "Ventes" + + # Feature flag is on, but only one language is configured -> no translation. + assert i18n.translate("Sales") == "Sales" + + +def test_translate_returns_original_for_default_locale(mocker: MockerFixture) -> None: + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + current_app.config["BABEL_DEFAULT_LOCALE"] = "en" + current_app.config["TRANSLATION_HOOK"] = lambda *a, **k: "should not be used" + + class _En: + def __str__(self) -> str: + return "en" + + mocker.patch.object(i18n, "get_locale", return_value=_En()) + assert i18n.translate("Sales") == "Sales" + + +def test_translate_returns_original_when_no_hook( + mocker: MockerFixture, fr_locale: None +) -> None: + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + current_app.config["TRANSLATION_HOOK"] = None + + assert i18n.translate("Sales") == "Sales" + + +def test_translate_uses_hook_result(mocker: MockerFixture, fr_locale: None) -> None: + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + current_app.config["BABEL_DEFAULT_LOCALE"] = "en" + current_app.config["TRANSLATION_HOOK"] = lambda text, locale, **k: ( + "Ventes" if (text, locale) == ("Sales", "fr") else None + ) + + assert i18n.translate("Sales") == "Ventes" + + +def test_translate_forwards_context_to_hook( + mocker: MockerFixture, fr_locale: None +) -> None: + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + hook = mocker.MagicMock(return_value="Ventes") + current_app.config["TRANSLATION_HOOK"] = hook + + i18n.translate("Sales", model_name="Slice", field_name="slice_name") + + hook.assert_called_once_with( + "Sales", "fr", model_name="Slice", field_name="slice_name" + ) + + +def test_translate_falls_back_when_hook_returns_falsy( + mocker: MockerFixture, fr_locale: None +) -> None: + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + current_app.config["TRANSLATION_HOOK"] = lambda *a, **k: "" + + # Hook found no translation -> original text is preserved. + assert i18n.translate("Sales") == "Sales" + + +def test_translate_falls_back_when_hook_raises( + mocker: MockerFixture, fr_locale: None +) -> None: + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + + def _boom(*_a: object, **_k: object) -> str: + raise RuntimeError("translation service down") + + current_app.config["TRANSLATION_HOOK"] = _boom + + # A broken hook must never blank out the name. + assert i18n.translate("Sales") == "Sales" + + [email protected]("empty", [None, ""]) +def test_translate_passes_through_empty_text( + mocker: MockerFixture, fr_locale: None, empty: str | None +) -> None: + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + hook = mocker.MagicMock(return_value="x") + current_app.config["TRANSLATION_HOOK"] = hook + + assert i18n.translate(empty) == empty + hook.assert_not_called() + + +def test_translate_falls_back_when_locale_is_none(mocker: MockerFixture) -> None: + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + current_app.config["TRANSLATION_HOOK"] = lambda *a, **k: "Ventes" + mocker.patch.object(i18n, "get_locale", return_value=None) + + assert i18n.translate("Sales") == "Sales" + + +def test_i18n_macro_translates_via_hook(mocker: MockerFixture, fr_locale: None) -> None: + """The {{ i18n('...') }} Jinja macro resolves through the same hook.""" + from superset.jinja_context import i18n_macro + + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + current_app.config["TRANSLATION_HOOK"] = lambda text, locale, **k: ( + "Ventes" if (text, locale) == ("Sales", "fr") else None + ) + + assert i18n_macro("Sales") == "Ventes" + + +def test_i18n_macro_returns_original_when_disabled(mocker: MockerFixture) -> None: + """When the feature is off the macro returns the source text unchanged.""" + from superset.jinja_context import i18n_macro + + _enable(mocker, enabled=False) + current_app.config["LANGUAGES"] = MULTI_LANG + current_app.config["TRANSLATION_HOOK"] = lambda *a, **k: "Ventes" + + assert i18n_macro("Sales") == "Sales" + + +def test_translate_many_resolves_batch_in_one_hook_call( + mocker: MockerFixture, fr_locale: None +) -> None: + """The whole collection is resolved with a single batch-hook invocation.""" + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + batch_hook = mocker.MagicMock(return_value={"Sales": "Ventes", "Costs": "Coûts"}) + current_app.config["TRANSLATION_BATCH_HOOK"] = batch_hook + + resolved = i18n.translate_many( + ["Sales", "Costs"], model_name="Slice", field_name="slice_name" + ) + + assert resolved == {"Sales": "Ventes", "Costs": "Coûts"} + batch_hook.assert_called_once_with( + ["Sales", "Costs"], "fr", model_name="Slice", field_name="slice_name" + ) + + +def test_translate_many_falls_back_to_single_hook( + mocker: MockerFixture, fr_locale: None +) -> None: + """Without a batch hook, priming still works via the per-string hook.""" + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + hook = mocker.MagicMock(side_effect=lambda text, _locale, **_k: f"{text}-fr") + current_app.config["TRANSLATION_HOOK"] = hook + + resolved = i18n.translate_many(["Sales", "Costs"]) + + assert resolved == {"Sales": "Sales-fr", "Costs": "Costs-fr"} + assert hook.call_count == 2 + + +def test_translate_many_dedupes_and_skips_empty_texts( + mocker: MockerFixture, fr_locale: None +) -> None: + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + batch_hook = mocker.MagicMock(return_value={"Sales": "Ventes"}) + current_app.config["TRANSLATION_BATCH_HOOK"] = batch_hook + + resolved = i18n.translate_many(["Sales", "Sales", None, ""]) + + assert resolved == {"Sales": "Ventes"} + # Duplicates and empties never reach the hook. + batch_hook.assert_called_once_with(["Sales"], "fr") + + +def test_translate_many_falls_back_for_untranslated_entries( + mocker: MockerFixture, fr_locale: None +) -> None: + """A partial batch result keeps canonical text for the missing keys.""" + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + current_app.config["TRANSLATION_BATCH_HOOK"] = lambda texts, locale, **k: { + "Sales": "Ventes" + } + + assert i18n.translate_many(["Sales", "Costs"]) == { + "Sales": "Ventes", + "Costs": "Costs", + } + + +def test_translate_many_falls_back_when_batch_hook_raises( + mocker: MockerFixture, fr_locale: None +) -> None: + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + + def _boom(*_a: object, **_k: object) -> dict[str, str]: + raise RuntimeError("translation service down") + + current_app.config["TRANSLATION_BATCH_HOOK"] = _boom + + # A broken batch hook must never blank out names. + assert i18n.translate_many(["Sales", "Costs"]) == { + "Sales": "Sales", + "Costs": "Costs", + } + + +def test_translate_many_ignores_non_mapping_result( + mocker: MockerFixture, fr_locale: None +) -> None: + """A hook returning the wrong shape degrades to canonical text.""" + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + current_app.config["TRANSLATION_BATCH_HOOK"] = lambda texts, locale, **k: ["Ventes"] + + assert i18n.translate_many(["Sales"]) == {"Sales": "Sales"} + + +def test_translate_many_returns_original_when_feature_disabled( + mocker: MockerFixture, fr_locale: None +) -> None: + _enable(mocker, enabled=False) + current_app.config["LANGUAGES"] = MULTI_LANG + batch_hook = mocker.MagicMock(return_value={"Sales": "Ventes"}) + current_app.config["TRANSLATION_BATCH_HOOK"] = batch_hook + + assert i18n.translate_many(["Sales"]) == {"Sales": "Sales"} + batch_hook.assert_not_called() + + +def test_translate_routes_through_batch_hook_when_only_batch_configured( + mocker: MockerFixture, fr_locale: None +) -> None: + """The batch hook is a complete replacement, not an add-on.""" + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + current_app.config["TRANSLATION_HOOK"] = None + current_app.config["TRANSLATION_BATCH_HOOK"] = lambda texts, locale, **k: { + "Sales": "Ventes" + } + + assert i18n.translate("Sales") == "Ventes" + + +def test_prefetched_batch_serves_later_single_lookups( + mocker: MockerFixture, fr_locale: None +) -> None: + """Priming a collection makes the per-item lookups free for the request.""" + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + batch_hook = mocker.MagicMock(return_value={"Sales": "Ventes", "Costs": "Coûts"}) + current_app.config["TRANSLATION_BATCH_HOOK"] = batch_hook + + with current_app.test_request_context(): + i18n.translate_many(["Sales", "Costs"]) + assert i18n.translate("Sales") == "Ventes" + assert i18n.translate("Costs") == "Coûts" + + # The two follow-up lookups hit the request memo, not the hook. + batch_hook.assert_called_once() + + +def test_repeated_lookups_are_memoized_within_a_request( + mocker: MockerFixture, fr_locale: None +) -> None: + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + hook = mocker.MagicMock(return_value="Ventes") + current_app.config["TRANSLATION_HOOK"] = hook + + with current_app.test_request_context(): + assert i18n.translate("Sales") == "Ventes" + assert i18n.translate("Sales") == "Ventes" + + hook.assert_called_once() + + +def test_memo_distinguishes_the_same_text_across_fields( + mocker: MockerFixture, fr_locale: None +) -> None: + """Identical strings in different fields resolve independently.""" + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + current_app.config["TRANSLATION_HOOK"] = lambda text, locale, **kwargs: ( + "Ventes" if kwargs.get("model_name") == "Slice" else "Tableau des ventes" + ) + + with current_app.test_request_context(): + assert i18n.translate("Sales", model_name="Slice") == "Ventes" + assert i18n.translate("Sales", model_name="Dashboard") == "Tableau des ventes" + + +def test_memo_is_skipped_outside_a_request( + mocker: MockerFixture, fr_locale: None +) -> None: + """Background jobs have no request scope and simply resolve directly.""" + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + hook = mocker.MagicMock(return_value="Ventes") + current_app.config["TRANSLATION_HOOK"] = hook + # Patched rather than relying on the ambient context: the test harness may + # push one, and this asserts the no-request branch specifically. + mocker.patch.object(i18n, "has_request_context", return_value=False) + + assert i18n.translate("Sales") == "Ventes" + assert i18n.translate("Sales") == "Ventes" + + assert hook.call_count == 2 + + +def test_batch_hook_takes_precedence_when_both_are_configured( + mocker: MockerFixture, fr_locale: None +) -> None: + """A direct lookup must not resolve against a different store than a batch.""" + _enable(mocker) + current_app.config["LANGUAGES"] = MULTI_LANG + single_hook = mocker.MagicMock(return_value="from-single-hook") + current_app.config["TRANSLATION_HOOK"] = single_hook + current_app.config["TRANSLATION_BATCH_HOOK"] = lambda texts, locale, **k: { + "Sales": "Ventes" + } + + assert i18n.translate("Sales") == "Ventes" + single_hook.assert_not_called() Review Comment: Adopted in d182815. The assertion was not strictly incomplete — only the batch hook returns "Ventes" while the single hook returns "from-single-hook", so a broken batch path would have failed the equality check — but asserting the call directly states the intent and matches the other batch tests in the file. -- 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]
