-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
v3.0.4 schema updates #4132
v3.0.4 schema updates #4132
Changes from all commits
383a6ee
e81f016
a05aec4
00d66f0
559500b
bc55266
b285b78
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
id: https://spec.openapis.org/oas/3.0/schema/2021-09-28 | ||
id: https://spec.openapis.org/oas/3.0/schema/2024-10-10 | ||
$schema: http://json-schema.org/draft-04/schema# | ||
description: The description of OpenAPI v3.0.x documents, as defined by https://spec.openapis.org/oas/v3.0.3 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @karenetheridge I realize you are not the person who put the "as defined by ...v3.0.N" here, but I'd really like to drop it. This is for 3.0. All of 3.0. |
||
description: The description of OpenAPI v3.0.x documents, as defined by https://spec.openapis.org/oas/v3.0.4 | ||
type: object | ||
required: | ||
- openapi | ||
|
@@ -43,6 +43,7 @@ definitions: | |
'^\$ref$': | ||
type: string | ||
format: uri-reference | ||
|
||
Info: | ||
type: object | ||
required: | ||
|
@@ -66,7 +67,6 @@ definitions: | |
'^x-': {} | ||
additionalProperties: false | ||
|
||
|
||
Contact: | ||
type: object | ||
properties: | ||
|
@@ -593,7 +593,6 @@ definitions: | |
minProperties: 1 | ||
additionalProperties: false | ||
|
||
|
||
SecurityRequirement: | ||
type: object | ||
additionalProperties: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need this? If we are to accept or build something similar, the JSON variant becomes a downstream artifact that is always regenerated from the YAML variant. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I edited the json files by hand in order to preserve the same property order as was in the yaml files. Autogenerating the json from the yaml would lose that, and cause a much larger diff. Since the file was almost identical already (and just lacking a few of the recent changes to the yaml file) it was easier to add those by hand and the diff was smaller. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are definitely order-preserving translators. I believe current versions of JavaScript maintain object property order. If the JSON is in a different order from the YAML, we should probably fix that. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
#!/usr/bin/env perl | ||
# vim: set ts=8 sts=2 sw=2 tw=100 et : | ||
|
||
# given a filename base, compare the .yaml and .json versions of the file for differences. | ||
# to run: | ||
# apt-get libjson-schema-modern-perl # (or equivalent on your distribution) | ||
# scripts/compare schemas/v3.0/schema | ||
# scripts/compare schemas/v3.1/schema | ||
# scripts/compare schemas/v3.1/schema-base | ||
|
||
# Author: Karen Etheridge - [email protected], [email protected] | ||
# see also https://github.com/karenetheridge/JSON-Schema-Modern | ||
|
||
use 5.020; | ||
use warnings; | ||
use Path::Tiny; | ||
use YAML::PP; | ||
use JSON::Schema::Modern (); | ||
use JSON::Schema::Modern::Utilities 0.591 'is_equal'; | ||
|
||
my $base = shift; # the filename, without .yaml or .json extension | ||
|
||
my $yaml_decoder = YAML::PP->new(boolean => 'JSON::PP'); | ||
my $json_decoder = JSON::Schema::Modern::_JSON_BACKEND()->new->utf8(1); | ||
|
||
my $yaml_path = path($base.'.yaml'); | ||
my $json_path = path($base.'.json'); | ||
|
||
my $yaml_data = $yaml_decoder->load_string($yaml_path->slurp_raw); | ||
my $json_data = $json_decoder->decode($json_path->slurp_raw); | ||
|
||
if (not is_equal($yaml_data, $json_data, my $state = {})) { | ||
say "$yaml_path and $json_path are not identical."; | ||
say "first difference is at $state->{path}: $state->{error}"; | ||
exit 1; | ||
} | ||
else { | ||
say "files are identical."; | ||
exit 0; | ||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need this? We already have https://github.com/OAI/OpenAPI-Specification/blob/main/scripts/yaml2json/yaml2json.js There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't "need" it. I wrote this script for my own use as I am not a javascript person. I simply thought it would be helpful to commit it. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
#!/bin/env perl | ||
use strict; | ||
use warnings; | ||
use YAML::XS; | ||
use JSON::PP; | ||
|
||
print JSON::PP->new->pretty->indent_length(2)->canonical->encode(Load(do { local $/; <> })); | ||
print "\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think part of the problem with our publishing process is has to do with this date. I'm fine with manually setting it when we push it out, but I think there was some expectation to set it automatically. Perhaps documented in #3715
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file in git should still have an
id
/$id
of some kind though.. perhaps something with "in_progress" or "latest" or something else non-dateish to indicate that it's not reflecting an officially-published version. Otherwise, it's yet one more barrier to tool vendors (on top of converting yaml to json) to make use of the interim file for development and testing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@karenetheridge I'd lean towards something like "in_progress" that discourages casual use.
There really shouldn't be interim files if our publication process is running smoothly. It might be different for pre-publication releases, but we should handle that separately as that's not a process we've defined at all yet.
In theory, every time we change a published schema, we just re-publish it immediately. If we ever have a reason not to do that, then we probably also don't want people to use the schemas out of
main
. We really only want the published ones to be usable, I think (again, excluding work-in-progress versions where we just haven't defined a process).I do agree that if we do want people to use things directly from
main
, then they need$id
/id
values that are somehow reasonable, and not identical to any published version.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The widely used @apidevtools/openapi-schemas package (1.7 million weekly downloads) seems to pull the OpenAPI schemas directly from
main
:And there may be other uses out there we don't know of.
Better have up-to-date YAML and JSON files with the most current identifier in the
main
branch.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ralfhandl better to not put schemas where some random 3rd-party tool can think they are the correct ones when they are not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, then let's replace the date with "in-progress" and see what gets published to the NPM package registry 😎
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ralfhandl I wouldn't object to trying to coordinate with them a bit. This is part of my (myriad) frustration with the schemas- we don't exactly make it easy for people who want to do something like package and distribute to do the right thing. It was only a couple of months ago that I even realized that "published" meant "put on the GitHub pages site" since we didn't even link to them on the index page of that site. It's far easier to find the "wrong" files than the "right" ones.