-
Notifications
You must be signed in to change notification settings - Fork 37
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
OPTIMADE v1.2 release planning #429
Comments
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. |
@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. |
The changes in #414 are actually somewhat breaking, as we are introducing a new mandatory info field |
Oh, right. To stick to minor version we probably need to demote
I think it is safer to say "do not assume anything if not given". This seems backwards-compatible to me 😃 |
Well, the property definitions PR definitely introduces many breaking changes to |
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 |
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.
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. |
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 |
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.
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?). |
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 |
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. |
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! |
@ml-evs Should we really close this before we actually release v1.2.0 proper? It is (still) a pinned issue :) |
Hmm, yes, this closed automatically... |
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:
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:
space_group_hall
andspace_group_it_number
by @merkys in Space group additions:space_group_hall
andspace_group_it_number
#405type
andid
to entry info endpoints by @ml-evs Addtype
andid
to the entry info endpoints #472database
describing fields tometa
by @ml-evs Adddatabase
field tometa
for describing the current database #424Outstanding PRs/issues
PRs that seem to required more time in the oven (and perhaps another round of in-person discussion):
Collections endpoint #386
Adjust handling of unknown properties #456
Add
optimization_type
property for structures #455Adding the trajectories endpoint to OPTIMADE #377
Adding the
property_ranges
query parameter and associated metadata #481Property 'fractional_site_positions' #206
Handled by future definition namespaces:
filter_smarts
) #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 SMILES data type #436 / InChIKey property #466).bonds
for structure type entries #465There are also some potentially blocking issues to address:
Insufficient space group descriptions #416 - plenty of good discussion, but needs quite an extensive PR adjusting the merged space group functionality after Space group additions:space_group_hall
andspace_group_it_number
#405. The current proposal also adds a definition of fractional site positions.Data(base) licenses #102 - although Means to specify database license #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.The text was updated successfully, but these errors were encountered: