dahlia / sqlalchemy-imageattach

SQLAlchemy extension for attaching images to entities.
https://sqlalchemy-imageattach.readthedocs.io/
MIT License
116 stars 25 forks source link

Implement `File` abstraction #13

Open mattupstate opened 11 years ago

mattupstate commented 11 years ago

This library looks great and I was just thinking that it might be useful to abstract things a bit to allow for generic file attachments to a particular SQLAlchemy model/entity. Perhaps this can be achieved with the current API but I'm having a hard time seeing if its possible given the focus on images.

dahlia commented 11 years ago

If we will do more abstraction as you suggest, we should rename its name first of all. :smile: Frankly I’ve been asked the same needs several times before, but currently have no idea how to achieve that.

mattupstate commented 11 years ago

I was thinking the same thing about the name change, but what you have going is nice and I didn't think it would be too hard to pull out the image specific functions of the Image model/entity.

peterlada commented 10 years ago

+1 for this

mwhite commented 10 years ago

+1

One way to implement this could be:

  1. s/Image/File
  2. Non-image original files are stored with a width and height of 0
  3. Image (now File) delegates to a FileType implementation based on a File's mimetype, which provides thumbnail generation.
  4. For many non-image filetypes for which thumbnails can be generated, it makes sense to have multiple thumbnails of the same size, like one for each page of a document or one each for several frames of a video. They are also ordered. So we should add an additional index integer primary key column to File, which the default query will be ordered by.

Alright if I do this and submit a pull request?

peterlada commented 10 years ago

@mwhite +1

dahlia commented 10 years ago

@mwhite Yay, I’ll be pleased and review the patch if you submit it. Though the project name should be changed, too.

mwhite commented 10 years ago

On second thought, maybe it would be better to have separate Video and Document classes which extend File/Image and have a ms and page integer property, respectively, to be more explicit.

Then a question would be whether to use joined table inheritance or one of the other inheritance schemes.

Another issue is if we include the ability to generate thumbnails for different types of videos and documents, which ones should have the external libraries be hard dependencies vs optional dependencies, and how to handle that.

Also, generating thumbnails might be something that should be a background task independent of the save transaction, so perhaps there should be a way to configure sqlalchemy-imageattach to do that with a task queue.

peterlada commented 10 years ago

Michael,

that is way better. There's a lot of meta information that only makes sense for a given doc type. It could have a superclass that has no met info (beyond mime type) for those people who just want to attach files. (Dropbox use case)

--p

On Wed, Aug 20, 2014 at 1:34 PM, Michael White notifications@github.com wrote:

On second thought, maybe it would be better to have separate Video and Document classes which extend File/Image and have a ms and page integer property, respectively, to be more explicit.

Then a question would be whether to use joined table inheritance http://docs.sqlalchemy.org/en/rel_0_9/orm/inheritance.html#joined-table-inheritance or one of the other inheritance schemes.

Another issue is if we include the ability to generate thumbnails for different types of videos and documents, which ones should have the external libraries be hard dependencies vs optional dependencies, and how to handle that.

Also, generating thumbnails might be something that should be a background task independent of the save transaction, so perhaps there should be a way to configure sqlalchemy-imageattach to do that with a task queue.

— Reply to this email directly or view it on GitHub https://github.com/crosspop/sqlalchemy-imageattach/issues/13#issuecomment-52812762 .

timbueno commented 10 years ago

+1 for this. I would love a generic file implementation.

mwhite commented 9 years ago

I no longer have a need for this in my project so I probably won't be doing it like I said.