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

Add section about streaming reads with the s3 driver #9173

Closed
wants to merge 1 commit into from
Closed

Add section about streaming reads with the s3 driver #9173

wants to merge 1 commit into from

Conversation

adi64
Copy link
Contributor

@adi64 adi64 commented Dec 4, 2023

This PR is the spiritual successor to #8371.

The s3 filesystem driver doesn't really stream reads by default, even when calling e.g. readStream. Instead, it copies the whole file to a temporary directory and then returns a stream to that temporary file. This is because streaming reads are disabled by default:

https://github.com/laravel/framework/blob/v10.34.2/src/Illuminate/Filesystem/FilesystemManager.php#L244

This was introduced in laravel/framework#34001, but without an explicit reason. I also asked about this behavior here, but without luck: laravel/framework#49232.
I suspect that it was disabled by default because Flysystem changed the default behavior to using streams, and that could've broken backwards compatibility in Laravel 7.x, and it was never touched since then.

@taylorotwell
Copy link
Member

Will try to work this in.

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