Add semantic version to configuration JSON schema and coalesce schema to single file#765
Add semantic version to configuration JSON schema and coalesce schema to single file#765Sbozzolo wants to merge 7 commits into
Conversation
|
cc: @ad-cqc |
Sbozzolo
added
the
no-long-tests
| @@ -1,5 +1,6 @@ | |||
| { | |||
| "$schema": "http://json-schema.org/draft-07/schema#", | |||
| "version": "0.1.0", | |||
There was a problem hiding this comment.
It should be something like x-schema-version. See https://json-schema.org/blog/posts/custom-annotations-will-continue
hughcars
left a comment
hughcars
left a comment
There was a problem hiding this comment.
I think this needs some more clarity on how versioning interacts with palace versioning, but on board with the idea because of how useful it will be for downstream tooling.
| | Schema version | First *Palace* release | | ||
| |:--------------:|:----------------------:| | ||
| | `0.1.0` | `0.17` | |
There was a problem hiding this comment.
I suspect no, but is there a mechanism we can have for enforcing this pairing outside just updating here? I.e. scrape from the CHANGELOG or the like. Will we be updating the schema version inside of Palace versions? Or does a schema update have to correspond to a palace update. I would posit a schema update will bump the palace update, but not vice versa. What is the map from schema major/minor/patch to palace major/minor/patch? I think we need some clarity on how these move, you can possibly illustrate with some of the changes from the historical changelog.
There was a problem hiding this comment.
We could add a dedicated record about schema version update to CHANGELOG.md, or have a separate log file for the schema.
There was a problem hiding this comment.
I have all the same questions as @hughcars. We have to think more carefully about what the Palace semver will mean now, as historically any changes to it have always been dictated by changes to the config schema so the Palace version effectively signaled the state of the schema and nothing else (please correct me if I'm wrong).
I'll note that there are other surfaces besides the config, such as metadata output (palace.json) and the whole slew of .csv outputs, any changes to which can also break downstream consumers and therefore we'd ideally start signaling changes to those as well. So maybe Palace version becomes a signal for changes to the inputs and outputs while the schemaver is strictly for the config schema.
There was a problem hiding this comment.
How about to add a schema version into palace --version output (doesn't exist now)? So the version map between executable and config will be explicitly maintained in the code. If you bump the schema version, it results in new palace version. For now, it'd be joint but separate changes (modify schema JSONs and source code), but eventually when the schema is going to be based on code, we won't need to modify schema files anymore.
There was a problem hiding this comment.
We have to think more carefully about what the Palace semver will mean now, as historically any changes to it have always been dictated by changes to the config schema so the Palace version effectively signaled the state of the schema and nothing else (please correct me if I'm wrong).
This is not correct. We bumped Palace version if anything in the input/output changed, as you suggest in the second part of your comment.
A change in the Palace version does not require a change in the schema, but a change in the schema required a change in the Palace version.
How about to add a schema version into palace --version output (doesn't exist now)?
Good idea!
There was a problem hiding this comment.
I also added a section to explain the relationship between the two versions
| @@ -1,5 +1,6 @@ | |||
| { | |||
| "$schema": "http://json-schema.org/draft-07/schema#", | |||
| "x-schema-version": "0.1.0", | |||
There was a problem hiding this comment.
BTW, we could use a urn with a schema version embedded into it as a proper schema identifier, "$id": "urn:palace:schema:0.1.0" which I think is better than a custom annotation field.
There was a problem hiding this comment.
I didn't account for $id to be used as a root location for a subschema path resolution. The JSON schema standard defines $id (see 8.2) as the base URI that other URI references within the schema are resolved against. This messes up the subschema file path construction with error: Cannot add a path (./config/boundaries.json) to an URN URI (urn:palace:schema:1-0-0).
So, we only would be able to use $id with a versioned URN if the schema is a single file which would not require subschema file lookup. Or, go back to x-schema-version custom attribute. In #723, I emit single file with "$id": "urn:palace:schema:0.1.0", but I could switch to x-schema-version.
The configuration schema was implicitly versioned alongside Palace, with no explicit marker of the configuration contract or its compatibility guarantees. In this PR, I add a top-level "version" field (semantic versioning, starting at 0.1.0) to the root schema config-schema.json. Closes #760
Address review feedback on the schema-versioning change: - Rename the root-schema field "version" to "x-schema-version". The "x-" prefix marks it as a custom annotation keyword, making explicit that it is Palace metadata rather than a JSON Schema construct (draft-07 validators ignore "x-" keywords regardless). - Document how the schema version relates to the Palace release version: the two move independently, with one-directional coupling (a schema change always rides a Palace release, but a Palace release rarely changes the schema), plus MAJOR/MINOR/PATCH rules illustrated with historical changes and the version map table. - Add a CI guard (scripts/check-schema-version, wired into the check-schema-version job in style.yml) that fails a pull request which edits a schema JSON file without bumping "x-schema-version". This enforces the pairing automatically instead of relying on reviewers to remember it.
Replace the custom "x-schema-version" annotation with the standard JSON Schema "$id" field using a URN: "urn:palace:schema:1-0-0". This uses SchemaVer (MODEL-REVISION-ADDITION) instead of SemVer, and makes the schema identifiable by any JSON Schema tooling without custom extensions. Also expose the schema version in `palace --version` output via a new GetSchemaVersion() helper.
Sbozzolo
force-pushed
the
gbozzola/schema-version
branch
from
3680897 to
8c4a31e
Compare
|
I:
|
Merge the 5 sub-schema files (config/boundaries.json, config/domains.json, config/model.json, config/problem.json, config/solver.json) into the root config-schema.json as $defs. This eliminates external $ref resolution, which conflicted with using "$id": "urn:palace:schema:1-0-0" as the standard schema identifier (JSON Schema resolves sub-schema paths against $id, and URNs cannot have paths appended). Also clean up: - Remove dead external-ref resolution code in jsonschema.cpp - Update SchemaCoverageGaps in test-config.cpp to use root-relative pointers - Simplify GLOB_RECURSE to GLOB in cmake (no subdirs anymore) - Update stale comments referencing multi-file structure
|
I collapsed the various schema files into a single json |
Sbozzolo
changed the title
4 participants
The configuration schema was implicitly versioned alongside Palace, with no explicit marker of the configuration contract or its compatibility guarantees.
In this PR, I add a top-level "version" field (semantic versioning, starting at 0.1.0) to the root schema config-schema.json.
Closes #760