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

Add README.md text to help others host index meta-databases #18

Merged
merged 7 commits into from
Dec 21, 2020

Conversation

rartino
Copy link
Contributor

@rartino rartino commented Feb 20, 2020

This PR adds a segment of text to README.md that explains how one can easily fork this repository to host one's own index meta-database that fulfills the requirements to be added to OPTiMaDe's central list of providers.

A preview is visible at: https://deploy-preview-18--optimade-providers.netlify.com/

Copy link
Member

@CasperWA CasperWA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very helpful! Thanks @rartino.
Now we have two official ways of creating index meta-databases (the other being Docker deployment via optimade-python-tools). Hopefully that lowers the barrier-of-entry significantly.

I found one change that should be applied (if I'm not mistaken), and then also a rather interesting concept I would like to discuss further - it's something we definitely should have, in my opinion.

README.md Outdated Show resolved Hide resolved
README.md Outdated
Comment on lines 64 to 76
- Include also the following segment:
```
{
"type": "parent",
"id": "optimade_index",
"attributes": {
"name": "OPTiMaDe providers",
"description": "OPTiMaDe index meta-database of known providers",
"base_url": "https://providers.optimade.org",
"homepage": "https://www.optimade.org"
}
}
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is new!
I like it - but it defeats our idea of "supporting" only two levels (as we write in the spec here) - whatever support means in this context.
I thought the way to include the providers, was simply to include them with the type provider, again, as is indicated in the spec here.
However! I am all for not replicating too much, i.e., having the same providers spread out over all the different implementations. And having the option to direct the client to a continuously updated list of providers seems to be a good one to me.

My only issue with this then, is actually how we should present this link to the list of providers. While this may be one way, it is a bit misinformative - since the list of providers is not a parent of the index meta-database, but rather it's on the same level (in some sense).
Another option would be to introduce a fourth type, like the plural of provider, i.e., "providers" or a combined type of "provider_list".
Another option would be to introduce a type that refers to implementation on the same "level", like "sibling" (to follow on with the child and parent nomenclature), but that feels a bit mis-informative as well, I feel.

So in the end, I am open to ideas, and it may even end up with what you have here. I would just like to raise the discussion to make sure it all makes sense with the rest of the spec (or adapt the spec accordingly).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh... I had somehow gotten the idea that this was how it was supposed to work so that an unbroken tree was presented to the client, which could be traversed both up and down. But you are right that the present specification certainly do not mandate this setup. In particular, the fact that providers are linked out with provider-type objects instead of child already breaks this symmetry.

So, for now I should just remove this segment. But I like your version with a provider_list link.

Copy link
Contributor Author

@rartino rartino Feb 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, maybe my confusion ties into Materials-Consortia/OPTIMADE#217 a bit. Should these link objects really have these kinds of different types, or would it be better to change them to be links-type and put the relationship as an attribute field?

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Co-Authored-By: Casper Welzel Andersen <43357585+CasperWA@users.noreply.github.com>
giovannipizzi
giovannipizzi previously approved these changes Jun 10, 2020
@giovannipizzi
Copy link
Contributor

Link types should be changed according to this issue comment as soon as this is officially merged in the OPTIMADE specs.

@CasperWA CasperWA dismissed giovannipizzi’s stale review June 10, 2020 15:44

The changes mentioned by @giovannipizzi should be implemented in the README.md

@rartino
Copy link
Contributor Author

rartino commented Jun 11, 2020

I've updated this to confirm with the latest changes and utilize PR #34.

@giovannipizzi
Copy link
Contributor

@rartino After #34, do we still want to have people create a new repo to have a meta-DB? (maybe yes if they want to host it still themselves to decide how often to make changes?)

I would then wait for #34 to be merged, clarify when one should create a new repo vs. just adding the files here, and check again the docs you wrote (I had to do some renaming in #34 after you updated this PR)

@rartino
Copy link
Contributor Author

rartino commented Jun 12, 2020

@rartino After #34, do we still want to have people create a new repo to have a meta-DB? (maybe yes if they want to host it still themselves to decide how often to make changes?)

@giovannipizzi Yes, this is still relevant. I'm going to use this setup to keep an index database that remains under my control.

You can look at it as this, there is a range of user needs:

  1. "I will never provide anything but a single OPTIMADE URL".
  2. "I will do changes now and then".
  3. "Our system dynamically generates databases on the fly"

Users category 1 hosts their index meta-database in the providers repo. Cloning the repo is for users in category 2.

@giovannipizzi
Copy link
Contributor

Ok I see, fine! I'll wait for #34 to be merged first before reviewing, as there might be further changes requested that might affect this PR

Copy link
Member

@CasperWA CasperWA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should just get this in by now. It's good information.

@giovannipizzi
Copy link
Contributor

There're some conflict that need to be solved before merging

@CasperWA
Copy link
Member

@rartino I will resolve the conflicts here and merge this.

@CasperWA CasperWA merged commit e2e6a53 into Materials-Consortia:master Dec 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants