glaterza opened a new pull request, #44611:
URL: https://github.com/apache/superset/pull/44611
### SUMMARY
This is **PR 2 of the plan in #44551**. @rusackas approved the approach
there:
> On PR 2: clearing the entries that error out or drop values seems right to
me, a translation that crashes SQL Lab is worse than falling back to English.
Go ahead and open it.
It clears **1,190 fuzzy translations in 18 catalogs**. Their placeholders do
not match the English source, so none of them can format correctly. Clearing
`msgstr` makes each entry untranslated, so the UI shows the English text with
its values instead of a crash or a literal `%s`.
**Confirmed (non-fuzzy) translations are not touched.** The 8 broken
confirmed entries (4 `mi`, 2 `es`, 1 `fr`, 1 `nl`) are PR 3. They are fixed by
hand so native speakers can review the wording.
#### What gets cleared
| Effect at runtime | Entries |
|---|---:|
| Raises an exception (backend `KeyError` / `TypeError`) | 37 |
| Drops a value: a name, a count, an error detail | 839 |
| Shows a blank or wrong value | 68 |
| Shows a literal placeholder, such as `Error: %s` | 26 |
| Shows nothing wrong: `Translator` catches the error and returns the
English key | 220 |
| **Total** | **1,190** |
The last row needs a word of caution, because it is easy to overstate this
bug. `Translator.translate` wraps `fetch` in a `try/catch` and returns the
input when formatting fails. So a broken frontend entry only shows a `%s` on
screen if the English source itself has a placeholder. 26 of them do. The other
220 already fall back to English today.
They are still worth clearing. They are translations that can never render.
They survive only because of a `catch`, and they count as "translated" in every
coverage number.
#### Why these entries are live
Superset serves fuzzy translations on purpose, so a broken fuzzy entry ships
like any other:
- `pybabel compile --use-fuzzy` reports **1,090 errors, exits 1, and still
writes all 30 `.mo` files**. The `Dockerfile` ends that command with `|| true`,
and `flask fab babel-compile` does not check the exit status.
- `superset-frontend/scripts/po2json.sh` passes `--fuzzy`, so the frontend
packs carry them too.
#### Three things to look at
1. **29 flagged entries are left alone**: 17 fail only when a count is 0,
which no call site passes, and 12 render correctly in practice. 26 of those 29
are fuzzy, so a simple "fuzzy and flagged" filter would have cleared entries
that work. The selection uses the runtime outcome, not the static check.
2. **97 entries had a `# Machine-translated via backfill_po.py (...)`
comment.** The comment described the translation being removed, so it goes too.
`backfill_po.py` picks entries by empty `msgstr`, so these are now clean
candidates for a future backfill.
3. **The diff only touches the 1,190 entries.** I edited the `.po` text
directly instead of rewriting the files with Babel or polib, because both
reformat unrelated entries. Babel adds a `python-format` flag next to
`no-python-format`; polib rewraps thousands of lines. I compared every entry in
all 18 catalogs before and after: msgid sets, headers, locations and extracted
comments are identical, and every changed entry is one of the 1,190.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
The user info page in `pt_BR`. Same build, same locale. Only the catalogs
differ.
**Before.** The *Reset my password* button reads `{} SENHA`. The catalog has
`msgstr "%s SENHA"` for a source string with no placeholder, and
Flask-AppBuilder's label handling turns the `%s` into `{}`.
**After.** The button reads *Redefinir minha senha*. Flask-AppBuilder has a
correct translation for this string in its own catalog, and Superset's broken
entry was overriding it. Clearing ours brings back the right Portuguese.
A second case, through the API. Creating a dataset that already exists, in
Dutch:
| | `master` | this PR |
|---|---|---|
| `nl` | **`HTTP 500 {"message": "Fatal error"}`** | `HTTP 422 {"table":
["Dataset main.fruit already exists"]}` |
| `en` (control) | `HTTP 422 {"table": ["Dataset main.fruit already
exists"]}` | same |
`Dataset %(table)s already exists` is translated with `%(name)s` in `nl`.
Building the validation message raises `KeyError`, so a clean 422 becomes a 500.
### TESTING INSTRUCTIONS
Measured at master `8141d666d6`. The flagged set is the same one reported in
#44551 at `269b9f99fe`: 1,227 entries, no drift.
**1. Compile errors drop**
```bash
cp -r superset/translations /tmp/after
pybabel compile --use-fuzzy -d /tmp/after > /tmp/after.log 2>&1; grep -c
'^error:' /tmp/after.log
# master: 1090 this PR: 154
```
The 154 left are a different problem: `pybabel`'s compile check is stricter
than the runtime. 97 are strings with a prose percent sign and no real format
code (`% of total`, `% calculation`; most already carry `no-python-format`), 44
are plural forms where one form spells its single number out, and 13 are others.
Only 7 of the 154 match an entry the runtime check still flags, and I can
name all 7. Six are the "fails only when the count is 0" plural forms left
alone on purpose (`%s day ago` and `%s min ago` in `pl` and `pt_BR`, `%s out of
%s column/metric` in `ar`). One is a confirmed `fr` translation that PR 3 fixes
(`There was an issue deleting the selected %s`).
**2. The Slovak SQL Lab failure is gone**
```bash
python -c '
from babel.support import Translations
t = Translations.load("/tmp/after", ["sk"], "messages")
print(t.ugettext("Running block %(block_num)s out of %(block_count)s") %
{"block_num": 1, "block_count": 1})
'
# master: KeyError: 'statement_num' this PR: Running block 1 out of 1
```
I re-checked all 37 backend exceptions this way. Every one raises on
`master`. None raises here.
**3. No translation regression**
```bash
python scripts/translations/check_translation_regression.py --count \
--translations-dir /path/to/master/superset/translations > /tmp/before.json
python scripts/translations/check_translation_regression.py --compare
/tmp/before.json
# "No translation regressions."
```
Confirmed-translation counts do not move in any of the 29 catalogs (`+0`
each). Only the fuzzy count drops, by exactly 1,190 across the 18 touched
catalogs.
**4. In a running app**
Enable the affected languages and compile the catalogs. You need `pybabel
compile -d superset/translations` and a Jed 1.x `messages.json` per locale, the
file `po2json.sh` produces. Released images ship neither, so the UI stays
English until you build them.
- Create a dataset that already exists, in Dutch: `master` returns **500
`Fatal error`**, English returns **422 `Dataset main.fruit already exists`**.
With this PR, Dutch returns the same 422.
- Open `/users/userinfo/` in `pt_BR`: the button reads `{} SENHA` on
`master` and *Redefinir minha senha* with this PR.
Two things that cost me time:
- Flask-AppBuilder picks the locale from `?_l_=<lang>` or the session,
**never from `Accept-Language`**. An API probe that only sets `Accept-Language`
runs in English and everything looks fine.
- The Slovak SQL Lab failure reproduces at the catalog level every time
(step 2, and `flask_babel.gettext` raises `KeyError: 'statement_num'` in a
Slovak request context). But I could not trigger it by running a query through
a released 6.1.0 image: that path logged the message in English while the same
request returned Slovak error text elsewhere. I have not worked out why, so use
step 2 as the reproducer.
### ADDITIONAL INFORMATION
- [x] Has associated issue: #44551
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
**Order.** #44551 also plans a CI check (PR 1) that fails when a
translation's placeholders cannot format. It should land after this PR and PR 3
so it starts green. This PR takes the flagged set from 1,227 to 37. PR 3 takes
8 more.
**Conflicts.** #43022 also edits `pt` and `pt_BR`, and already conflicts
with master. Whichever of the two lands second needs a rebase. I am happy to do
that in either order.
--
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]
