Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit e4870b0

Browse filesBrowse files
committed
bug #44416 [Translation] Make http requests synchronous when reading the Loco API (Kocal)
This PR was merged into the 5.3 branch. Discussion ---------- [Translation] Make http requests synchronous when reading the Loco API | Q | A | ------------- | --- | Branch? | 5.3 | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead --> | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> <!-- Replace this notice by a short README for your feature/bugfix. This will help reviewers and should be a good start for the documentation. Additionally (see https://symfony.com/releases): - Always add tests and ensure they pass. - Bug fixes must be submitted against the lowest maintained branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too.) - Features and deprecations must be submitted against branch 5.x. - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry - Never break backward compatibility (see https://symfony.com/bc). --> Hi! Since we migrated from the [PHP Translation's Loco Adapter](https://github.com/php-translation/loco-adapter) to the [Symfony's Loco Translation Provider](https://github.com/symfony/loco-translation-provider), we started to have 429 errors from the Loco API when running our CI or deploying our code on review apps/staging/production environments: <details> <summary>Some screenshots</summary> ![image](https://user-images.githubusercontent.com/2103975/144423896-4ee1a4f5-7705-475b-83dc-efd34adeeeab.png) ![image](https://user-images.githubusercontent.com/2103975/144423987-fa739f3a-da16-41f7-9284-ce145009b84d.png) </details> We have asked to the Loco support and this is their response: > Hello. > > You will never get a 429 if you make single threaded requests. i.e. waiting for a response before dispatching another requests. As noted in the API usage limits: parallel requests burst the response speed limit. This is currently the only thing that would invoke a 429. I can provide more detail on the implementation if you would like. > > This measure is in lieu of quota based rate limiting which will one day be added, but currently no other limits are in place in terms of number of requests. Which makes sense. AFAIK, the PHP Translation's Loco Adapter makes synchronous requests to get translations from Loco. Some Blackfire traces, it does not impact performances: - before: https://blackfire.io/profiles/315f0748-76f8-4817-b59d-54ea482e1558/graph - after: https://blackfire.io/profiles/a6779b3e-b2ae-4870-b67f-667fe2016c0b/graph - delta: https://blackfire.io/profiles/compare/406d1f86-d9d8-45f1-9f8b-5c371d0973a3/graph I consider this PR as a bug-fix and not a feature, that's why I've targeted 5.3. WDYT? ping @welcoMattic Commits ------- 80a9c9f [Translation][Loco] Make http requests synchronous when reading the Loco API
2 parents 931a345 + 80a9c9f commit e4870b0
Copy full SHA for e4870b0

File tree

1 file changed

+22
-32
lines changed
Filter options

1 file changed

+22
-32
lines changed

‎src/Symfony/Component/Translation/Bridge/Loco/LocoProvider.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Translation/Bridge/Loco/LocoProvider.php
+22-32Lines changed: 22 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -92,47 +92,37 @@ public function read(array $domains, array $locales): TranslatorBag
9292
{
9393
$domains = $domains ?: ['*'];
9494
$translatorBag = new TranslatorBag();
95-
$responses = [];
9695

9796
foreach ($locales as $locale) {
9897
foreach ($domains as $domain) {
99-
$responses[] = [
100-
'response' => $this->client->request('GET', sprintf('export/locale/%s.xlf', rawurlencode($locale)), [
101-
'query' => [
102-
'filter' => $domain,
103-
'status' => 'translated',
104-
],
105-
]),
106-
'locale' => $locale,
107-
'domain' => $domain,
108-
];
109-
}
110-
}
111-
112-
foreach ($responses as $response) {
113-
$locale = $response['locale'];
114-
$domain = $response['domain'];
115-
$response = $response['response'];
98+
// Loco forbids concurrent requests, so the requests must be synchronous in order to prevent "429 Too Many Requests" errors.
99+
$response = $this->client->request('GET', sprintf('export/locale/%s.xlf', rawurlencode($locale)), [
100+
'query' => [
101+
'filter' => $domain,
102+
'status' => 'translated',
103+
],
104+
]);
105+
106+
if (404 === $response->getStatusCode()) {
107+
$this->logger->warning(sprintf('Locale "%s" for domain "%s" does not exist in Loco.', $locale, $domain));
108+
continue;
109+
}
116110

117-
if (404 === $response->getStatusCode()) {
118-
$this->logger->warning(sprintf('Locale "%s" for domain "%s" does not exist in Loco.', $locale, $domain));
119-
continue;
120-
}
111+
$responseContent = $response->getContent(false);
121112

122-
$responseContent = $response->getContent(false);
113+
if (200 !== $response->getStatusCode()) {
114+
throw new ProviderException('Unable to read the Loco response: '.$responseContent, $response);
115+
}
123116

124-
if (200 !== $response->getStatusCode()) {
125-
throw new ProviderException('Unable to read the Loco response: '.$responseContent, $response);
126-
}
117+
$locoCatalogue = $this->loader->load($responseContent, $locale, $domain);
118+
$catalogue = new MessageCatalogue($locale);
127119

128-
$locoCatalogue = $this->loader->load($responseContent, $locale, $domain);
129-
$catalogue = new MessageCatalogue($locale);
120+
foreach ($locoCatalogue->all($domain) as $key => $message) {
121+
$catalogue->set($this->retrieveKeyFromId($key, $domain), $message, $domain);
122+
}
130123

131-
foreach ($locoCatalogue->all($domain) as $key => $message) {
132-
$catalogue->set($this->retrieveKeyFromId($key, $domain), $message, $domain);
124+
$translatorBag->addCatalogue($catalogue);
133125
}
134-
135-
$translatorBag->addCatalogue($catalogue);
136126
}
137127

138128
return $translatorBag;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.