open-contracting / standard

Documentation of the Open Contracting Data Standard (OCDS)
http://standard.open-contracting.org/
Other
139 stars 46 forks source link

Deprecate package metadata, improve packaging documentation, refactor record package schema #1640

Closed duncandewhurst closed 1 year ago

duncandewhurst commented 1 year ago
duncandewhurst commented 1 year ago

Regarding the test failures:

test_json.py::test_schema_valid[/home/runner/work/standard/standard/schema/record-package-schema.json-record-package-schema.json-data4]
  schema/record-package-schema.json has Error while resolving `[https://standard.open-contracting.org/schema/1__2__0/record-schema.json`:](https://standard.open-contracting.org/schema/1__2__0/record-schema.json%60:) HTTPError: 404 Client Error: Not Found for url: https://standard.open-contracting.org/schema/1__2__0/record-schema.json at properties/records/items

This will be resolved once 120 is released, I'm not sure what to do about it in the meantime. As a result, the record package schema browser does not display.

test_json.py::test_schema_valid[/home/runner/work/standard/standard/schema/record-schema.json-record-schema.json-data0]
  schema/record-schema.json includes "null" in "type" at /definitions/LinkedRelease/properties/url

test_json.py::test_schema_valid[/home/runner/work/standard/standard/schema/record-schema.json-record-schema.json-data0]
  schema/record-schema.json is missing "null" in "type" at /definitions/LinkedRelease/properties/tag

test_json.py::test_schema_valid[/home/runner/work/standard/standard/schema/record-schema.json-record-schema.json-data0]
  unused codelists: awardCriteria.csv, awardStatus.csv, classificationScheme.csv, contractStatus.csv, country.csv, currency.csv, documentType.csv, extendedProcurementCategory.csv, initiationType.csv, language.csv, linkRelationType.csv, mediaType.csv, method.csv, milestoneStatus.csv, milestoneType.csv, partyRole.csv, partyScale.csv, procurementCategory.csv, relatedProcess.csv, relatedProcessScheme.csv, submissionMethod.csv, tenderStatus.csv, unitClassificationScheme.csv

I think the tests need to be updated to add exceptions for the new record schema since these warnings were not previously reported when the Record definition was embedded in the record package schema:

jpmckinney commented 1 year ago

Hmm, url being required but nullable seems like an error/oversight. A linked release is not at all useful without a URL. Let's remove null.

Edit: I made some changes. The other errors shouldn't occur again (hard to test the 404 fix locally).

duncandewhurst commented 1 year ago

Hmm, url being required but nullable seems like an error/oversight. A linked release is not at all useful without a URL. Let's remove null.

Done in https://github.com/open-contracting/standard/pull/1640/commits/80d245b62755f1dc472c0aa1f6ce4c41d66c5c51

Edit: I made some changes. The other errors shouldn't occur again (hard to test the 404 fix locally).

The 404 error still occurs

Need to delete the two changelog entries for #1422.

Done in https://github.com/open-contracting/standard/pull/1640/commits/710ab21ca9c5bfc0a118e30b34450affee53124f

Also missing changelog entries for:

* Deprecations (the deprecated fields and any sub-fields are no longer required)

* records and releases are no longer required to be unique

Done in https://github.com/open-contracting/standard/pull/1640/commits/d4e689743b54252cd124f86ffa9767686c9e83fb

Governance-wise, we can't justify the present with the future, so I changed the deprecation notices.

Please make identical or corresponding changes to the release package schema and page.

Done in https://github.com/open-contracting/standard/pull/1640/commits/3d1b39d75edd3b6aeb8bcf25d69a47021144ea32 and https://github.com/open-contracting/standard/pull/1640/commits/5b3358e8c8cd2572b7d043239fcb74636cab9398

jpmckinney commented 1 year ago

The 404 error still occurs

Ah, test_json.py was trying to use GITHUB_BASE_REF to determine whether the PR was for an unreleased tag or not (e.g. if the base ref contained "1.2", like 1.2-dev), but that environment variable is only available if the workflow is triggered by the pull_request event. The workflow has an if condition in order to not run twice for both the push and pull_request events (it would always run for push, and only for pull_request if the PR was on a fork).

I've removed that if condition. That said, I expect the PR will still fail on the push event, but it should pass on the pull_request event. Edit: I now skip the push event on PRs.

duncandewhurst commented 1 year ago

The new commits look good!