OPTIMADE v1.2 release planning

[ml-evs]

Following from @merkys's comment on the mailing list, we should begin preparations for a v1.2 release.

I think I also foolishly "volunteered" to manage this release a while ago, but please help out if you want to. As part of this process, I've been doing a bit of issue triaging -- please re-open any issues that I closed that you think are still valid. In particular, I've also been clearing up the v1.2 milestone https://github.com/Materials-Consortia/OPTIMADE/milestone/3 which should now reflect what I write below.

We can use the comments here to have some async discussion that can perhaps be finalized at the final meeting this year.

Summary

As far as I see it, we need to decide:

1. Whether to release v1.2 before the paper
2. Whether to release and wait for providers to implement v1.2 before the paper
3. Whether to write or merge any more PRs before releasing v1.2

My suggestion would be to finalize any PRs/issues we know we want in before the next meeting, then make a release candidate before Christmas. We can write the paper based on that release candidate and hopefully servers will start picking up the changes too (we can certainly get most of it into optimade-python-tools in January). We can submit the paper whenever with references to the release candidate, ready to be updated with the real release version in time for its (hopeful) publication.

Proposed changelog (updated 22/06)

I've just made a draft v1.2.0 release on GitHub (that everyone in the organization should be able to see) --- we could also convert this into a proper release candidate sooner rather than later v1.2.0-rc1, if desired. This draft has the following abridged (removed typo/testing PRs) changelog:

• Allow provider-specific nested properties within OPTIMADE properties. by @JPBergsma in https://github.com/Materials-Consortia/OPTIMADE/pull/401
• Space group additions: space_group_hall and space_group_it_number by @merkys in https://github.com/Materials-Consortia/OPTIMADE/pull/405
• Allow boolean values in queries by @merkys in https://github.com/Materials-Consortia/OPTIMADE/pull/348
• Files endpoint by @merkys in https://github.com/Materials-Consortia/OPTIMADE/pull/360
• Property definitions by @rartino in https://github.com/Materials-Consortia/OPTIMADE/pull/376 (plus several associated tweaks and fixes)
• Support fuzzy comparison lists by @merkys in https://github.com/Materials-Consortia/OPTIMADE/pull/415
• Backoff time by @merkys in https://github.com/Materials-Consortia/OPTIMADE/pull/411
• Means to specify database license by @merkys in https://github.com/Materials-Consortia/OPTIMADE/pull/414
• Means to provide partial entries and pagination mechanism through properties @rartino https://github.com/Materials-Consortia/OPTIMADE/pull/467
• Per entry and per property metadata by @JPBergsma https://github.com/Materials-Consortia/OPTIMADE/pull/463
• Update to JSON:API v1.1 by @rartino https://github.com/Materials-Consortia/OPTIMADE/pull/461
• Adding type and id to entry info endpoints by @ml-evs https://github.com/Materials-Consortia/OPTIMADE/pull/472
• Adding database describing fields to meta by @ml-evs https://github.com/Materials-Consortia/OPTIMADE/pull/424

• #445
• #473
• #464

Outstanding PRs/issues

PRs that seem to required more time in the oven (and perhaps another round of in-person discussion):

• #386

• #456

• #455

• #377

• #481

• #206

Handled by future definition namespaces:

• #396 / #400
• #392 & #398 are both mature PRs with a lot of discussion that would probably benefit from being included in a pre-release so that we can begin implementing them in our servers (and other associated SMILES PRs #436 / #466).
• #465

There are also some potentially blocking issues to address:

• ~#416 - plenty of good discussion, but needs quite an extensive PR adjusting the merged space group functionality after #405. The current proposal also adds a definition of fractional site positions.~
• ~#102 - although #414 was merged, there is ongoing discussion about per-entry licensing. It seems the consensus is that this should be added to the free text license info for the overall database, but we should confirm this at the next meeting.~
• #410 c.f. #206
• #469 and #479

[merkys]

Thanks for putting this together @ml-evs. Many of the features included in the current manuscript have been added post-v1.1, thus I think releasing v1.2 before publication would be a good idea.

#416 seems to be a real blocker. @rartino did a great job by summarizing the consensus, but it may take some time to convert that to a PR and merge it since a lot of new properties are proposed to be added.

#102 probably can be left out for this release (@blokhin, will you be able to support v1.2 with a per-database license file?)

#392 and #398 may be too far from reaching consensus at the moment. I am drafting an alternative proposal to implement #368, but I do not think it will be accepted any sooner.

[blokhin]

@merkys per-database license file should be no problem! In general, I'd like to support the new release at the MPDS as early as possible.

[ml-evs]

The changes in #414 are actually somewhat breaking, as we are introducing a new mandatory info field license. Are we happy that this is introduced in a minor version release? (I vaguely remember a discussion of an implicit CC-BY if the license was not provided in this field)

[merkys]

The changes in #414 are actually somewhat breaking, as we are introducing a new mandatory info field license. Are we happy that this is introduced in a minor version release?

Oh, right. To stick to minor version we probably need to demote license to an optional field and define a default behavior ("check the website" as before, I assume?).

(I vaguely remember a discussion of an implicit CC-BY if the license was not provided in this field)

I think it is safer to say "do not assume anything if not given". This seems backwards-compatible to me :smiley:

[ml-evs]

Well, the property definitions PR definitely introduces many breaking changes to /info/<entry> too. We might need to discuss whether this is really already OPTIMADE 2 :)

[rartino]

Did we not agree to interpret SemVer for communication protocols to say that a backwards-breaking protocol change is one that breaks clients written to the older specification, but not one that requires change of the server code to conform to the newer version?

In that case, is adding a new mandatory field breaking? I suppose that, yes, very strictly speaking because we never explicitly say that a client MUST NOT crash on unrecognized non-database-specific fields. Still, I thought we agreed that this is just an omission we should just add as a clarification at some point.

I don't think the property definitions PR is backwards breaking (but I have not checked very carefully yet). In defining type in the info endpoint for OPTIMADE v1.1, I specifically insisted on inserting the following to make it possible to add this feature without being backwards-breaking: "For the purpose of compatibility with future versions of this specification, a client MUST accept values that are not string values specifying any of the OPTIMADE Data types, but MUST then also disregard the type field." I think that addresses the most clear place where the change could have been breaking.

[ml-evs]

Did we not agree to interpret SemVer for communication protocols to say that a backwards-breaking protocol change is one that breaks clients written to the older specification, but not one that requires change of the server code to conform to the newer version?

Sure (and we should write this up as I end up requesting to re-litigate this point every time...), but pragmatically, each of the providers will have a non-zero amount of work to do to support v1.2. I'm going through that process now for optimade-python-tools and could potentially have something ready for our next meeting on the 22nd.

In that case, is adding a new mandatory field breaking? I suppose that, yes, very strictly speaking because we never explicitly say that a client MUST NOT crash on unrecognized non-database-specific fields. Still, I thought we agreed that this is just an omission we should just add as a clarification at some point.

I don't think the property definitions PR is backwards breaking (but I have not checked very carefully yet). In defining type in the info endpoint for OPTIMADE v1.1, I specifically insisted on inserting the following to make it possible to add this feature without being backwards-breaking: "For the purpose of compatibility with future versions of this specification, a client MUST accept values that are not string values specifying any of the OPTIMADE Data types, but MUST then also disregard the type field." I think that addresses the most clear place where the change could have been breaking.

Thanks for pointing out that note, I hadn't spotted it. Maybe that digs us out of the hole on a technicality, but I'll still need to update the validator and client behaviour in optimade-python-tools to work with any new 1.2 databases. This does lead to a bit of an explosion of work in terms of supporting multiple v1 versions simultaneously, but perhaps that is unavoidable. As I said above, I'll try to present the work it required to support "v1.2" in optimade-python-tools and can make an informed decision then.

[rartino]

In that case, I don't think the key question is if the next version number is v1.2 or v2.0, but whether we need to go through the new features merged into develop and see if there are features that require so much work that we rather omit them from this release (saving them for the next release).

On the other hand, doesn't the new features mostly amount to optional features and a few new required fields? I get the point that to fully validate the info endpoint properties field is significant work (and lets then not talk about /trajectories if we merge that before the release...), but does the validator have to handle this before the release? Can't the validator just be changed to ignore the new mandatory fields under properties and type if it is unrecognized?

[ml-evs]

In that case, I don't think the key question is if the next version number is v1.2 or v2.0, but whether we need to go through the new features merged into develop and see if there are features that require so much work that we rather omit them from this release (saving them for the next release).

Agreed, in my comment I was also conflating the issue of who will actually have time to implement these features before we talk about them in the paper! We can see if anyone has any objections to the changeset outlined in the OP at the next meeting, and to vaguely cross-post my answer from there, I guess we are looking at January-February for a specification release now anyway.

but does the validator have to handle this before the release? Can't the validator just be changed to ignore the new mandatory fields under properties and type if it is unrecognized?

Yes, I'm fine with this, although the knock-on effect will be that the fuzzy query aspect of the validator will not be able to run -- assuming that providing properties in the old style is no longer supported (which seemed to be the case from last reading of the spec?).

[rartino]

but does the validator have to handle this before the release? Can't the validator just be changed to ignore the new mandatory fields under properties and type if it is unrecognized?

the knock-on effect will be that the fuzzy query aspect of the validator will not be able to run -- assuming that providing properties in the old style is no longer supported (which seemed to be the case from last reading of the spec?).

I'm not opposed that we add back the option for the old types to the specification alongside the "new" ones, with a "SHOULD"-level suggestion that one rather should use the new ones. It may be very helpful for people upgrading their code. Should we do that? I'd still mean to require the new mandatory fields, just that type is allowed to be a string of a single OPTIMADE type.

[ml-evs]

Suggestion is now to have release candidate 2 out by next week, at which point we can write the paper etc and hopefully find anything that breaks during implementations.

[ml-evs]

The consensus in today's meeting now seemed to shift back to delaying the release for several months until all of the PRs listed above are merged, and the paper will be written in such a way that assumes they will be merged, with the intermediate time being for initial implementations.

It's not entirely clear to me that this opinion is unanimous so please comment here if you disagree!

[rartino]

@ml-evs Should we really close this before we actually release v1.2.0 proper? It is (still) a pinned issue :)

[ml-evs]

Hmm, yes, this closed automatically...