sqlalchemy-imageattach icon indicating copy to clipboard operation
sqlalchemy-imageattach copied to clipboard

Implement `File` abstraction

Open mattupstate opened this issue 12 years ago • 10 comments

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.

mattupstate avatar Sep 16 '13 13:09 mattupstate

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.

dahlia avatar Sep 16 '13 15:09 dahlia

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.

mattupstate avatar Sep 16 '13 15:09 mattupstate

+1 for this

peterlada avatar Jul 01 '14 01:07 peterlada

+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?

mwhite avatar Aug 07 '14 21:08 mwhite

@mwhite +1

peterlada avatar Aug 07 '14 22:08 peterlada

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

dahlia avatar Aug 08 '14 03:08 dahlia

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.

mwhite avatar Aug 20 '14 17:08 mwhite

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 [email protected] 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 .

peterlada avatar Aug 20 '14 17:08 peterlada

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

timbueno avatar Sep 30 '14 00:09 timbueno

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

mwhite avatar Oct 02 '14 19:10 mwhite