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

Feature/gaufrette storage #353

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

kevincerro
Copy link

@kevincerro kevincerro commented May 3, 2020

This adds Gaufrette support to handle file uploads.

Some improvements can be made, feel free to suggest any necessary changes.

Solves #316

@coveralls
Copy link

coveralls commented May 3, 2020

Coverage Status

Coverage decreased (-44.5%) to 55.214% when pulling 30a75da on kevinc123:feature/gaufrette-storage into 0b2dcc1 on craue:master.

@Warxcell
Copy link
Contributor

Warxcell commented Jun 25, 2020

Really cool. I like it but I would rather make it one more level of abstraction. This will make possible also to use FlySystem for storage. And it will also remove the hard dependency of Gaufrette.

Something like BlobStorage interface and pass concrete implementation to DataManager. Current implementation of file storage can be also moved to SessionBlobStorage implementation for example. This will save the many if - else and make the code looks cleaner.

@fdiedler
Copy link

fdiedler commented Feb 18, 2022

@kevincerro @Warxcell @craue Nice feature. I use Gaufrette to store images on AWS because my PAAS does not have a filesystem.
Does this feature will be merged ?
Thanks,

@craue
Copy link
Owner

craue commented Feb 21, 2022

I see a lot of work went into this already. And basically, I also like the idea of being able to save files in different ways. But @Warxcell has a good point on abstraction. Adding adapters for soft dependencies to various libraries would be nice. Tests would be needed as well.

Anyone up for working on this? @kevincerro, are you in the mood to resume?

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

Successfully merging this pull request may close these issues.

5 participants