search

zotero / translation-server

A Node.js-based server to run Zotero translators
Other
117 stars 49 forks source link

Make maxResponseSize configurable #132

Closed mvolz closed 2 years ago

mvolz commented 2 years ago

Change-Id: Ifee154e495e4b659d69d66b24583547cd288b9cc

dstillman commented 2 years ago

What are you looking to set it to? I'm not sure why we have it at 50 by default. That seems extremely large.

dstillman commented 2 years ago

Oh, sorry, this is about the maximum response size for every HTTP request. I don't think maxResponseSize is an appropriate name for that — it suggests it's about the maximum size of the translation-server response, or something. If we want this to be configurable at all, it should probably be httpMaxResponseSize, and the comment should clarify what it's for and what happens if a response is larger (400 error).

@mrtcode, do you remember the cases we were trying to address with this or how we decided upon 50 MB?

mvolz commented 2 years ago

Oh, sorry, this is about the maximum response size for every HTTP request. I don't think maxResponseSize is an appropriate name for that — it suggests it's about the maximum size of the translation-server response, or something. If we want this to be configurable at all, it should probably be httpMaxResponseSize, and the comment should clarify what it's for and what happens if a response is larger (400 error).

@mrtcode, do you remember the cases we were trying to address with this or how we decided upon 50 MB?

It was us again - dealing with memory leaks - issue #68 . I think this was mostly from trying to load pdfs, but this was added as well - I think we'd want to lower it to 10mb? I've put that as the default in the new pr but can change it again.

dstillman commented 2 years ago

Thanks!