codeant-ai-for-open-source[bot] commented on code in PR #39357:
URL: https://github.com/apache/superset/pull/39357#discussion_r3190432361
##########
superset/views/base.py:
##########
@@ -436,6 +436,34 @@ def get_default_spinner_svg() -> str | None:
return None
+def _load_language_pack(language: str | None) -> dict[str, Any] | None:
+ """Read the Jed language pack for ``language`` off disk.
+
+ Returns ``None`` for English, when the JSON file is missing, or when
+ it can't be parsed. The result is injected into the bootstrap payload
+ so the frontend can configure the translator synchronously before any
+ code-split chunk evaluates a module-level ``const X = t('...')``
+ (see upstream issue #35330).
+ """
+ if not language or language == "en":
+ return None
+ pack_path = os.path.join(
+ os.path.dirname(__file__),
+ "..",
+ "translations",
+ language,
+ "LC_MESSAGES",
+ "messages.json",
+ )
+ if not os.path.isfile(pack_path):
+ return None
+ try:
+ with open(pack_path, encoding="utf-8") as fp:
+ return cast(dict[str, Any], json.loads(fp.read()))
+ except (OSError, ValueError):
+ return None
Review Comment:
**Suggestion:** This introduces a second language-pack loader instead of
using the existing shared utility
(`superset.translations.utils.get_language_pack`) that already provides
process-level caching and consistent behavior. Because
`cached_common_bootstrap_data` is keyed per user, this new direct file read
path can repeatedly hit disk for the same locale across many users and diverge
from the established translation-loading flow. Reuse the existing utility to
avoid repeated I/O and keep bootstrap/endpoint language-pack semantics aligned.
[performance]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Extra disk reads per user for non-English locales.
- ⚠️ Bootstrap language loader duplicates translation utils' caching logic.
- ⚠️ Minor latency under heavy localized traffic due to I/O.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Hit any SPA endpoint that uses BaseSupersetView.render_app_template(),
such as SQL Lab
(`/superset/sqllab/`), where `render_app_template` in
`superset/views/base.py:1–24` calls
`get_spa_template_context`, which in turn calls `get_spa_payload`
(`superset/views/base.py:595–611`) and then `common_bootstrap_payload`
(`superset/views/base.py:159–163`).
2. In `common_bootstrap_payload`,
`cached_common_bootstrap_data(utils.get_user_id(),
locale_str)` is invoked (`superset/views/base.py:159–163`), and the function
is memoized
with `@cache_manager.cache.memoize(timeout=60)` at
`superset/views/base.py:248–256`,
meaning the cache key includes `user_id` and `locale_str`, so each user with
the same
locale gets a separate cached entry.
3. For each (user_id, locale) combination with a non-English language,
`cached_common_bootstrap_data` normalizes the locale to `language`
(`superset/views/base.py:78–89`) and then calls
`_load_language_pack(language)` at
`superset/views/base.py:126–133`, which constructs a path under
`superset/translations/<language>/LC_MESSAGES/messages.json`
(`superset/views/base.py:231–237`) and reads the file from disk with
`open()` when it
exists (`superset/views/base.py:241–243`), performing one disk read per
distinct user and
locale every cache window.
4. The existing shared utility `get_language_pack(locale)` in
`superset/translations/utils.py:7–30` already loads the same JSON pack and
caches it in
the process-wide `ALL_LANGUAGE_PACKS` dict
(`superset/translations/utils.py:2, 15–25`),
avoiding repeat disk I/O per locale; because `_load_language_pack` bypasses
this utility
and is invoked on a high-traffic bootstrap path, deployments with multiple
users sharing
the same non-English locale will incur redundant file reads and potentially
divergent
behavior between the bootstrap path and any future code that relies on
`get_language_pack`.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset%2Fviews%2Fbase.py%0A%2A%2ALine%3A%2A%2A%20439%3A464%0A%2A%2AComment%3A%2A%2A%0A%09%2APerformance%3A%20This%20introduces%20a%20second%20language-pack%20loader%20instead%20of%20using%20the%20existing%20shared%20utility%20%28%60superset.translations.utils.get_language_pack%60%29%20that%20already%20provides%20process-level%20caching%20and%20consistent%20behavior.%20Because%20%60cached_common_bootstrap_data%60%20is%20keyed%20per%20user%2C%20this%20new%20direct%20file%20read%20path%20can%20repeatedly%20hit%20disk%20for%20the%20same%20locale%20across%20many%20users%20and%20diverge%20from%20the%20established%20translation-loading%20flow.%20Reuse%20the%20existing%20utility%20to%20avoid%20repeated%20I%2FO%20and%20keep%20bootstrap%2Fendpoint%20language-pack%20semantics%20aligned.%0A%0AValidate%20the%20correctness%20of%20the%20fl
agged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset%2Fviews%2Fbase.py%0A%2A%2ALine%3A%2A%2A%20439%3A464%0A%2A%2AComment%3A%2A%2A%0A%09%2APerformance%3A%20This%20introduces%20a%20second%20language-pack%20loader%20instead%20of%20using%20the%20existing%20shared%20utility%20%28%60superset.translations.utils.get_language_pack%60%29%20that%20already%20provides%20process-level%20cac
hing%20and%20consistent%20behavior.%20Because%20%60cached_common_bootstrap_data%60%20is%20keyed%20per%20user%2C%20this%20new%20direct%20file%20read%20path%20can%20repeatedly%20hit%20disk%20for%20the%20same%20locale%20across%20many%20users%20and%20diverge%20from%20the%20established%20translation-loading%20flow.%20Reuse%20the%20existing%20utility%20to%20avoid%20repeated%20I%2FO%20and%20keep%20bootstrap%2Fendpoint%20language-pack%20semantics%20aligned.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
*(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:** 439:464
**Comment:**
*Performance: This introduces a second language-pack loader instead of
using the existing shared utility
(`superset.translations.utils.get_language_pack`) that already provides
process-level caching and consistent behavior. Because
`cached_common_bootstrap_data` is keyed per user, this new direct file read
path can repeatedly hit disk for the same locale across many users and diverge
from the established translation-loading flow. Reuse the existing utility to
avoid repeated I/O and keep bootstrap/endpoint language-pack semantics aligned.
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=1a45d5e6f9eef23e5ada11a224a4724fdaefc8840850b37a180272cd1437d743&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39357&comment_hash=1a45d5e6f9eef23e5ada11a224a4724fdaefc8840850b37a180272cd1437d743&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]
