OPTIMADE icon indicating copy to clipboard operation
OPTIMADE copied to clipboard

Add `database` field to `meta` for describing the current database

Open ml-evs opened this issue 3 years ago • 4 comments
trafficstars

Closes #422.

This PR adds the optional database field to all meta responses. It can be used to provide the same data on an individual database as that found at the index meta-database, without requiring several requests and potential filtering of the index meta-database response.


Only other thought when writing this up is that this info could also be provided as a self entry in the links field for every response (though the fields would be limited to just using a meta field as part of a JSON API Links object).

If we prefer this route, then an alternative note could be added to the "JSON Response Schema: Common Fields" section of the spec that reserves the self keyword or something similar for this use, the only complication being that JSON:API self links should just return the exact URL of the current response.


Or one final option is just to add this kind of info at /info/ directly, and exclude it from meta.

ml-evs avatar Sep 29 '22 15:09 ml-evs

On line 655 there is still an example of a response. I think your database field should be added to this example.

Not sure it's super important, as database is an optional field (and other optional fields are also omitted in that example, like implementation).

ml-evs avatar Sep 30 '22 09:09 ml-evs

Yes you are right. As it is an optional field it is not so bad that it is not in the example on line 655.

JPBergsma avatar Oct 03 '22 09:10 JPBergsma

Suggestion in #422 describes two places where this info can be added: /info endpoint (preferred then by @ml-evs) or meta of every response. I would also prefer /info to retain smaller responses, but maybe there are more arguments backing meta proposal?

Edit: I made up my mind, I support the current proposal now.

merkys avatar Oct 03 '22 15:10 merkys

I agree with Merkys, that it would be sufficient to only place this info only at the info entry point. It is however a bit strange to return the data about the maintainer of the implementation with each response but not the data of the database maintainer. I think it would be best if both were moved to the info endpoint. This may however qualify as a breaking change, so perhaps we should do a major version update before implementing this.

JPBergsma avatar Oct 03 '22 15:10 JPBergsma

For parity with implementation, I think we should also allow to specify a version for the database from which the response comes.

Fine by me.

I agree that it make sense to (also?) allow database information in /info, but I can see reasons why people may want it in every response from their database. It isn't many bytes.

I guess as it stands these fields could also be provided in the meta response from /info. If we are happy having them in meta then we should not duplicate it in /info too. I wouldn't be against moving it to just /info but perhaps that is a v2.0 change.

We could also consider adding a query parameter that elides the meta response, which could, for example, be included in pagination links from an implementation by default. ?meta=false or ?hide_meta etc. We have added a few unbounded fields to meta (e.g., description could be the full text of a paper describing the provider/database) so perhaps this would be helpful as an optional parameter.

I'll re-request review from everyone now to see if anyone has remaining objections to adding this to meta.

ml-evs avatar Dec 31 '22 16:12 ml-evs

Regarding the placement, I noticed that "provider" is allowed to appear in "meta" of any response, thus it makes sense to me for "database" to follow suit (as is suggested here). Another point in favor of current proposal is provenance preservation - in case a database does not provide immutable IDs or URLs, this could be extracted from the response as a last-resort means. Therefore I am leaning to accepting (after merging develop in).

merkys avatar Jun 05 '23 15:06 merkys

If we are happy with the field names etc here then I think @rartino this is ready for a final review+merge?

ml-evs avatar Jun 13 '23 12:06 ml-evs