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

Doesn't work with assets on S3 #11

Open
madsem opened this issue Feb 2, 2019 · 6 comments
Open

Doesn't work with assets on S3 #11

madsem opened this issue Feb 2, 2019 · 6 comments
Assignees

Comments

@madsem
Copy link
Contributor

madsem commented Feb 2, 2019

When uploading assets in a container that uses the S3 driver, the assets are logged as successfully optimized, but never actually are.

I assume we should use either Guzzle or Curl, to download the asset if a container is S3 enabled, then optimizing it inside the tmp dir, and then moving/uploading it back to S3.

@madsem
Copy link
Contributor Author

madsem commented Feb 2, 2019

Actually no, not Guzzle/Curl. The way this should work so it’s compatible with all asset container drivers is to use the File Facade imho.

So assets should be moved only using ‘File’ and not by the built in optimizer libraries.

I think in general assets should be moved to tmp dir, then the optimizer library optimizes it there and copies it back to tmp, and then using the File Facade it has to be moved back to the asset container.

That way I believe S3 should also work automatically.

@madsem
Copy link
Contributor Author

madsem commented Feb 3, 2019

The more I think about it, the more I feel the addon should have an API for this?
The problem when using a cloud driver is, that not only would the ImageOptimizer download the asset that was just uploaded, and then optimize it and upload it again.

It would also connect to s3 for every filesize & lastmodified check...

If there was an API one could trigger the optimization when needed, and only optimize it on Glide events so that local copies are optimized. But for AssetUploaded events do nothing, and instead, make the optimization available through API.

Then again, on the other hand, as all these extra calls would only be made whenever an asset is newly uploaded, that might be okay as it doesn't happen on page load... And Glide cache would still be local even if assets are on s3, so still fast

Thoughts?

@4rn0
Copy link
Owner

4rn0 commented Feb 3, 2019

Hey man!

assets should be moved to tmp dir, then the optimizer library optimizes it there and copies it back to tmp, and then using the File Facade it has to be moved back to the asset container.

I think this would be the easiest way to get this working with S3 and other Flysystem drivers. But optimising the Glide manipulations is the most important thing here and I think the Glide cache is always on the local filesystem?

@4rn0 4rn0 self-assigned this Feb 3, 2019
@madsem
Copy link
Contributor Author

madsem commented Feb 3, 2019

Yes I think so too, Glide cache is always local, and the additional checks for filesize + lastmodified date, can all be done via the File facade, that would make it all compatible with all Flysystem drivers.
https://docs.statamic.com/addons/api/api-file

@4rn0
Copy link
Owner

4rn0 commented Feb 3, 2019

I'll set up a branch and implement some of the Flysystem stuff. I'm a bit swamped with client work right now, but I'll hopefully get something done this week!

@madsem
Copy link
Contributor Author

madsem commented Feb 3, 2019

Ok I'll probably send a pull request, as I'm implementing the flysystem stuff in a custom addon atm, then we can just look it over and you can merge it in after changes.

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

No branches or pull requests

2 participants