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

Use jsonschema to validate messages #37

Merged
merged 2 commits into from
Dec 6, 2017

Conversation

takluyver
Copy link
Member

Instead of custom validation built on traitlets. JSONSchema is a format designed to express precisely this kind of thing.

I haven't thrown all messages into one giant schema, because I think that will make unhelpful errors. In nbformat we deal with that by catching the errors and doing some extra work to produce more useful errors, but in this case I think it's more straightforward to break the schema up and use the appropriate parts for the message type.

@@ -0,0 +1,336 @@
"""Message schemas for message spec version 5"""
Copy link
Member

Choose a reason for hiding this comment

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

This is excellent! I think this and the .json file it emits should be in nbformat.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd say jupyter_client would probably be the place for it - that's where the messaging protocol is defined. But I thought I'd put it here first, where there's a clear requirement for it.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, right, agreed!


schema_fragments["input_reply"] = {"properties": {
"value": {"type": "string"},
}}
Copy link
Member

Choose a reason for hiding this comment

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

This is all awesome, I'm so glad you've put all these schemas together.

@takluyver
Copy link
Member Author

I'm going to merge this now. It's useful to have the schemas, and we can move them into jupyter_client at any point.

@takluyver takluyver merged commit 0564c97 into jupyter:master Dec 6, 2017
@takluyver takluyver deleted the jsonschema branch December 6, 2017 14:51
@blink1073 blink1073 added this to the 0.4 milestone Nov 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants