Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sync max body size #3334

Merged
merged 1 commit into from
Nov 27, 2023
Merged

sync max body size #3334

merged 1 commit into from
Nov 27, 2023

Conversation

sjanssen2
Copy link
Contributor

@sjanssen2 sjanssen2 commented Nov 27, 2023

I am not totally sure if my analysis is accurate. Here is my understanding of the problem.

While setting up a qiita instance, we stumbled upon issues when uploaded files. File < chunk size were not affected. Also, this error only occurred when qiita run within the nginx setup, not as pure python instance. It looks like there is a default maximum body size for GET commands of 1M in nginx. Therefore, you might want to increase this size in the nginx_example.conf or at least leave a comment to inform others.
I have not tested to set this value to 3M, but I recognized that the actual size of the payload various around 3400000 byte, thus some buffer could be useful.

Ideally, both values would be sources from a configure file instead of "hard coding" them into the javascript and the nginx config file.

While setting up a qiita instance, we stumbled upon issues when uploaded files. File < chunk size were not affected. Also, this error only occurred when qiita run within the nginx setup, not as pure python instance. It looks like there is a default maximum body size for GET commands of 1M in nginx. Therefore, you might want to increase this size in the `nginx_example.conf` or at least leave a comment to inform others.
@antgonza
Copy link
Member

Thank you @sjanssen2; I think your explanation makes sense.

Now, I'm not sure if having this in a config file would actually be possible, to start the nginx config file is completely independent of qiita (in the main qiita system, they are actually started by different users). Anyway, the change looks good and will merge when the tests passes.

@coveralls
Copy link

Coverage Status

coverage: 92.894%. remained the same
when pulling fc5991a on sjanssen2:patch-1
into 1c1a4a7 on qiita-spots:dev.

@antgonza antgonza merged commit 4cef03a into qiita-spots:dev Nov 27, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants