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

Remove 500M restriction on file uploads. Use 10G instead. [affects NGINX, PHP, Calibre-Web, Moodle, Nextcloud, WordPress, PBX, ETC] #3148

Merged
merged 4 commits into from
Dec 21, 2023

Conversation

@holta holta added this to the 8.0 milestone Mar 25, 2022
@holta
Copy link
Member Author

holta commented Mar 25, 2022

This PR is especially for "LMS's" like WordPress, Nextcloud, Moodle, as large files (videos, educational documentaries, etc) become increasingly common.

But also potentially for Kolibri, PBX and others that might be affected by NGINX?

While Calibre-Web might be internally restricted to 200M for now? As mentioned here:
https://github.com/janeczku/calibre-web/wiki/FAQ#what-is-the-maximum-file-size-which-can-be-uploaded-through-calibre-web

@holta
Copy link
Member Author

holta commented Mar 25, 2022

CLARIFICATION: One should ignore the claim at https://www.php.net/manual/en/ini.core.php#ini.post-max-size that "Generally speaking, memory_limit [512M when nginx_high_php_limits: True] should be larger than post_max_size [10000M when nginx_high_php_limits: True]"

REASON: Several stackoverflow.com articles confirm that uploaded files are cached in /tmp not memory (during upload).

@holta holta changed the title Remove 500M restriction on file uploads. Use 10G instead. Remove 500M restriction on file uploads. Use 10G instead. [affects NGINX, PHP, Moodle, Nextcloud, WordPress, ETC] Mar 25, 2022
@jvonau
Copy link
Contributor

jvonau commented Mar 28, 2022

Think using 10G is a bit much, good way to run the machine out of storage space when the filesystem is near maximum capacity causing a DOS situation as noted in #2584

@jvonau
Copy link
Contributor

jvonau commented Mar 28, 2022

Might be better to expose 'client_max_body_size' directly as a end user editable variable via local_vars.yml?

@holta holta modified the milestones: 8.2, 8.1 Dec 21, 2023
…igh_php_limits or moodle_install or nextcloud_install
@holta
Copy link
Member Author

holta commented Dec 21, 2023

@EMG70 @deldesir it's time to move forward with this PR or similar. As too many grassroots/indigenous communities are now getting blocked from uploading their very own multi-gigabyte phone videos to Calibre-Web, Nextcloud, Moodle, WordPress, etc.

Of course there will be glitches en route, e.g. when local educators misunderstand that they really do need to compress their own phone videos prior to sharing them.

But let's not make their lives impossible. So let's fine tune as we go forward — figuring it out en route to help as many educators as we can — and adjusting as needed to help them across the thousands of communities who want this:

  1. Local educators having authentic ownership refining their community's curricular content (or their own family's personal videos) is the goal — it's high time that we stop blocking local voices / local contributors by default.
  2. IIAB's main page will eventually in fact need a colorful "Disk Nearly Full" warning right on top (something similar to RACHEL's perhaps?) making "Vital Signs" visible to everyone without a password — e.g. things like {Temperature, WiFi status, Disk Almost Full} would be really great: Hardware Failure Icons on top of IIAB main page (http://box.lan) e.g. Overheating, Disk Full/Malfunction, WiFi Dead, RTC Dead #3309

Related:

@holta holta changed the title Remove 500M restriction on file uploads. Use 10G instead. [affects NGINX, PHP, Moodle, Nextcloud, WordPress, ETC] Remove 500M restriction on file uploads. Use 10G instead. [affects NGINX, PHP, Calibre-Web, Moodle, Nextcloud, WordPress, PBX, ETC] Dec 21, 2023
@holta holta requested a review from deldesir December 21, 2023 16:49
Copy link
Contributor

@deldesir deldesir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@holta
Copy link
Member Author

holta commented Dec 21, 2023

Might be better to expose 'client_max_body_size' directly as a end user editable variable via local_vars.yml?

@jvonau's above suggestion is a really good one as we later fine-tune.

But first let's set the default to something far higher (personally I advocate for 10000M to see how this works out in coming months, in keeping with modern phone videos and educational documentaries that schools/educators need in all countries, ETC!)

@holta
Copy link
Member Author

holta commented Dec 21, 2023

Thanks @deldesir:

Let's go with this for starters, gauge impact, and adjust en route where that proves necessary. As @EMG70 & others do further testing. FWIW this has been repeatedly tested on Raspberry Pi OS and Ubuntu 24.04 pre-releases over the past 2 years — now's the time to start welcoming more communities to battle-test this with their own grassroots/indigenous use cases — helping us/all to evolve!

@holta holta merged commit aceb236 into iiab:master Dec 21, 2023
3 checks passed
@tim-moody
Copy link
Contributor

while 500M is a little low, I find 10G pretty big. I would have gone for 1 or 2 G.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants