perf(s3): Provide direct pre-signed download link #54436
Conversation
|
FYI I've been working on #53634 to enable previews to generate by passing a presigned URL to ffmpeg. |
|
very excited for this! it'll be a gamechanger for nextcloud users no longer being "choked" behind nextcloud proxying everything. can't wait! |
|
I am also super excited, as it would allow to distribute data geographically with several auto mirroring minio instances across continents. Web browsers are automatically getting the file from the shortest round trip servers. |
|
Oh, this would be amazing for something I've been cooking up on the side! |
CarlSchwan
force-pushed
the
s3-signed-url
branch
from
f9f2e9a to
3a04f90
Compare
CarlSchwan
added
3. to review
CarlSchwan
requested review from
Altahrim,
come-nc,
icewind1991 and
salmart-dev
and removed request for
a team
|
Is there any way we can get a basic guide to testing this PR? I'm looking to get some basic performance data so I can plan my setup ahead of the release (and also have a bit of fun with it) |
CarlSchwan
force-pushed
the
s3-signed-url
branch
2 times, most recently
from
09235cc to
fd292e4
Compare
Best way is to test this PR is with juliusknorr/nextcloud-docker-dev#431 just change in .env PRIMARY to minio and start it up with |
CarlSchwan
force-pushed
the
s3-signed-url
branch
3 times, most recently
from
a9ac87c to
9fa3576
Compare
artonge
left a comment
artonge
left a comment
There was a problem hiding this comment.
- How is this behaving for files without read or download permissions?
- What happens if the object store is not available publicly?
CarlSchwan
force-pushed
the
s3-signed-url
branch
2 times, most recently
from
93cca86 to
e903621
Compare
I added a check for read. For the download permissions, the downloadUrl dav properties is already completely disabled for shares.
This is opt-in an behavior. You need to mark 'use_presigned_url' to true in your config.php for the object store. I need to add it to the documentation |
CarlSchwan
force-pushed
the
s3-signed-url
branch
from
e903621 to
d6bfac4
Compare
Should we add a setup check to hint to this new possibility? |
I would propose to specify the URL instead, as the URL of the front S3 server might be and often is different than the one set internally |
|
Quick question.
Does this mean the actual download url provided to the user within nextcloud while using this PR is still proxied? |
@ldpr Have you set |
Yes, I really should have mentioned that, sorry. It's set within the S3 config block. |
This is faster than going back to nextcloud to download the files. This is an opt-in setting that can be enabled by setting use_presigned_url in the object store config. Additionally add support for the proxy config which is needed in a docker setup. See juliusknorr/nextcloud-docker-dev#431 Signed-off-by: Carl Schwan <[email protected]>
CarlSchwan
force-pushed
the
s3-signed-url
branch
from
d6bfac4 to
b6313f6
Compare
| * @return array{url: ?string, expiration: ?int}|false | ||
| * @since 33.0.0 | ||
| */ | ||
| public function getDirectDownloadById(string $fileId): array|false; |
There was a problem hiding this comment.
This is a breaking change for apps that provide a storage wrapper 🙈
Please document accordingly
There was a problem hiding this comment.
Indeed. Surprised me a bit while doing some groupfolders changes; thought it was my fault. ;-)
There was a problem hiding this comment.
Correction: the real issue was outdated stubs in gf repo that lacked this new method. All good now (at least in terms of fallback behavior).
Summary
This is faster than going back to nextcloud and setting up the file system to download the files.
TODO
Currently in my dev docker setup, the minio service is not available from outside, so the url can't be accessed. There should be a way to configure whether the s3 service is available or not from outsideMinio + pre-signed urls juliusknorr/nextcloud-docker-dev#431Also expose the preview url like this(let's move that to a seperate PR)Checklist