Materials-Consortia / optimade-python-tools

Tools for implementing and consuming OPTIMADE APIs in Python
https://www.optimade.org/optimade-python-tools/
MIT License
71 stars 44 forks source link

Implement version negotiation for validator #662

Open CasperWA opened 3 years ago

CasperWA commented 3 years ago

@ml-evs I think the error occurring here is not the fault of the index meta-database, but rather our validator (see here).

Basically it shouldn't rely on the unversioned base URL endpoints, but rather the mandatory vMAJOR-versioned base URL endpoints.

Originally posted by @CasperWA in https://github.com/Materials-Consortia/providers/issues/57#issuecomment-755273260

In essence, the validator shouldn't fail if an unversioned base URL is not present. This is not a MUST have for an implementation according to the specification, only the vMAJOR-versioned base URL is absolutely mandatory, and hence should be the initial source of truth for the validator.

ml-evs commented 3 years ago

Are you suggesting that the validator should do the version negotiation itself when given an unversioned URL? We could certainly do that. Would you still want a mode to validate just the implementation served at the unversioned URL too?

CasperWA commented 3 years ago

I would indeed think that the /version endpoint should be utilized, as it is also mandatory. From there the mandatory versioned base URL can be deduced and used.

Concerning the unversioned URL, I think it could be checked optionally, i.e., in the same tier as the other optional versioned base URLs? Of course, specifically for the unversioned base URLs, we need to check that the meta -> api_version value matches the one from the vMAJOR-versioned base URL, as I believe this is the criterium/what is expected from the unversioned base URL, right?

ml-evs commented 3 years ago

What is there that you can't do at the moment by just specifying v1 in the URL you pass to the validator?

CasperWA commented 3 years ago

True. My point here is more that if I supply an unversioned base URL, the validator should recognize this and first go for some version negotiation? I'll change the title of the issue accordingly.

I have also lowered the priority to medium, since your point is indeed true, I can simply provide it the versioned base URL.

ml-evs commented 3 years ago

Potential solution for this: if given a versioned URL, always test that /versions exists after you strip the version from it (rather than just checking that e.g. /v1/versions doesn't exist). I think this minimises impedance mismatch with usage of the validation-action whilst enabling those who don't have full unversioned implementations to use the validator

ml-evs commented 3 years ago

Potential solution for this: if given a versioned URL, always test that /versions exists after you strip the version from it (rather than just checking that e.g. /v1/versions doesn't exist). I think this minimises impedance mismatch with usage of the validation-action whilst enabling those who don't have full unversioned implementations to use the validator

I've just implemented this as part of #665.

CasperWA commented 3 years ago

Am I right in thinking that this issue has been resolved @ml-evs?

ml-evs commented 3 years ago

Ish - we still don't do version negotiation properly, e.g. if you give it an unversioned URL it will try to find a whole implementation there (otherwise how would you validate this). If you give it a versioned URL it checks if /versions is present at the unversioned URL.

I think the suggestion here was to automatically validate e.g. v1 after finding it in the version endpoint (perhaps if it also finds an info endpoint at the unversioned URL it could validate it, or skip it if it does not, rather than erroring).

We also have to start worrying about people hosting v1.1 endpoints, and the models we would then use for validation...

CasperWA commented 3 years ago

Right - so the motivation I had for originally suggesting this is that it's only the /vMAJOR versioned base URL that's mandatory. So if one could validate the version negotiation to get to that versioned base URL and test that as default (not anything else unless specified) that'd be optimal for "minimum requirements"...

Concerning this comment

We also have to start worrying about people hosting v1.1 endpoints, and the models we would then use for validation...

I thought we decided to simply keep the models in this package up-to-date with the latest spec version (at least for v1)? So we simply need to make sure we change the minor/major version of this package when we change the specification version support as well. Just release a version that only has commits to up the specification number. Or something like that? But I think that's not a discussion for this particular issue 😅