search

encode / django-rest-framework

Web APIs for Django. 🎸
https://www.django-rest-framework.org
Other
28.45k stars 6.84k forks source link

Translation missing for part of throttle warning #6177

Closed GitRon closed 5 years ago

GitRon commented 6 years ago

I'm using the throttling mechanism of DRF. When the exception is shown that actually the request was throttled, one part of the message is translated, the other part is not:

grafik

I couldn't find the part "expected available in X seconds" in the translation file so I guess it is just missing?

I'm using djangorestframework==3.8.2.

Best regards Ron

rpkilby commented 6 years ago

It looks like the Throttled exception doesn't have translations for the extra detail.

https://github.com/encode/django-rest-framework/blob/5f1f2b100353970ac0dec672154fedd3df9d0a9f/rest_framework/exceptions.py#L223-L228

dennishylau commented 5 years ago

Any quick workaround possible?

rpkilby commented 5 years ago

Re-raise as a custom Exception class that has translations?

cristianocca commented 5 years ago

Bump. Just faced this, is it possible to get this fixed in the next minor release?

Rather than a missing translation, this is also looks like a misuse of the translation framework.

Monkey patch the exception in the meantime.

# Adding this here so we force the translation to happen
# because DRF is bugged, monkey patch it.
# Remove once https://github.com/encode/django-rest-framework/issues/6177 is fixed
from django.utils.translation import gettext_lazy as _
from rest_framework.exceptions import Throttled
Throttled.extra_detail_singular = _("Expected available in {wait} second.")
Throttled.extra_detail_plural = _("Expected available in {wait} seconds.")
rpkilby commented 5 years ago

Adding this to the milestone, as I don't think there is any significant complexity in fixing this.

Rather than a missing translation, this is also looks like a misuse of the translation framework.

Can you clarify? ngettext is still used to switch between the two messages.

cristianocca commented 5 years ago

Adding this to the milestone, as I don't think there is any significant complexity in fixing this.

Rather than a missing translation, this is also looks like a misuse of the translation framework.

Can you clarify? ngettext is still used to switch between the two messages.

I'm not 100% sure about what's the issue, but ngettext doesn't seem to be working unless the text itself is also a translated string. Please see my monkey patch above.

Looks like this line:

force_text(ungettext(self.extra_detail_singular.format(wait=wait),
                                     self.extra_detail_plural.format(wait=wait),
                                     wait))))

Will only work if extra_detail_singular and extra_detail_plural are lazy translation strings (or translated in place). Perhaps it is my Windows environment and gettext, but ungettext wasn't giving me translated text even if I added the translation string by hand.

Edit: Probably related to the fact that a variable is used there, and that .format is also used. Please read the django docs for a good example about how to use ungettext: https://docs.djangoproject.com/en/2.2/topics/i18n/translation/#pluralization

rpkilby commented 5 years ago

Probably related to the fact that a variable is used there, and that .format is also used.

This would make sense.

Please read the django docs for a good example about how to use ungettext

DRF uses .format instead of %-based formatting, so we can't just pass the strings directly to the various gettext functions. That said, wrapping the format strings in gettext_lazy should fix the issue.