search

silverstripe / silverstripe-s3

Silverstripe module to store assets in S3 rather than on the local filesystem (SS4/SS5 only)
BSD 3-Clause "New" or "Revised" License
20 stars 25 forks source link

RFC Move the caching to a separate module or to `silverstripe/assets` #16

Closed maxime-rainville closed 5 years ago

maxime-rainville commented 6 years ago

Most of the plugin has nothing to do with S3 and just provides caching for low latency filesystem. It's coded in a generic way. You could literally give you site a different adapter and it would just work.

I think it should be in a separate non-S3 specific module or in the core assets module.

The following files would need to be moved.

This would only leave to S3 specific files: ProtectedS3Adapter and PublicS3Adapter.

obj63mc commented 5 years ago

With this module now using flysystem's cache adapter, not sure if this is needed any more. Referencing #19 and #20

madmatt commented 5 years ago

@maxime-rainville Do you think this is still relevant? It's a bit more specific to S3 now, and relies on flysystem caching for metadata cache now, so is a little less generic.

We already have the assets module and flysystem itself as generic modules, I'm not sure we need another one? We can just create modules for another other filesystem we want to use (e.g. Rackspace, Azure Blob Storage or similar), hopefully leveraging on an existing Flysystem adapter for the hard work :-)

If you think this is still relevant feel free to re-open!

maxime-rainville commented 5 years ago

Sounds like your latest changes make this idea redundant. All good.