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

S3 streaming support #1444

Closed
wants to merge 1 commit into from
Closed

S3 streaming support #1444

wants to merge 1 commit into from

Conversation

andonovn
Copy link

@andonovn andonovn commented Apr 14, 2020

Not sure if this is considered a bug fix or a new feature.

Checklist:

Why this change is needed?

Currently, it's impossible to stream the file to s3. The following is the error when someone attempts to do so:

ZipArchive::close()
/var/www/application/vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Writer/Xlsx.php:409

The solution provided in this PR is very naive, and not elegant at all, but it works.
Another option would be to extract strtolower($pFilename) == 'php://output' || strtolower($pFilename) == 'php://stdout' to a protected method, which could be overriden by the consumers.
There are more complex options as well, fe creating different adapters, but unfortunately I don't really have time to work on them.

But the main goal of this PR is to open a discussion around this. I am curious to know if you are willing to add S3 streaming support at all.

@PowerKiKi
Copy link
Member

We have #1292 to add support for writing to resources in a more general way. I believe that would make this PR obsolete.

@PowerKiKi PowerKiKi closed this Apr 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants