Skip to content
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

Consider a "strict" schema to introduce JSON Schema validation keywords prior to 2.0 #1046

Open
jpmckinney opened this issue Jul 22, 2020 · 8 comments · May be fixed by #1710
Open

Consider a "strict" schema to introduce JSON Schema validation keywords prior to 2.0 #1046

jpmckinney opened this issue Jul 22, 2020 · 8 comments · May be fixed by #1710
Assignees
Labels
Schema: Validation Relating to constraints in the JSON Schema
Milestone

Comments

@jpmckinney
Copy link
Member

Several issues deal with incorrect or missing JSON Schema validation keywords. See milestone: https://github.com/open-contracting/standard/milestone/25

Typically, we would need to wait for a major version (2.0) to make changes that invalidate data from earlier versions (i.e. backwards-incompatible changes).

However, we can offer a voluntary, "strict" schema that adds in these keywords, removes deprecated fields and codes, removes deprecated types, etc.

Users of the Data Review Tool could voluntarily select the strict schema. Similarly, OCDS Helpdesk analysts could inform publishers of opportunities to improve their data by passing the stricter criteria.

@jpmckinney jpmckinney added the Schema: Validation Relating to constraints in the JSON Schema label Jul 22, 2020
@jpmckinney jpmckinney added this to the 1.2.0 milestone Jul 22, 2020
@duncandewhurst duncandewhurst self-assigned this Nov 30, 2021
@duncandewhurst
Copy link
Contributor

However, we can offer a voluntary, "strict" schema that adds in these keywords, removes deprecated fields and codes, removes deprecated types, etc.

It might be preferable to keep deprecated fields, codes, and types. If we remove them, then the DRT will report them as additional in which case it's less clear what to do about them than if they are flagged as deprecated in which case the deprecation notes usually explain what to do instead.

@duncandewhurst duncandewhurst mentioned this issue Feb 10, 2022
14 tasks
@jpmckinney
Copy link
Member Author

jpmckinney commented Feb 10, 2022

Yes, we can keep the deprecated fields and codes.

The DRT (and schema) has nothing about deprecated types (e.g. "number" for ID fields), so it is better to remove them and cause a structural error.

@duncandewhurst
Copy link
Contributor

Sounds good!

@duncandewhurst
Copy link
Contributor

Are there any other types that we should remove, apart from "number" for ID fields and "null" for required fields?

@jpmckinney
Copy link
Member Author

Not that I'm aware. I think most fields are single-type, but we can run some code to find any other multi-type.

@duncandewhurst
Copy link
Contributor

I checked using a Python script and the only other multi-type fields are:

  • Budget.projectID - I've updated the script in the PR to remove the integer type from this field
  • Amendment.changes.former_value - this is part of a deprecated object and is in any case intentional so no need to change it

@duncandewhurst
Copy link
Contributor

Noting that #1480 doesn't close this issue because we still need to prepare a second PR to add the documentation discussed in #1480. Once the docs are ready, we can run the manage.py command to generate the strict files, and then merge.

@jpmckinney
Copy link
Member Author

prepare a second PR to add the documentation discussed in #1480.

Copying relevant content here for easier reference:

From #1480 (comment)

Do you have a view on if and how we should feature the strict schema(s) in the documentation?

I figure few users would do anything with the actual JSON schema, but adding schema viewers for the strict versions of the release, release package and record package schemas seems overly duplicative. I think we should include some mention of it in the docs if we are going to include it in the DRT and helpdesk feedback processes. We could either add an admonition to https://standard.open-contracting.org/staging/1.2-dev/en/schema/# or a new page under the reference section which can give a summary of the differences to the regular schema and direct users to the DRT to check their data against the strict schema.

#1480 (comment)

Do you have a view on if and how we should feature the strict schema(s) in the documentation?

I think a concise admonition as you suggest would be best. It should briefly describe why the strict schema is preferred for new implementations (and how it can be useful to improve data quality of existing implementations).

In terms of duplication, we can consider using https://sphinx-design.readthedocs.io/en/latest/ to allow browsing of each schema. That way it's not taking up a lot of the page.

#1480 (comment)

In terms of duplication, we can consider using https://sphinx-design.readthedocs.io/en/latest/ to allow browsing of each schema. That way it's not taking up a lot of the page.

I tried this approach but only the first docson widget is displayed. The second widget throws an Uncaught Error: only one instance of babel-polyfill is allowed (see lbovet/docson#73).

In other projects that have multiple docson widgets per page we use https://github.com/OpenDataServices/docson which is a fork of an earlier version of docson that doesn't use babel-polyfill, but I see that https://github.com/open-contracting/docson makes several improvements that would be lost if we used the other fork.

I found https://www.npmjs.com/package/idempotent-babel-polyfill/ which might be a solution but my Javascript knowledge falls short of how to update docson to use it.

@duncandewhurst duncandewhurst linked a pull request Oct 24, 2024 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Schema: Validation Relating to constraints in the JSON Schema
Projects
Status: Review in progress
Development

Successfully merging a pull request may close this issue.

2 participants