IMGIITRoorkee / omniport-docker

The Dockerised setup of the one true portal for any and every educational institute
GNU General Public License v3.0
37 stars 23 forks source link

Add NGINX directive #35

Closed algomaster99 closed 3 years ago

algomaster99 commented 3 years ago

@dhruvkb It is true for timeouts. I did that because IITR has a better network connection than an average household so naturally, we would want a higher timeout in case of internet. No, it isn't true for client_max_body_size. We just wanted it customisable. @pradumangoyal

dhruvkb commented 3 years ago

I don't think increasing the NGINX timeout values is a solution for slow internet. These timeouts are for NGINX <-> Gunicorn/Daphne communication which will be unaffected by the internet speed of the client.

pradumangoyal commented 3 years ago

@algomaster99 Agreed!

The main purpose is to have both values customisable. Even if they are same for internet and intranet, maintainers should have option to set some values while setting up their instance of Omniport. What are your views on this @dhruvkb ?

dhruvkb commented 3 years ago

I don't prefer a lot of values to be customisable because fewer variables means fewer differences between installations means fewer cases of "works on my setup". Then again, upload limit is a minor inconsequential thing so do whatever you want.

algomaster99 commented 3 years ago

I don't think increasing the NGINX timeout values is a solution for slow internet. These timeouts are for NGINX <-> Gunicorn/Daphne communication which will be unaffected by the internet speed of the client.

Yes, I get that. But when uploading huge files if the NGINX doesn't receive a response from Gunicorn, it throws 502 so we have to adjust NGINX's timeout according to what we have set for Gunicorn.

dhruvkb commented 3 years ago

@algomaster99 use workers on separate threads for long tasks, else you'll block the thread for all users!

algomaster99 commented 3 years ago

@dhruvkb I am not sure how that will help with uploading files. Can you elaborate? We have already configured 145 workers in gunicorn.

dhruvkb commented 3 years ago

My bad, the timeout increase might still be needed. I'll withdraw my arguments against this 👍

algomaster99 commented 3 years ago

@dhruvkb We although have to make uploading a background process (maybe via Websockets). The file-manager team has scheduled it for v2.