search

inveniosoftware / invenio-files-rest

REST API for uploading/downloading files for Invenio.
https://invenio-files-rest.readthedocs.io
MIT License
9 stars 68 forks source link

models: new object version tags model #132

Closed egabancho closed 7 years ago

egabancho commented 8 years ago
jirikuncar commented 8 years ago

Why do we need another key, value store? I thought that there was an agreement on storing additional metadata in records. Are you using it for filtering? Wouldn't be sufficient to have just a JSON field? It reminds be the more_info mapping from BibDocFiles.

egabancho commented 8 years ago

Why do we need another key, value store? I thought that there was an agreement on storing additional metadata in records

The other key, value store we have is at the level of the bucket, sometimes one needs to store some technical metadata with the file, which doesn't belong in the record metadata, i.e. the bitrate of a video file. Indeed we thought that it would be enough to store it in the record but as we work more with it, we see that it is more convenient and seems more natural to have it at level of the ObjectVersion instead.

Wouldn't be sufficient to have just a JSON field?

Do you mean adding a JSON field to the current ObjectVersion? We discuss this option, but I think it is better to keep the same structure (I like consistency) across the file storage module, Bucket - BucketTags, ObjectVersion - ObjectVersionTags

It reminds be the more_info mapping from BibDocFiles.

Well, yes, but I think it reminds Invenio 1.X time in the same way as invenio-indexer reminds BibIndex. Anyhow it seems that we are not the only ones thinking about that, amazon S3 has something similar and also Backblaze (although this last one is lest mature)

egabancho commented 8 years ago

It looks like it needs https://github.com/inveniosoftware/invenio-accounts/pull/167 to pass the tests.

lnielsen commented 8 years ago

Why do we need another key, value store?

I see a use case for ObjectTags for technical file metadata (files-rest) as opposed to descriptive metadata (record).

Wouldn't be sufficient to have just a JSON field?

I wouldn't go with a JSON field to ensure that we can align with S3:

egabancho commented 8 years ago

As I discuss IRL with @lnielsen, I have changed the primary key of ObjectVersion to be only version_id and have added a unique constraint to replace the old primary key.

Only the DB model should be affected by this change, no code change needed.

egabancho commented 8 years ago

ping 😉

jirikuncar commented 8 years ago

FYI this is a good candidate that could benefit from an Alembic upgrade recipe.

egabancho commented 8 years ago

🤔 I didn't know we are using Alembic now ... Anyhow, should we make migration recipes for packages in alpha versions? I thought the whole point of releasing packages in their early stages was to be able to be more agile in this aspect.

jirikuncar commented 8 years ago

@egabancho many packages are already running in production and we not released only due to lack of maintainers time. I see that the milestone is set to v1.0.0 so we can wait until all packages are in beta (I3B) with initial upgrade recipes.

egabancho commented 7 years ago

https://github.com/inveniosoftware/invenio-files-rest/pull/147