Repository navigation
fix(gt-flask,gt-fastapi): load translations for every configured locale - #37
Kevin-Liu-01 wants to merge 3 commits into
Conversation
Flask eager-loaded only the locales passed to initialize_gt, so an app that lists its locales in gt.config.json, as the quickstart does, never loaded any translations. Neither integration loaded a locale on its first request, and nothing reloaded expired translations. Flask now eager-loads the resolved locales, also inside a running event loop. Both integrations load each configured locale before its first request, which also starts the background reload once it expires. Locales outside the configuration are never loaded. "cacheUrl": null in gt.config.json turns CDN loading off, and the file's _branchId goes to the CDN loader unless version_id is passed explicitly. The example apps drop their hand-written loading hooks. The floors move to gt-i18n>=0.3.1, the release with the cache fix, and generaltranslation>=0.1.0. Co-Authored-By: Claude Opus 5.5 <[email protected]>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if cache_url is not None: | ||
| resolved_cache_url: str | None = cache_url | ||
| elif "cache_url" in file_config: | ||
| # "cacheUrl": null (or "") in gt.config.json turns CDN loading off. | ||
| resolved_cache_url = file_config.get("cache_url") or "" |
There was a problem hiding this comment.
FastAPI's new CDN configuration rules have no integration tests. The Flask tests cannot catch mistakes in FastAPI's separate cache_url resolution or branch_id forwarding, so the two integrations could drift without a test failing.
Add FastAPI cases for null cache settings and file-provided version/branch values. Also test explicit cache_url and version_id overrides in both integrations, including that an explicit version drops the file's branch.
Knowledge Base Used: Web framework integrations
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/gt-fastapi/src/gt_fastapi/_setup.py
Line: 58-62
Comment:
**CDN config paths lack tests**
FastAPI's new CDN configuration rules have no integration tests. The Flask tests cannot catch mistakes in FastAPI's separate `cache_url` resolution or `branch_id` forwarding, so the two integrations could drift without a test failing.
Add FastAPI cases for null cache settings and file-provided version/branch values. Also test explicit `cache_url` and `version_id` overrides in both integrations, including that an explicit version drops the file's branch.
**Knowledge Base Used:** [Web framework integrations](https://app.greptile.com/generaltranslation/-/custom-context/knowledge-base/generaltranslation/gt-python/-/docs/web-framework-integrations.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Fixed in b98c0b3. Both integration test files now run the same table of five cases: the file's version and branch reach the CDN, an explicit version_id drops the file's branch, a null or empty cacheUrl turns CDN loading off, and an explicit cache_url replaces the file's. Reverting FastAPI's null cache_url handling or its branch_id rule makes the matching case fail.
| now[0] += 120 # two minutes later, past the 60 s cache expiry | ||
| later = client.get("/hello", headers={"Accept-Language": "es"}) | ||
|
|
||
| # The reload runs in the background on the app's event loop. |
There was a problem hiding this comment.
For expired entries, get_translations() starts a daemon thread, not work on the app's event loop. This comment could mislead a maintainer into reusing resources tied to that loop. Change it to say “background thread.”
| # The reload runs in the background on the app's event loop. | |
| # The reload runs on a background thread. |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/gt-fastapi/tests/test_fastapi_integration.py
Line: 137
Comment:
**Reload comment names wrong thread**
For expired entries, `get_translations()` starts a daemon thread, not work on the app's event loop. This comment could mislead a maintainer into reusing resources tied to that loop. Change it to say “background thread.”
```suggestion
# The reload runs on a background thread.
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
…-locales Co-Authored-By: Claude Opus 5.5 <[email protected]>
FastAPI had no tests for its cache_url and branch_id handling, which it implements apart from Flask's, so the two could drift without a test failing. Both integration test files now run one table of cases: the file's version and branch reach the CDN, an explicit version_id drops the file's branch, a null or empty cacheUrl turns CDN loading off, and an explicit cache_url replaces the file's. The table replaces Flask's two single-case tests. The FastAPI expiry test's comment now says that the reload of its synchronous loader runs on a background thread. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This branch has not been deployed
1 participant
Summary
This PR is stacked on fix(gt-i18n): keep translations loaded after the cache expires (#36 (
k/gt-i18n-cache-expiry)). Open it against that branch, or against main once that PR merges.gt-flask's
initialize_gteager-loaded only thelocalesargument (if eager_loading and locales:). An app whose locales come fromgt.config.json, like the quickstart's, never loaded any translations. Neither integration loaded a locale on its first request, soeager_loading=Falseneeded a hand-written hook, and both example apps had one. After startup, nothing called the loader again.Changes in gt-flask and gt-fastapi:
gt.config.json, throughload_all_translations_blocking(), which also works when an event loop is already running.before_requesthook and FastAPI's HTTP middleware load the request's locale before the route runs, when that locale is configured and needs translation. After expiry the call returns the cached translations and starts one background reload.esis configured and a customget_localereturnses-MX, nothing loads."cacheUrl": null(or"") ingt.config.jsonturns CDN loading off. Acache_url=argument still takes precedence._branchIdfromgt.config.jsongoes to the CDN loader, unlessversion_id=is passed explicitly, because the file's branch belongs to the file's version.gt-i18n>=0.3.1andgeneraltranslation>=0.1.0.Size: 11 files, +353/-60 on top of the gt-i18n PR. Product code is +26/-5, tests are +312/-18, example apps are +5/-33,
pyproject.tomlfiles are +4/-4 and the changeset is +6.Decisions
>=0.3.0floor would accept a gt-i18n with the 60 s bug.uv lock --checkpasses anduv.lockdoes not change.Overlap with #20
#20 (open since 2026-03-08, last updated 2026-03-20) moves the four example apps to
gt.config.jsonand_gt/<locale>.json, and it makes the same one-line change ingt_flask/_setup.py(if eager_loading and resolved_locales:). This PR also fixes gt-fastapi and adds tests for both packages. The two PRs conflict ingt_flask/_setup.pyand the two lazy example apps. Whichever merges second keeps this PR's_setup.py, which contains #20's line, and merges the example apps by hand.Test plan
After review, on b98c0b3, which merges #36's review fix (3b065a2) in 2ac6e44 (origin/main is still c3fe22b on 2026-10-17), with Python 3.12.13:
uv lock --check,uv sync --all-packages --locked,ruff check .,ruff format --check .andmypy .: clean.pytest: 1,443 passed. The new tests are now 17 cases: 9 intest_flask_integration.pyand 8 intest_fastapi_integration.py. Both files run the same five CDN settings cases, which replace Flask's two single-case tests._setup.pyfiles reverted to fix(gt-i18n): keep translations loaded after the cache expires #36's versions, 12 of the 17 cases fail, and so do the 8 example-app tests. Five pass on both. Two are the Flask cases that turn the CDN off, since the old Flask code never loaded those locales. Three are FastAPI cases the old code already handled: an emptycacheUrl, an explicitcache_url, and an explicit version, for which it sent no branch.Before review, on this branch (
k/flask-fastapi-configured-localesat 29580f4, on 0332c94, which is on origin/main c3fe22b, still the latest main on 2026-10-11), with Python 3.12.13:uv lock --checkanduv sync --all-packages --locked: the lockfile does not changeruff check .,ruff format --check .andmypy .: cleanpytest: 1,429 passed. That includes 9 new tests: 6 intest_flask_integration.pyand 3 intest_fastapi_integration.py._setup.pyfiles reverted to the gt-i18n PR's versions, 8 of the 9 new tests fail, and so do the 8 example-app tests, whose hooks this PR removes. The"cacheUrl": nulltest passes on both, because the old code never loaded that locale at all.Earlier, on the combined SDK fix: the quickstart's app, run exactly as printed against a stand-in CDN, answered in Spanish, while the published 0.1.0 answered in English. Two requests 65 s apart on real wall-clock time were both answered in Spanish.
Merge order
k/publish-missing-versions)), which must merge before the release PR.k/gt-i18n-cache-expiry)), then this PR.k/python-quickstart-loads-translations)), after 0.3.1 is on PyPI.Ernest McCarter wrote most of gt-python, so he should review this PR.
🤖 Generated with Claude Code
The PR appears safe to merge, with non-blocking improvements to configuration tests and the reload comment.
Fix with agent prompt
Summary
Both integrations now load configured request locales before routes run. Flask also eager-loads locales from
gt.config.json.Author acknowledgments:
es-MXwhen onlyesis configured and intentionally drops the file's branch whenversion_idis explicit.gt-i18nversion remains0.3.0; the stated release order supplies the required0.3.1version and publishing prerequisites.Diagram
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Read arguments and config] --> B[Create translation manager] B --> C{Eager loading enabled?} C -->|Yes| D[Load configured locales] C -->|No| E[Wait for requests] D --> F[Select request locale] E --> F F --> G{Configured locale needs translation?} G -->|No| K[Run route] G -->|Yes| H{Cache state} H -->|Missing| I[Wait for first load] H -->|Fresh| K H -->|Expired| J[Return cached text and start background reload] I --> K J --> KReviews (1) · Last reviewed commit: "fix(gt-flask,gt-fastapi): load translati..." · Reviewed by Greptile