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: check that object store backend supports multi part uploads #39432

Merged
merged 1 commit into from
Jul 17, 2023

Conversation

kesselb
Copy link
Contributor

@kesselb kesselb commented Jul 17, 2023

Summary

A object store backend may not support multi part uploads.

TODO

  • CI

Checklist

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@kesselb kesselb added bug 3. to review Waiting for reviews labels Jul 17, 2023
@kesselb kesselb added this to the Nextcloud 28 milestone Jul 17, 2023
@kesselb kesselb self-assigned this Jul 17, 2023
@kesselb
Copy link
Contributor Author

kesselb commented Jul 17, 2023

/backport to stable27

@kesselb
Copy link
Contributor Author

kesselb commented Jul 17, 2023

/backport to stable26

@artonge
Copy link
Contributor

artonge commented Jul 17, 2023

I thought we kept a fallback in those cases.

@kesselb
Copy link
Contributor Author

kesselb commented Jul 17, 2023

I thought we kept a fallback in those cases.

That's correct.

The exception, thrown in checkPrerequisites, is handled in afterMkcol, beforeDelete, beforeMove and beforePut. If a file upload is taking the new path, ChunkingV2Plugin.beforePut returns false and prevent the old ChunkingPlugin to run.

@juliusknorr juliusknorr merged commit 6f0086a into master Jul 17, 2023
@juliusknorr juliusknorr deleted the objectstore-without-multipart branch July 17, 2023 18:24
@kesselb
Copy link
Contributor Author

kesselb commented Jul 17, 2023

/backport to stable27

@kesselb
Copy link
Contributor Author

kesselb commented Jul 17, 2023

/backport to stable26

@backportbot-nextcloud
Copy link

The backport to stable27 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable27
git pull origin stable27

# Create the new backport branch
git checkout -b fix/foo-stable27

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

2 similar comments
@backportbot-nextcloud
Copy link

The backport to stable27 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable27
git pull origin stable27

# Create the new backport branch
git checkout -b fix/foo-stable27

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@backportbot-nextcloud
Copy link

The backport to stable27 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable27
git pull origin stable27

# Create the new backport branch
git checkout -b fix/foo-stable27

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@backportbot-nextcloud
Copy link

The backport to stable26 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable26
git pull origin stable26

# Create the new backport branch
git checkout -b fix/foo-stable26

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable26

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

1 similar comment
@backportbot-nextcloud
Copy link

The backport to stable26 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable26
git pull origin stable26

# Create the new backport branch
git checkout -b fix/foo-stable26

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable26

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@kesselb
Copy link
Contributor Author

kesselb commented Jul 18, 2023

Backport 27: #39458
Backport 26: #39459

@max-nextcloud
Copy link
Contributor

ChunkingV2Plugin.beforePut returns false

I think it returns true:
https://github.com/nextcloud/server/blob/master/apps/dav/lib/Upload/ChunkingV2Plugin.php#L149-L151

But I assume that still does the trick.

@kesselb
Copy link
Contributor Author

kesselb commented Jul 27, 2023

If a file upload is taking the new path, ChunkingV2Plugin.beforePut returns false and prevent the old ChunkingPlugin to run.

The first part is the important one ;)

https://github.com/nextcloud/3rdparty/blob/1ff71da76e24d442afab28586072409ef3b9660e/sabre/event/lib/WildcardEmitterTrait.php#L90

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MultipartUpload for uploading chunks to s3 may break other object store implementations
5 participants