Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix translation update script #14794

Merged
merged 4 commits into from
Jan 30, 2025
Merged

Fix translation update script #14794

merged 4 commits into from
Jan 30, 2025

Conversation

jfaltermeier
Copy link
Contributor

@jfaltermeier jfaltermeier commented Jan 30, 2025

What it does

Fixes this error: https://github.com/eclipse-theia/theia/actions/runs/13048443071/job/36403212934#step:7:20

How to test

Follow-ups

Breaking changes

  • This PR introduces breaking changes and requires careful review. If yes, the breaking changes section in the changelog has been updated.

Attribution

Review checklist

Reminder for reviewers

@jfaltermeier
Copy link
Contributor Author

Triggered a test run here: https://github.com/eclipse-theia/theia/actions/runs/13048824155/job/36404384953#step:7:146
Now fails with "Too many requests", which should be unrelated to the script itself.
I'll retrigger

@tsmaeder
Copy link
Contributor

Alternatively, we could add a "translate" script to the root package.json.

@jfaltermeier
Copy link
Contributor Author

jfaltermeier commented Jan 30, 2025

Getting "Too many requests" again (https://github.com/eclipse-theia/theia/actions/runs/13048824155). I don’t think there’s anything I can do about it. Deepl status looks fine: https://www.deeplstatus.com/
Maybe we’ve hit our limit? I’m not sure what plan or API token we’re using for the translation service.
@msujew Do you know?

@jfaltermeier jfaltermeier changed the title [WIP] Fix translation update script Fix translation update script Jan 30, 2025
@msujew
Copy link
Member

msujew commented Jan 30, 2025

@jfaltermeier My API token almost hasn't been used this month. It seems like we're hitting some rate limiting? I'll take a look at it.

@msujew
Copy link
Member

msujew commented Jan 30, 2025

@jfaltermeier I pushed a commit to this PR that should resolve the issue. It seems like DeepL has implemented rather strict rate limiting (without saying what it is - the documentation just says to "try again after a delay").

@msujew msujew marked this pull request as ready for review January 30, 2025 10:39
@jfaltermeier
Copy link
Contributor Author

Thanks, with these commits I get: https://github.com/eclipse-theia/theia/actions/runs/13050402257/job/36409193760#step:7:151

Could not translate into language "RU" Error: Requested tokens 11 exceeds maximum tokens per interval 10
    at RateLimiter.removeTokens (/home/runner/work/theia/theia/node_modules/limiter/dist/cjs/RateLimiter.js:42:19)
    at postWithRetry (/home/runner/work/theia/theia/dev-packages/localization-manager/lib/deepl-api.js:55:27)
    at postWithRetry (/home/runner/work/theia/theia/dev-packages/localization-manager/lib/deepl-api.js:64:20)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Promise.all (index 0)
    at async LocalizationManager.deepl [as localizationFn] (/home/runner/work/theia/theia/dev-packages/localization-manager/lib/deepl-api.js:37:23)
    at async LocalizationManager.translateLanguage (/home/runner/work/theia/theia/dev-packages/localization-manager/lib/localization-manager.js:88:45)
    at async Promise.all (index 8)
    at async LocalizationManager.localize (/home/runner/work/theia/theia/dev-packages/localization-manager/lib/localization-manager.js:63:25)
    at async Object.handler (/home/runner/work/theia/theia/dev-packages/cli/lib/theia.js:399:29)

@jfaltermeier jfaltermeier requested a review from tsmaeder January 30, 2025 11:18
Copy link
Contributor

@tsmaeder tsmaeder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks reasonable to me. Does it work with the deepl API, though?

@msujew
Copy link
Member

msujew commented Jan 30, 2025

@tsmaeder This fix has resulted in the PR #14796. This seems to work as expected :)

@jfaltermeier jfaltermeier merged commit a497457 into master Jan 30, 2025
12 checks passed
@github-actions github-actions bot added this to the 1.58.0 milestone Jan 30, 2025
@jfaltermeier jfaltermeier deleted the jf/translation-update branch January 30, 2025 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants