wagtail-localize
wagtail-localize copied to clipboard
Support Deepl free api
Essentially it enables the Deepl API to be changed since they have two different (free and pro). Also some improvements were made on error handling:

There's a new option to define the Deepl API endpoint:
WAGTAILLOCALIZE_MACHINE_TRANSLATOR = {
"CLASS": "wagtail_localize.machine_translators.deepl.DeepLTranslator",
"OPTIONS": {
"AUTH_KEY": os.environ.get('DEEPL_API_KEY', None),
"API_ENDPOINT": os.environ.get('DEEPL_API_ENDPOINT', None),
},
}
That doesn't seem to apply for AUTH_KEY and seems too verbose for an option. Since it can also be used in the future for other endpoints I'd leave it as it is.
On Tue, 15 Feb 2022 at 18:48, Dan Braghis @.***> wrote:
@.**** commented on this pull request.
In wagtail_localize/machine_translators/deepl.py https://github.com/wagtail/wagtail-localize/pull/505#discussion_r807134928 :
@@ -17,8 +27,9 @@ class DeepLTranslator(BaseMachineTranslator): display_name = "DeepL"
def translate(self, source_locale, target_locale, strings):
api_endpoint = self.options.get("API_ENDPOINT", "https://api.deepl.com/v2/translate")nitpick: missed the first time around, but we do prefix our settings with WAGTAILLOCALIZE_ so this should be consistent. i.e. WAGTAILLOCALIZE_DEEPL_API_ENDPOINT
— Reply to this email directly, view it on GitHub https://github.com/wagtail/wagtail-localize/pull/505#pullrequestreview-883404415, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAUGTDMVB76J7T7RQZIZH7DU3KGXJANCNFSM5MF5JKHQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID: @.***>
That doesn't seem to apply for AUTH_KEY and seems too verbose for an option. Since it can also be used in the future for other endpoints I'd leave it as it is.
@vladox sorry for the noise, I realised the options already came via WAGTAILLOCALIZE_MACHINE_TRANSLATOR after I made the comment, so removed it
#604 provides a simpler approach to this, so may use that instead
Went with #604. Thank you very much for this @vladox
@zerolab thanks for the update but be aware that the implementation in #604 is missing the validation and error messages in case the URL doesn't match the key.
@vladox #604 conditionally loads the endpoint based on the key (deepl library reference)
@zerolab there's also a part of the PR that handles exceptions from the API and shows them correctly formatted to the end user, see class ApiEndpointError(Exception).
I can do a separate PR only for that part since I think it's necessary to make the integration more user friendly.
@vladod you're right, let's do it. Just make sure you mock the request/response