search

Materials-Consortia / schemas

This repository hosts the permanent URLs to OPTIMADE schemas
MIT License
0 stars 2 forks source link

How should these schemas be updated? #5

Closed ml-evs closed 3 years ago

ml-evs commented 3 years ago

Since the release of v1.0.0, we have made improvements to the schema, whilst still following the release text. Should we update the schemas here with the best approximation of the v1.0.0 specification? This will just be a tweak on the version presently in the specification repo, minus any fields we have added since v1.0.0.

rartino commented 3 years ago

Given that the schemas provided here are the ones in the repository for the v1.0.0 release, does it not seem a bit weird to update them without a patch release with the new versions. We would essentially be modifying schemas without having version numbers to distinguish them by?

If we are firm on not supporting point releases at all, then we probably shouldn't "fix" broken schemas in old point releases either?

ml-evs commented 3 years ago

Given that the schemas provided here are the ones in the repository for the v1.0.0 release, does it not seem a bit weird to update them without a patch release with the new versions. We would essentially be modifying schemas without having version numbers to distinguish them by?

Yeah, this is the crux of my question really... the trouble is we know that these 1.0.0 schemas are "wrong" in that they did not properly validate with the swagger tools (due to a pydantic bug), so they are not super useful, especially if this is the link that people are expected to point to.

If we are firm on not supporting point releases at all, then we probably shouldn't "fix" broken schemas in old point releases either?

I'm not against the idea that the schemas in this repository always provide our best approximation of that version of the human-readable specification, as opposed to the version that was bundled with the release itself. Another option would be to release a spec v1.0.1 that ONLY updates the schema in the spec repo, excluding all other changes we had made.

ml-evs commented 3 years ago

Here's the bug I mention above, as reported in https://github.com/Materials-Consortia/OPTIMADE/issues/314 https://validator.swagger.io/validator/debug?url=https://raw.githubusercontent.com/Materials-Consortia/schemas/master/openapi/v1.0/optimade.json

rartino commented 3 years ago

I'm not against the idea that the schemas in this repository always provide our best approximation of that version of the human-readable specification, as opposed to the version that was bundled with the release itself.

We open for a lot of potential frustration if there is no version identifier that changes when we do this, and no way to reference an older schema that was replaced. I really want there to also be a fixed URL under schema.optimade.org where one can reference a specific version of a schema that never changes (flawed or not).

Another option would be to release a spec v1.0.1 that ONLY updates the schema in the spec repo, excluding all other changes we had made.

But if we do this, there really is very little reason not to conservatively select commits which are just patches to that v1.0.1, and thus we are at a "normal" v1.0.1 patch release. Which there is nothing wrong with.

Since we took the decision to release v1.1.0, I can take it onto myself to make a v1.0 branch at a point before the issue tracker commit, cherry-pick every patch change since then, and then you can push the updated schemas to that branch. Does that sound ok?

rartino commented 3 years ago

Ok, there is now a v1.0 branch: https://github.com/Materials-Consortia/OPTIMADE/tree/v1.0

Are the schemas correct in this version? If you are ok with us staging a v1.0.1 release from this branch we can then update the schemas here. However, we should create new https://schemas.optimade.org/openapi/v1.0.x/optimade.json that points to the respective schemas, and have https://schemas.optimade.org/openapi/v1.0/optimade.json point at the latest one.

How is it with github pages and symlinks? Can we just make openapi/v1.0 a symlink to openapi/v1.0.1 ?

ml-evs commented 3 years ago

This all sounds good to me @rartino. The schemas look fine, though bear in mind they also include the mass->species becoming a list rather than a float, although we convinced ourselves that this wasn't really a breaking change.

I think symlinks should work... happy to test either way

rartino commented 3 years ago

bear in mind they also include the mass->species becoming a list rather than a float, although we convinced ourselves that this wasn't really a breaking change.

The point being that this is a fix both in the specification and in the schemas of a specification bug, right?

I'm a bit reluctant against just trying this in the repository since it is bad form to run "experiments" on our stable immutable URL provider. But on the other hand, is there anyone that presently rely that hard on this service yet? Perhaps it is ok to try quickly; people must in any case be prepared for outages; github pages do go down from time to time.

ml-evs commented 3 years ago

bear in mind they also include the mass->species becoming a list rather than a float, although we convinced ourselves that this wasn't really a breaking change.

The point being that this is a fix both in the specification and in the schemas of a specification bug, right?

:+1:

I'm a bit reluctant against just trying this in the repository since it is bad form to run "experiments" on our stable immutable URL provider. But on the other hand, is there anyone that presently rely that hard on this service yet? Perhaps it is ok to try quickly; people must in any case be prepare for outages; github pages do go down from time to time.

I don't think anyone is relying on this, but we could test it with some dummy files in https://github.com/Materials-Consortia/materials-consortia.github.io...

Not necessarily related to this repo, but I am slightly worried about all the things that will be broken by having multiple existing spec versions, e.g. the providers dashboard https://github.com/Materials-Consortia/providers-dashboard/issues/52 and the validator action https://github.com/Materials-Consortia/optimade-validator-action/issues/66.

rartino commented 3 years ago

The documentation says symlinks are supposed to work, so I think I'll just do the adjustment in master and verify that nothing breaks.

Not necessarily related to this repo, but I am slightly worried about all the things that will be broken by having multiple existing spec versions, e.g. the providers dashboard Materials-Consortia/providers-dashboard#52 and the validator action Materials-Consortia/optimade-validator-action#66.

Oh, right, the validator probably needs to become aware of different versions (or different validators run, depending on version). But that is the case essentially regardless on how we handle versioning.

rartino commented 3 years ago

The symlink solution and v1.0.0 is now in place and seems to work. So, all we need to do once we are releasing v1.0.1 is to upload the schemas here and re-point the symlink.

But, there seem to be some additional patches being collected right now for v1.1.0, so perhaps we should hold off releasing v1.0.1 until those are in place.

rartino commented 3 years ago

@ml-evs Thanks for the work with releasing v1.1.0. I've updated the v1.0 branch to contain everything relevant from v1.1.0.

Just to make perfectly sure: the schemas that now are in that branch are the ones you want to replace the ones currently served at:

right?

Do you want me to create the v1.0.1 release, or should you?

Do you know how GitHub reacts when one releases a version with a lower version number after one with a higher number? I still want the "Latest release" to point to v1.1.0, and v1.1.0 to stay on top of the list of releases. (But I suppose we can "release" v1.0.1 by just tagging it in git, without creating a GitHub release for it.)

ml-evs commented 3 years ago

@ml-evs Thanks for the work with releasing v1.1.0. I've updated the v1.0 branch to contain everything relevant from v1.1.0.

Just to make perfectly sure: the schemas that now are in that branch are the ones you want to replace the ones currently served at:

* https://shemas.optimade.org/openapi/v1.0/optimade/optimade.json

* https://shemas.optimade.org/openapi/v1.0/optimade/optimade_index.json

right?

They are the correct schemas, but you will need to add a commit that changes the version from 1.0.0~develop to 1.0.1.

Do you want me to create the v1.0.1 release, or should you?

I don't mind, I'm happy to do it next week as I've just gone through the process again more recently.

Do you know how GitHub reacts when one releases a version with a lower version number after one with a higher number? I still want the "Latest release" to point to v1.1.0, and v1.1.0 to stay on top of the list of releases. (But I suppose we can "release" v1.0.1 by just tagging it in git, without creating a GitHub release for it.)

This is a good point... I'm pretty sure the releases will just be displayed chronologically. Just setting the tag is a useful workaround, as we will still get the automatic deployment to Zenodo but the order will not change).

ml-evs commented 3 years ago

I've updated the CHANGELOG on the v1.0 branch in the spec repo, and will make a PR that merges in the v1.0.1 changelog into the develop branch. Once this is accepted, I will tag the HEAD of the v1.0 as v1.0.1.

ml-evs commented 3 years ago

I think this can now be closed, we have all of the schemas up to date under schemas.optimade.org, and I've released v1.0.1 of the spec (for which I just set a tag, and not a full release, as we discussed above).