codeant-ai-for-open-source[bot] commented on code in PR #39357:
URL: https://github.com/apache/superset/pull/39357#discussion_r3526326732
##########
superset/views/base.py:
##########
@@ -576,7 +577,24 @@ def common_bootstrap_payload() -> dict[str, Any]:
locale = get_locale()
# Convert locale to string for proper cache key hashing
locale_str = str(locale) if locale else None
- return cached_common_bootstrap_data(utils.get_user_id(), locale_str)
+ payload = dict(cached_common_bootstrap_data(utils.get_user_id(),
locale_str))
+ # Inject the Jed language pack outside the per-user memoize so the cached
+ # payload stays small and the pack is shared across users for the same
+ # locale. The frontend uses it to configure the translator synchronously,
+ # before any code-split chunk evaluates a module-level `const X = t('...')`
+ # (upstream issue #35330).
+ language = payload.get("locale")
+ if language and language != "en":
+ # Respect a pack already provided via COMMON_BOOTSTRAP_OVERRIDES_FUNC
+ # (the workaround in #35330 does exactly that), otherwise load the
+ # shared one. `get_language_pack` returns the empty English pack on a
+ # miss, which is the right result (English) when no translation file
+ # exists.
+ pack = payload.get("language_pack") or get_language_pack(language)
Review Comment:
**Suggestion:** Using `or` here treats an explicitly provided but empty
`language_pack` (for example `{}` from `COMMON_BOOTSTRAP_OVERRIDES_FUNC`) as
missing, so the override is ignored and the code unnecessarily reloads from
disk/fallback. This breaks the stated override contract and can cause repeated
I/O/error logging; check for `None` explicitly instead of truthiness.
[incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ COMMON_BOOTSTRAP_OVERRIDES_FUNC cannot enforce empty language pack.
- ⚠️ Non-English SPA payloads ignore explicit empty override packs.
- ⚠️ Extra language-pack I/O despite caller-provided override value.
- ⚠️ Potential repeated translation load errors despite override intent.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a custom `COMMON_BOOTSTRAP_OVERRIDES_FUNC` using the documented
extension
point at `superset/config.py:1001-1004`, in `superset_config.py`, so that
for a
non-English locale (e.g. `"fr"`) it returns a dict including
`"language_pack": {}`. This
dict is merged into `bootstrap_data` in `superset/views/base.py` where
`cached_common_bootstrap_data` builds the base payload and then applies
`COMMON_BOOTSTRAP_OVERRIDES_FUNC` (see `superset/views/base.py` lines 13-31
in the snippet
around
`bootstrap_data.update(app.config["COMMON_BOOTSTRAP_OVERRIDES_FUNC"](bootstrap_data))`).
2. Start Superset with this configuration and request any SPA page rendered
via
`superset/spa.html`, for example a normal application entry that uses
`BaseSupersetView.render_app_template` in `superset/views/base.py:705-707`,
which calls
`get_spa_template_context()`, then `get_spa_payload()`, which in turn calls
`common_bootstrap_payload()` at `superset/views/base.py:576-597`. Ensure the
user locale
for the request is set to `"fr"` so that `get_locale()` returns `"fr"`.
3. Inside `common_bootstrap_payload()` (`superset/views/base.py:576-597`),
`cached_common_bootstrap_data(utils.get_user_id(), locale_str)` returns a
payload that
includes `{"locale": "fr", "language_pack": {}}` because of the override in
step 1. The
function then runs:
- `language = payload.get("locale")` → `"fr"`, so the `if language and
language !=
"en":` branch is taken.
- `pack = payload.get("language_pack") or get_language_pack(language)` at
line 593,
where `payload.get("language_pack")` is `{}` (falsy), so Python evaluates
the `or` and
calls `get_language_pack("fr")` from
`superset/translations/utils.py:5-29` instead of
using the explicitly overridden `{}`.
4. Observe in the resulting `common` payload sent to the frontend (e.g. by
inspecting the
JSON embedded in `superset/spa.html` or by asserting in a unit test similar
to
`superset/tests/unit_tests/views/test_bootstrap_auth.py:147-167`) that:
- `payload["language_pack"]` is the non-empty pack returned by
`get_language_pack("fr")`, not the `{}` provided via
`COMMON_BOOTSTRAP_OVERRIDES_FUNC`,
so the explicit override is ignored.
- `get_language_pack("fr")` is invoked unnecessarily despite an override
being present,
which can cause extra disk I/O and, if the underlying file is missing or
malformed,
repeated error logging from `get_language_pack()` even though the intent
of the
override was to supply a specific (possibly empty) pack.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0c18d855fae44927be171a1477330171&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=0c18d855fae44927be171a1477330171&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/views/base.py
**Line:** 593:593
**Comment:**
*Incorrect Condition Logic: Using `or` here treats an explicitly
provided but empty `language_pack` (for example `{}` from
`COMMON_BOOTSTRAP_OVERRIDES_FUNC`) as missing, so the override is ignored and
the code unnecessarily reloads from disk/fallback. This breaks the stated
override contract and can cause repeated I/O/error logging; check for `None`
explicitly instead of truthiness.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39357&comment_hash=68dab49ef6f1df83515f178bbdd6ab647cf41b01e3ef14065d575a4b5b1b77a4&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39357&comment_hash=68dab49ef6f1df83515f178bbdd6ab647cf41b01e3ef14065d575a4b5b1b77a4&reaction=dislike'>👎</a>
--
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]
