Move generic schemas from Resource to kinto.core #1054
Conversation
This is needed to allow importing generic schemas on other parts of kinto core without breaking the import order. Related to Kinto#1033
gabisurita
force-pushed
the
move-generic-schemas
branch
from
cbe1336 to
ea52f52
Compare
5.3.2 is already released and some merges were listed on the wrong version.
| - Request schemas (including validation and deserialization) are now isolated by method | ||
| and endpoint type (#1047). | ||
| - Move generic API schemas (e.g TimeStamps and HeaderFields) from `kinto.core.resource.schema` | ||
| to a sepate file on `kinto.core.schema`. |
There was a problem hiding this comment.
I remember we had that before, and we changed... What does the docs say? Will the old location still be accesible? deprecated?
There was a problem hiding this comment.
There was a problem hiding this comment.
Hmm, that's interesting. I agree ResourceSchema should be on kinto.core.resource because it's only related to the resource (and I'm keeping it there), but I'm not so sure about other things like Timestamps or Header fields that could be reused in other parts of the core.
Most of these schemas are imported on kinto.core.resource so they are still accessible. Only Timestamp and URL aren't, and they aren't used anywhere on Kinto. Do you know why we have them there?
There was a problem hiding this comment.
Do you know why we have them there?
Originally (archeology) they were in core because we thought they were common types of fields. They're just used in third parties or plugins. We can deprecated them, no worries.
There was a problem hiding this comment.
We can deprecated them, no worries.
OK. I don't have a strong opinion about it. We can keep them too. In case we deprecate or move them, do you think we should add warning messages or we can just make it a breaking change?
There was a problem hiding this comment.
About the documentation: I couldn't fond mentions to these schemas anywhere. Also in some parts we use a colander String with url validator to implement an url instead of the URL schema.
| @@ -0,0 +1,110 @@ | |||
| import six | |||
There was a problem hiding this comment.
It might be nice to have a module-level docstring explaining what's supposed to go in this file vs. what goes in kinto.core.resource.schema.
There was a problem hiding this comment.
I don't see many module-level docstrings on kinto.core (__init__.py is the only exception), that's why I didn't use them, but I agree it would be nice to have this written in the code somewhere.
Keep backward compatibility from all the schemas moved from `kinto.core.resource.schema` to `kinto.core.schema` by keeping a deprecated reference to the missing schemas (URL and TimeStamp). Also set a docstring on `kinto.core.schema` saying which schemas should be included there and instructions to where other schemas should live.
| - Upgraded to Kinto-Admin 1.8.1 | ||
| - Configure the Kinto Admin auth methods from the server configuration (#1042) | ||
| - Request schemas (including validation and deserialization) are now isolated by method | ||
| and endpoint type (#1047). |
There was a problem hiding this comment.
Are these deletions expected?
There was a problem hiding this comment.
Yeah, these are unreleased. They probably got there by a problem in some auto merge.
Related to #1033
This allow importing generic schemas on other parts of kinto core without breaking the import order.