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

Added league/flysystem to also be a possible filesystem #213

Merged
merged 3 commits into from
Jan 28, 2016

Conversation

lsv
Copy link
Contributor

@lsv lsv commented Jan 19, 2016

Integrated Flysystem so not only gaufrette is a option

@lsv
Copy link
Contributor Author

lsv commented Jan 19, 2016

Could someone test it with s3?

I will also write some documentation on how to use it, if accepted

@lsv
Copy link
Contributor Author

lsv commented Jan 19, 2016

Tested with

  • Local
  • Dropbox
  • Orphange

@bytehead
Copy link
Member

Hi @lsv ! Thanks for your PR, looks really good to me. And thanks for testing! My bad, that I can't test it with s3...

@lsv
Copy link
Contributor Author

lsv commented Jan 20, 2016

The failed tests are PHP 5.3 as flysystem requires PHP 5.4
I would suggest not support 5.3 anymore?

@bytehead
Copy link
Member

Yes, I saw it. I would say we can skip 5.3 due to the fact that 5.3 is EOL since 14th of august 2014.

@lsv
Copy link
Contributor Author

lsv commented Jan 20, 2016

And the reason for using flysystem imo gaufrette is somekind of dead, and lot of other bundles are using flysystem instead.

@bytehead
Copy link
Member

Can you drop the 5.3 support in travis.yml within this PR?

@rvmourik
Copy link

Is there an estimate when this fork will be merged? I'm looking forward in using flysystem with this bundle

bytehead added a commit that referenced this pull request Jan 28, 2016
Added league/flysystem to also be a possible filesystem
@bytehead bytehead merged commit 6d94579 into 1up-lab:master Jan 28, 2016
@bytehead
Copy link
Member

@rvmourik 1.5.0 is out :)

@rvmourik
Copy link

@bytehead Thanks, that was pretty quick :-)

@bytehead
Copy link
Member

It was on my list for at least 8 days ;-)

@bytehead
Copy link
Member

@lsv: @sheeep is pretty happy about your PR and much thanks from him as well! 🎉

@Sander-Toonen
Copy link

Wow, this is very good news! It might need some documentation though.

Is it as straightforward as replacing gaufrette with flysystem? Does it support chunked uploading?

@lsv
Copy link
Contributor Author

lsv commented Jan 29, 2016

When I have a few moments i will write a proper documentation.

But for short, change gaufrette to flysystem in storage type.
And then setup flysystem bundle

  • Orpange is tested
  • Dropbox is tested (so maybe other external ones works - havent tested with others, please do if you have access to other external storages)
  • Local also tested :)
  • streamwrapper is not fully tested, but I will do this one of the following days, as I just started with a project based on flysystem (hence this PR ;-)

@lsv
Copy link
Contributor Author

lsv commented Jan 29, 2016

@bytehead well it actually took some time for me to find out that 1up-lab also have flysystem bundle, so I was a bit confused why you have chosen gaufrette :)

Anyways, it was actually pretty easy to create this, when I figured out that FileInterface missing a few methods, like move() and so on - I will properly make a PR for this.

And also a PR for documentation will come in a near future

@bytehead
Copy link
Member

@lsv I can totally understand your confusion 😄
The uploader bundle came out way before our flysystem bundle - that's probably why it ended up like this...

Sounds great - your PRs are always welcome!

@SrgSteak
Copy link
Contributor

SrgSteak commented Feb 6, 2016

I get a OutOfMemoryException on oneup/uploader-bundle/Oneup/UploaderBundle/Uploader/Storage/FlysystemStorage.php line 47 when uploading a larger file (~400MB). The chunked upload works, but the last step seems to cause the exception.

Any ideas?

UPDATE:
It seems the file_get_contents($file) is the cause. I replaced it with
$stream = fopen($file->getPathname(), 'r+');
$this->filesystem->putStream($name, $stream);
if (is_resource($stream)) {
fclose($stream);
}
and it works. Anyone interested in a PR?

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