Suggestion: Asynchronous Support

[nfnfgo]

It really a nice thing to hear DeepL provide a Python client module.

Through the doc I found that this module seems not support async functions, and I think it would be better to use a async request module such as aiohttp to make the translation can be used asynchronously

[daniel-jones-deepl]

Hi @nfnfgo, that is right, currently only synchronous calls are possible because the library is based on requests.

We would like to add asynchronous functionality to this library in future.

[tivaliy]

I think the fastest solution for now could be to generate async client based on available OpenAPI specification. But yes, it's nice to have officially supported "async" version

[j-adamczyk]

@daniel-jones-deepl if this library would use httpx, it could support both sync and async easily. It has API most similar to requests, so it wouldn't be that hard to rewrite at first

[ignxdank]

Are there any further updates on if this will be happening/the current status?

[AutonomousCat]

+1 for async support. aiohttp or httpx

[marek-jelo-jelinek]

+1

[AutonomousCat]

Just wanna add, it seems like a single file, the http client file, that needs to be updated. Shouldn't be that hard for anyone who wants this and has some time. (this is directed towards non-maintainers looking at this)

[daniel-jones-dev]

Hi everyone, thanks for adding your interest on this feature. (Note: this is @daniel-jones-deepl, I've migrated to my personal account).

I had a quick look into this, aiohttp looks like a good choice.

Regarding only changing the file http_client.py: it would be an option. Another user implemented a DeepL Python library using something similar. However it doesn't allow proper typing support.

I think the best way to support async would be add a new class TranslatorAsync with the same interface as Translator, except that most functions (translate_text etc.) are labelled async.

On Friday I have some time to work on that more, I'll post another update then.

[AutonomousCat]

@daniel-jones-dev Heck yeah! This will make lots of Discord bot developers (who like accessibility through readily available translations) happy

[daniel-jones-dev]

I've pushed some WIP changes to the async branch, and I'd be interested in feedback.

Lots of things are broken (see below), but the following should work with aiohttp installed:

async def async_func():
    async with deepl.TranslatorAsync(auth_key) as async_translator:
        text_result = await async_translator.translate_text(
            "Hello, world!", target_lang="de"
        )
        print(text_result.text)

asyncio.run(async_func())

Currently only translate_text works (also the synchronous version), the other functions are broken. Also proxies and SSL-verification are broken.

Anyway, feedback is welcome!

[Nereg]

Hi! The WIP async version works for me, but I see there is not a lot of progress here. Maybe I could contribute some code to help with it. The proxy and SSL verification should be easy enough to implement, and then I could do the user-facing APIs. I wonder how the test will be adapted to test async implementation too.

[Nereg]

It seems that the WIP async branch has broken sync implementation! I don't know (but assume) that it is fine in the main branch. So I will need to find the issue with sync first - they use the same code to process the results from the network. Here is the code to reproduce:

import asyncio
import traceback

import deepl


async def main():
    trans = deepl.Translator(
        "KEY",
        # proxy={"https": "https://localhost:8080"},
        # verify_ssl=False,
    )
    trans_async = deepl.TranslatorAsync("KEY")
    try:
        print(trans.translate_text("test", source_lang="EN", target_lang="RU"))
    except Exception as e:
        traceback.print_exception(e)
    print("translation results: " + await trans_async.translate_text("test", source_lang="EN", target_lang="RU"))
    await trans_async.close()


asyncio.run(main())

and the output on my side:

Traceback (most recent call last):
  File "***\deepl-python-testing\test.py", line 16, in main
    print(trans.translate_text("test", source_lang="EN", target_lang="RU"))
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "***\.venv\Lib\site-packages\deepl\translator.py", line 230, in translate_text
    return self._translate_text_post(response, base_context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "***\Lib\site-packages\deepl\translator_base.py", line 384, in _translate_text_post
    translations = response.json.get("translations", [])
                   ^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'get'
translation results: тест

So the async version works, while sync doesn't.

[daniel-jones-dev]

Hi @Nereg, I did make some progress on it a few weeks back, but I forgot to push those changes. I've pushed them to the async branch now.

The changes fix all the existing tests for the sync implementation, and the async translate_text function works too. The async tests are incomplete and failing in the CI because asyncio is not installed.

Some remaining work to be done:

• implement remaining async functionality - mostly trivial aside from the document download/upload functions.
• complete async tests - I wanted to try a more elegant way to test both the sync and async implementation without duplicating test code. Maybe a possibility is to rewrite all tests to be async, and wrap the sync implementation to behave as async, then parameterise all tests with a sync and async translator.
• proxies and SSL verification
• documentation

[Nereg]

Sorry for the delay, anyway. In my testing (although, I can't replicate that if I install directly from async branch, for some reason) I got this error:

Traceback (most recent call last):
  File "...\test.py", line 19, in main
    print(trans.translate_text("test", source_lang="EN", target_lang="RU"))
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "...\.venv\Lib\site-packages\deepl\translator.py", line 230, in translate_text
    return self._translate_text_post(response, base_context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "...\.venv\Lib\site-packages\deepl\translator_base.py", line 384, in _translate_text_post
    translations = response.json.get("translations", [])
                   ^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'get'

It is caused by this code, not recognizing that it is a JSON response https://github.com/DeepLcom/deepl-python/blob/c70f06afa34b7b85eeb9111b4607b0b4108239f1/deepl/translator_base.py#L101-L109 There could be many variations of both Content-Type and application/json. Which is why I changed it to try to decode JSON regardless of MIME type: https://github.com/Nereg/deepl-python-async/commit/9b85972327d2da200cc84ce3bb30c9fade4154e1 Do you think there is a better way? I will now work on proxies and SSL verification, it could be difficult, if we want full interoperability.

[Nereg]

https://github.com/Nereg/deepl-python-async/commit/24fd1328b0aadfbb318eed6162537a3b92c0898d Implemented both full disablement of SSL verification and using your own SSL CA certificates. Not sure, if it will behave precisely like request's implementation, but seems fine,

[Nereg]

For now, I added proxy support: https://github.com/Nereg/deepl-python-async/commit/efd945528c6c00dd627a8767e005208471cd9a05 But, I am very confused, where I should implement the user-facing APIs. The place seems to be https://github.com/DeepLcom/deepl-python/blob/async/deepl/translator_async.py but in your version, it doesn't even implement any requests. In my version (probably forked from a different point in git's history) even the sync _api_call is not working. Any advice would be appreciated!

[daniel-jones-dev]

Hi @Nereg, thanks for testing and fixing these things. I struggle at the moment to find time to work on this.

Your JSON-decoding, SSL and proxy changes all look good. You should add the SSL and proxy parameters to TranslatorAsync -- it does implement the requests, see the with_base_pre_and_post decorator: the request-functions are all implemented in three steps: pre-function, request, post-function. The pre- and post-functions come from TranslatorBase and are shared with the synchronous Translator.

I've just pushed some more changes -- all async request functions work now, including document translation. I incorporated some of your changes above (will fix those commits later to give you credit). Also added some more async tests, but ideally we'd add some more for SSL and proxy before merging.

[Nereg]

While browsing the code, I found a weird thing, that I think should not be this way. https://github.com/DeepLcom/deepl-python/blob/adcb68294c8b424fc8d4a2282ead16a55d57e731/deepl/aiohttp_http_client.py#L15-L19 https://github.com/DeepLcom/deepl-python/blob/adcb68294c8b424fc8d4a2282ead16a55d57e731/deepl/aiohttp_http_client.py#L86-L89 So, in those lines, we first import the aiohttp library, and if it fails - we store the exception to throw it later. Wouldn't it be better to fail-fast ? As far as I can see - it is only imported in translator_async, which is a resonable place to fail, if there is no aiohttp. Furthermore, there is no such code in the requests_http_client.py which could also fail, if there is no requests installed. I can fix that, if there is no reason behind it. Currently working on applying my SSL and proxy changes to the code you pushed. It seems like you've done the JSON detection part already. Then will work on covering those with tests.

[Nereg]

So, I've applied the SSL bypass and proxy support, added tests for them, and fixed the needs_mock_proxy_server condition. It wrongly required mock_server_port to be set, which is not related to proxy support. I can also write tests for the API calls, if needed. Also, please check the tests thoroughly, I have little experince with async tests.

[daniel-jones-dev]

I admit I haven't fully thought out the design of the conditional importing. My idea was to simplify the user experience: import deepl should pull in the sync version, and the async version too if aiohttp is available. Then constructing a TranslatorAsync should fail if aiohttp is unavailable. Perhaps it is clearer to instead expect an additional import deepl.async statement to use async, and that import should fail immediately if a dependency is missing.

I didn't include conditionals on requests simply because it is already a requirement for the library. However maybe it makes sense to make requests an optional dependency, if that is possible.

Thanks for the contributions; I have to review them later.