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

Fix uploading large files #481

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Fix uploading large files #481

wants to merge 3 commits into from

Conversation

quietsy
Copy link
Member

@quietsy quietsy commented Oct 25, 2024

Fix uploading large files which is a very common support issue

@quietsy quietsy requested a review from a team October 25, 2024 13:45
@drizuid
Copy link
Member

drizuid commented Oct 25, 2024

As per the nextcloud documentation, this should not be done as it can break low memory systems; user should read the documentation, understand the risks, and update accordingly. The default we use is the recommended official default from nextcloud.

@LinuxServer-CI
Copy link
Collaborator

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/nextcloud/develop-30.0.1rc2-pkg-edec5770-dev-115ac77edb2ea60a075e498abbe93753741e982f-pr-481/index.html
https://ci-tests.linuxserver.io/lspipepr/nextcloud/develop-30.0.1rc2-pkg-edec5770-dev-115ac77edb2ea60a075e498abbe93753741e982f-pr-481/shellcheck-result.xml

Tag Passed
amd64-develop-30.0.1rc2-pkg-edec5770-dev-115ac77edb2ea60a075e498abbe93753741e982f-pr-481
arm64v8-develop-30.0.1rc2-pkg-edec5770-dev-115ac77edb2ea60a075e498abbe93753741e982f-pr-481

@LinuxServer-CI
Copy link
Collaborator

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/nextcloud/develop-30.0.1rc2-pkg-edec5770-dev-94dff3871a6eff8d67d86e8b8dc99caea141e30c-pr-481/index.html
https://ci-tests.linuxserver.io/lspipepr/nextcloud/develop-30.0.1rc2-pkg-edec5770-dev-94dff3871a6eff8d67d86e8b8dc99caea141e30c-pr-481/shellcheck-result.xml

Tag Passed
amd64-develop-30.0.1rc2-pkg-edec5770-dev-94dff3871a6eff8d67d86e8b8dc99caea141e30c-pr-481
arm64v8-develop-30.0.1rc2-pkg-edec5770-dev-94dff3871a6eff8d67d86e8b8dc99caea141e30c-pr-481

@LinuxServer-CI
Copy link
Collaborator

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/nextcloud/develop-30.0.1rc2-pkg-edec5770-dev-1572ee2283d31dac7e200c1fcb37d4f017832051-pr-481/index.html
https://ci-tests.linuxserver.io/lspipepr/nextcloud/develop-30.0.1rc2-pkg-edec5770-dev-1572ee2283d31dac7e200c1fcb37d4f017832051-pr-481/shellcheck-result.xml

Tag Passed
amd64-develop-30.0.1rc2-pkg-edec5770-dev-1572ee2283d31dac7e200c1fcb37d4f017832051-pr-481
arm64v8-develop-30.0.1rc2-pkg-edec5770-dev-1572ee2283d31dac7e200c1fcb37d4f017832051-pr-481

@nemchik
Copy link
Member

nemchik commented Oct 25, 2024

Ref: https://docs.nextcloud.com/server/latest/admin_manual/configuration_files/big_file_upload_configuration.html

Note that some of the defaults you're proposing here are significantly higher than the values NC's own documentation provides as examples for increasing the values.

@quietsy
Copy link
Member Author

quietsy commented Oct 25, 2024

Ref: https://docs.nextcloud.com/server/latest/admin_manual/configuration_files/big_file_upload_configuration.html

Note that some of the defaults you're proposing here are significantly higher than the values NC's own documentation provides as examples for increasing the values.

Yup, I don't see a downside, the real limits are user quotas and disk space

@nemchik
Copy link
Member

nemchik commented Oct 25, 2024

Ref: https://docs.nextcloud.com/server/latest/admin_manual/configuration_files/big_file_upload_configuration.html
Note that some of the defaults you're proposing here are significantly higher than the values NC's own documentation provides as examples for increasing the values.

Yup, I don't see a downside, the real limits are user quotas and disk space

If we're confident that there are no concerns with lower resource devices then I'm ok with these changes. Main reason we stuck with what is in NC's docs is because the thought was that lower powered devices might not love the higher limits.

@quietsy
Copy link
Member Author

quietsy commented Oct 26, 2024

If we're confident that there are no concerns with lower resource devices then I'm ok with these changes. Main reason we stuck with what is in NC's docs is because the thought was that lower powered devices might not love the higher limits.

I've tested it on a container with a 512mb ram limit and everything still works fine, even uploading huge files.
I'll push it only to develop for a few weeks and continue to test it on my prod instance for that duration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: PRs Ready For Team Review
Development

Successfully merging this pull request may close these issues.

4 participants