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

Update jsonschema version to 3.0.1 & JSON Schema draft to 7 #996

Closed
wants to merge 2 commits into from
Closed

Update jsonschema version to 3.0.1 & JSON Schema draft to 7 #996

wants to merge 2 commits into from

Conversation

danie1k
Copy link

@danie1k danie1k commented Jul 10, 2019

Fixes #983.

Changes proposed in this pull request:

  • Update jsonschema requirement version to >=3.0.1,4.0.0
  • Update default JSON Schema draft from 4 to 7

Related Pull Request(s):

@danie1k danie1k closed this Jul 10, 2019
@danie1k danie1k reopened this Jul 10, 2019
@danie1k danie1k changed the title Update jsonschema version to 3.0.1 WIP: Update jsonschema version to 3.0.1 Jul 10, 2019
@danie1k danie1k changed the title WIP: Update jsonschema version to 3.0.1 Update jsonschema version to 3.0.1 & JSON Schema draft to 7 Jul 10, 2019
@@ -151,7 +151,7 @@ def schema_response_integer(valid):
if valid == "valid":
return 3
else:
return 3.0
return 3.14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the background of this change?

Copy link
Author

@danie1k danie1k Jul 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that jsonschema draft 7 treats 3.0 as integer; because from a mathematical point of view 3.0 == 3, I think.
I havent search where/when such change has been made in jsonschema.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit tests were failing without that update. And because it looks pretty resonable, I applied that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dtkav
Copy link
Collaborator

dtkav commented Jul 11, 2019

the openapi spec is explicitly based on draft 4.

@danie1k
Copy link
Author

danie1k commented Jul 12, 2019

the openapi spec is explicitly based on draft 4.

I didn't know that. What are the reasons for that?

@dtkav
Copy link
Collaborator

dtkav commented Jul 13, 2019

It looks like my response wasn't quite right (sorry I was on mobile).
OpenAPI 2.0 (fka swagger) is on draft-4
OpenAPI 3.0 is on wright draft 00 (draft 5)

I don't have the details on why, but i do think it's important to follow the spec.

Copy link
Collaborator

@dtkav dtkav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think swagger 2.0 needs to continue to use draft-4, but openapi 3.0 could be updated to draft-5.

@danie1k
Copy link
Author

danie1k commented Jul 14, 2019

I think swagger 2.0 needs to continue to use draft-4...

I assume that there might not be easy way of using two different jsonchema drafts simultaneously?
I have to dig deeper into Swagger/OpenApi specs.

but openapi 3.0 could be updated to draft-5.

According to official JSON Schema website, Draft 5 should not be used, so Draft 6 is natural choice then, as a closest next version... But because Draft 7 is is fully backwards-compatible with Draft 6, we should use it, IMO.

@danie1k
Copy link
Author

danie1k commented Jul 14, 2019

There might be one more problem... The p1c2u/openapi-spec-validator package, which is the dependency of Connexion. It looks not very well maintained 🤔 and it still relies on Draft 4.

@danie1k
Copy link
Author

danie1k commented Jul 24, 2019

Such update is far more complex than it looks, more details in #983.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support jsonschema3
3 participants