Document download buffers the whole file, then writes it one byte at a time #168

Document translation downloads the whole file into memory and then writes it one byte at a time. On a 20 MB document that is 8.0 seconds of work that takes 0.17 seconds with a three line change, and 80 MB of peak memory where 0.1 MB is enough.

There are two defects and they sit on either side of the same stream=True download in deepl/translator.py. They matter together, because each one hides the other.

1. _api_call reads .text on the streamed response, which downloads and decodes the entire document before the caller can stream any of it. The only thing that value is used for is a debug log line that is normally discarded.
2. translate_document_download defaults to chunk_size=1, and its only caller does not override it, so the document is written one byte per loop iteration.

Checked against v1.32.0 and against main at 7257b99.

Reproduction, measurements, tests and a patch:
https://github.com/nvaishnavisriyaa/deepl-python-document-streaming-repro

No API key and no network access are needed. A local stand-in answers the three document endpoints, so python reproduce.py ./deepl-python runs on its own.

The measurements

One document translation, end to end, through the public translate_document_from_filepath:

Peak memory in the client process, same translations:

Peak memory is 4x the document size and grows with it. With the patch it is flat, which is what stream=True was for.

The third column is the part I would flag hardest. Fixing the buffering on its own makes a 20 MB document roughly 24 times slower than it is today, because it restores real streaming and the one byte chunk size then reads a socket instead of memory. Anyone who notices the .text read and fixes just that would make things much worse.

Absolute times depend on the machine. The ratios and the memory shape do not.

1. The streamed body is read into memory for a log line that is thrown away

deepl/translator.py:163, in _api_call:

json = None
if isinstance(content, str):
    content_str = content
    try:
        json = json_module.loads(content)
    except json_module.JSONDecodeError:
        pass
else:
    content_str = content.text          # the whole document, decoded

util.log_info("DeepL API response", url=url, status_code=status_code)
util.log_debug("Response details", content=content_str)

content is a requests.Response only when stream=True, and the one call in the library that streams through _api_call is the document download at line 1063. So on every document translation that else branch pulls down the whole body and decodes it to str before translate_document_download gets to stream anything.

content_str has one consumer, the log_debug two lines below.

That call does not short circuit. From deepl/util.py:

def _get_log_text(message, **kwargs):
    return (
        message
        + " "
        + " ".join(f"{key}={value}" for key, value in sorted(kwargs.items()))
    )


def log_debug(message, **kwargs):
    text = _get_log_text(message, **kwargs)
    logger.debug(text)

The message is built before logger.debug is called and there is no level check, so a second full copy of the document is formatted into a string on every download whether or not debug logging is on. That is where the 4x comes from: the raw body, the decoded str, and the log line.

One detail that makes this safe to change: on the download path the error branch does not use this value either. translate_document_download passes the literal string "<file>" to _raise_for_status rather than the response, so no error message loses anything when the .text read goes away. There are tests in the repo pinning both that and the non-streamed branch, which still reports the response body as it does today.

2. The download loop runs one byte at a time

deepl/translator.py:1037:

def translate_document_download(
    self,
    handle: DocumentHandle,
    output_file: Union[TextIO, BinaryIO, Any, None] = None,
    chunk_size: int = 1,
) -> Optional[requests.Response]:
    ...
    if output_file:
        chunks = response.iter_content(chunk_size=chunk_size)
        for chunk in chunks:
            output_file.write(chunk)

One call site, deepl/translator.py:834, inside translate_document, and it does not override the default:

status = self.translate_document_wait_until_done(handle, timeout_s)
if status.ok:
    self.translate_document_download(handle, output_document)

DeepLClient subclasses Translator and overrides none of these methods, so the recommended entry point is on the same path. That covers every user of translate_document and translate_document_from_filepath.

The newer code in this repo already gets both of these right

deepl/deepl_client.py:1299 declares the same chunk_size: int = 1 default on download_translation_memory_export, but its caller overrides it, at deepl/deepl_client.py:1415:

with open(output_path, "wb") as output_file:
    self.download_translation_memory_export(
        job, output_file, chunk_size=8192
    )

That path also avoids the first defect. It goes through _asset_call, which hands the response back without touching .text, and _raise_for_asset_status uses "" rather than the body for a streamed response.

So the translation memory export does both things correctly and the older document path does neither, which is what makes me fairly sure this is an oversight rather than a decision.

For what it is worth, the sibling clients also stream properly: deepl-node pipes the response into a write stream, deepl-dotnet uses Content.CopyToAsync, and deepl-php hands the path to curl. deepl-python is the only one of the four that iterates the body itself.

Suggested fix

patch/document-streaming.patch in the repo applies to v1.32.0 and to main at 7257b99 with git apply. Six insertions, three deletions, across two files, and three of the insertions are a comment.

--- a/deepl/translator.py
+++ b/deepl/translator.py
@@ -160,7 +160,10 @@ class Translator:
             except json_module.JSONDecodeError:
                 pass
         else:
-            content_str = content.text
+            # Streamed response. Reading .text here would download and decode
+            # the whole body before the caller gets to stream it, which
+            # defeats the streaming that was asked for.
+            content_str = "<streamed response>"
 
         util.log_info("DeepL API response", url=url, status_code=status_code)
         util.log_debug("Response details", content=content_str)
@@ -1038,7 +1041,7 @@ class Translator:
         self,
         handle: DocumentHandle,
         output_file: Union[TextIO, BinaryIO, Any, None] = None,
-        chunk_size: int = 1,
+        chunk_size: int = 8192,
     ) -> Optional[requests.Response]:

plus the same one line change to the translation memory default in deepl/deepl_client.py, since a caller using that public method directly still gets 1 today.

black --check, flake8 and mypy all pass on the patched files under your own settings. black does flag deepl/http_client.py, but it does that on an untouched v1.32.0 checkout too, so it is not from this change.

Nothing in the test suite pins either default. The only tests that mention chunk_size pass 128 explicitly.

Two choices in there that are yours to make:

• A placeholder rather than dropping the log_debug call. This keeps the debug line for anyone using it for request tracing and leaves out only the body. The alternative is to make the logging lazy, behind logger.isEnabledFor(logging.DEBUG), which would help every other call in the library too. That is a wider change than this one, and a multi-megabyte body does not really belong in a log line at any level, so I kept it narrow. Happy to do it the other way.
• 8192 rather than something larger. It is the value already used at deepl_client.py:1415, so it keeps the two paths consistent.

Tests

test_streaming.py in the repo has the two defects as tests plus guards on what the fix must not change:

4 failed, 4 passed    against the shipped source
8 passed              with the patch applied

The guards cover the downloaded bytes being unchanged at a size that is not a multiple of any chunk size, a failed download still reporting its status code, a non-streamed error still carrying the response body, and the returned status still reporting billed characters.

A separate, smaller finding

In translate_document_from_filepath, at deepl/translator.py:725:

with open(input_path, "rb") as in_file:
    with open(output_path, "wb") as out_file:   # truncates before any work
        try:
            return self.translate_document(...)
        except Exception as e:
            out_file.close()
            os.unlink(output_path)              # and deletes it
            raise e

Point it at a path that already holds a file, have the translation fail, and that file is gone. test_output_file.py shows it against the shipped source.

patch/output-file-guard.patch is offered separately, since it is a different fix. It translates into a temporary file beside the output path and moves it into place with os.replace once the translation has succeeded. I went that way rather than refusing to overwrite, which is what the Node, PHP and .NET clients do, because refusing would be a behaviour change for anyone passing an existing path on purpose. Both patches apply cleanly together, in either order.

I am glad to open a pull request for either or both of these if that is useful, in whatever shape you prefer.