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

Image Store #165

Closed
bolismauro opened this issue Feb 7, 2014 · 10 comments
Closed

Image Store #165

bolismauro opened this issue Feb 7, 2014 · 10 comments

Comments

@bolismauro
Copy link

Hi everyone,
I would like to suggest to adopt a different approach to store images. Instead of creating a field type for a specific storage service (eg: cloudinary), keystone should expose an "image/images" fields with a "store" parameter. The store is an object that provides an interface to store/retrieve images from a remote service (S3, cloudinary...). In this way developers are able to add new ways to persist images without add a lot of filelds.
This approach is very similar to the Django ones and it seems more flexible to me.

PS: the same approach could be used for the email and the files :)

@mbayfield
Copy link
Contributor

+1

@blasterpistol
Copy link

+1 it would be nice

@JedWatson
Copy link
Member

Hey guys,

I like the idea, and it would be great to decouple the back-end services from the functionality a bit. Given how many providers could be integrated, it'll get a bit out of hand, and I really want Keystone to be as neutral as possible.

The services that have been integrated are really just there because there was a problem they solve well.

On a side note, one of the initial goals for Keystone was to have it work in a PAAS / multi-server clustered environment like Heroku, where you can't rely on persistent storage on the server. This is a limitation of other node.js cms platforms (not criticism; you can do other cool things when you can rely on the filesystem node.js is running on) but basically to work at scale, you have to distribute file handling to other services (internal or external) and keep your web servers light and disposable.

To get back to the topic, while I like the abstraction idea, I'm not so clear on how it would go down at a code level. For example: so much of the code in the cloudinaryimage field is tied to the underlying cloudinary functionality - not so much the uploading, but the fit / fill / scale (etc) methods you can use to display the images, and the schema structure it creates.

So if we created generic file / image field types, the service plugins behind them would impact the schema and api of the field type - because you wouldn't be able to call a .fit() method on a locally stored image the way you would a cloudinary image (unless we were to replicate the cloudinary functionality within Keystone), and wouldn't need to store a cloudinary_id.

I think it's important to have consistent functionality for each field type.

It's a simpler problem to solve with files than images, or for simple image fields (s3image and fsimage could have an identical API) so maybe the cloudinary integration is just an exception. Or maybe we split the fields based on their interface - any service that can conform to a field API and structure could become a 'store' plugin for it.

@bolismauro can you point me at the docs for the django feature you're referring to? it would be good to see how they've done it.

Another idea is to break out the non-core field types into 'plugin' packages - so if you wanted to use the S3 functionality, you could include a 'keystone-s3' package and it would provide you with the s3file and s3image field types. Similar for Cloudinary, Mandrill, etc. so Keystone isn't bundling functionality and dependencies that may not be used.

The only thing preventing that at this point is the level of integration there is between the field types and core functionality (e.g. special cases in the List and UpdateHandler classes) but as Keystone matures that can be consolidated.

@bolismauro
Copy link
Author

Hi,
here is the documentation of django files
https://docs.djangoproject.com/en/dev/topics/files/
Look at the last example, it creates an image field and then specifies that the storage to use is "FileSystemStorage".

The really cool thing is that developers can use packages like this
http://django-storages.readthedocs.org/en/latest/
to add storages in a really simple way.

The standard image model doesn't provide complex features (like resizing..) but you have to implement or import external code every time. This is mitigated by the fact that there are custom models too (like https://github.com/un1t/django-resized).

Back to keystone, I really like the idea to break out the non-core fields and import them using, for example, npm. It's a win-win solution because

  1. the keystone core will be simple and "easy-to-maintein"
  2. developers will be able to create and share their own fields (so you don't have to develop every single plugin on your own but the community can improve the project in a really easy way)

Anyway i haven't watched the source code of keystone yet, or at least not good enough to know if the implementation of this approach is a complex or simple thing.

@JedWatson
Copy link
Member

@bolismauro thanks for that. Those docs make sense given what I was saying before - the different stores abstract the upload / retrieval and deleting of files, but not the handling of them.

So we could certainly consolidate the local / s3 / etc handling of files and images, and it'd be a good thing. Cloudinary still stands apart though, due to the major difference in functionality.

I'm pretty sure we will support adding field types through packages in the future, I'm really looking forward to making Keystone more plugin-friendly. Might start by allowing new services to be plugged in behind generic file / image fields like you've suggested, that could be done pretty easily.

@edword
Copy link

edword commented Feb 9, 2014

👍

@jdarling
Copy link
Contributor

jdarling commented Feb 9, 2014

One way would be to have a base Image type with no ability to resize, fill, etc... Then offer up specialized types for the more advanced plugins (eg: Cloudinary) as additional plugins. Depending on the plugin you are using would provide the functionality you need. If you could come up with a way to overload plugins then something interesting happens as well:

Load in standard image store plugin, just gives you get, put, and list functionality.
Load in image utilities plugin, this adds in resize, fill, etc...

The only real complaint I (personally) have right now about Keystone is that it isn't modular at all. If it were based on a module (plugin whatever) system then it would be even better.

And by the way Jed, really loving Keystone as is right now. Just built a complete website in under a week and that included learning and hacking on Keystone. That should say something :)

@JedWatson
Copy link
Member

@jdarling that's great to hear, thanks :D

@bolismauro
Copy link
Author

I totally agree with @jdarling , modularity would be a huge improvement!

Anyway we were working on a django-like admin panel but we stopped when we saw this project. Keystone is awesome and we will start developing plugins/modules as soon as you release public apis!

@JedWatson
Copy link
Member

This is coming, closing this issue in favor of #526 which has a lot of momentum.

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

No branches or pull requests

6 participants