You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by stigsb <gi...@git.apache.org> on 2014/12/02 09:32:32 UTC
[GitHub] thrift pull request: Run JSON schema validator from test
GitHub user stigsb opened a pull request:
https://github.com/apache/thrift/pull/299
Run JSON schema validator from test
This patch makes "make check" test the JSON generator by running a schema validator on generated files.
Jira: THRIFT-2869
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/stigsb/thrift THRIFT-2869
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/thrift/pull/299.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #299
----
commit 692cc1fdd74ca6abb1165810f01f612e81c8e0ba
Author: Stig Bakken <st...@zedge.net>
Date: 2014-12-02T08:21:24Z
THRIFT-2869: run JSON schema validator from test
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: THRIFT-2869: Run JSON schema validator from t...
Posted by nsuke <gi...@git.apache.org>.
Github user nsuke commented on a diff in the pull request:
https://github.com/apache/thrift/pull/299#discussion_r46765156
--- Diff: lib/json/schema.json ---
@@ -20,66 +21,78 @@
"map",
"union",
"struct",
- "exception",
"binary"
]
},
"base-type": {
- "title": "Base types",
+ "title": "Base type schema",
"type": "object",
"properties": {
"typeId": {
- "enum": [ "void", "string", "bool", "byte", "i16", "i32", "i64", "double", "binary" ]
+ "enum": ["void", "string", "bool", "byte", "i16", "i32", "i64", "double", "binary" ]
}
},
"required": [ "typeId" ]
},
"list-type": {
- "title": "List and set types",
+ "title": "List and set schema",
"type": "object",
"properties": {
- "typeId": { "enum": [ "list", "set" ] },
- "elemTypeId": { "$ref": "#/definitions/type-id" },
- "elemType": { "$ref": "#/definitions/type-spec" }
+ "typeId": {
+ "enum": [ "list", "set" ]
+ }
},
- "required": [ "typeId", "elemTypeId", "elemType" ]
+ "required": [ "typeId", "elemTypeId" ]
},
"map-type": {
- "title": "Map type",
+ "title": "Map schema",
"type": "object",
"properties": {
- "typeId": { "enum": [ "map" ] },
- "keyTypeId": { "$ref": "#/definitions/type-id" },
- "keyType": { "$ref": "#/definitions/type-spec" },
- "valueTypeId": { "$ref": "#/definitions/type-id" },
- "valueType": { "$ref": "#/definitions/type-spec" }
+ "typeId": {
+ "enum": [ "map" ]
+ }
},
"required": [ "typeId", "keyTypeId", "valueTypeId" ]
},
- "struct-spec": {
- "title": "Struct and union types",
+ "struct-type": {
+ "title": "Struct, union and exception schema",
"type": "object",
"properties": {
- "typeId": { "enum": [ "union", "struct" ] },
- "class": { "type": "string" }
+ "typeId": {
+ "enum": [ "union", "struct", "exception" ]
+ }
},
"required": [ "typeId", "class" ]
},
- "type-spec": {
+ "type-desc": {
+ "title": "Type descriptor schema",
"allOf": [
- { "type": "object" },
+ {
+ "type": "object",
+ "properties": {
+ "typeId": { "type": "string" },
+ "keyTypeId": { "$ref": "#/definitions/type-id" },
+ "keyType": { "$ref": "#/definitions/type-desc" },
+ "valueTypeId": { "$ref": "#/definitions/type-id" },
+ "valueType": { "$ref": "#/definitions/type-desc" },
+ "class": { "type": "string" },
+ "elemTypeId": { "$ref": "#/definitions/type-id" },
+ "elemType": { "$ref": "#/definitions/type-desc" }
--- End diff --
@stigsb Not sure you're still around but would you mind elaborating on why you moved key/value/class/elem here ?
Thanks for sharing this BTW. It's pity this one escaped our eyes for so long.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: THRIFT-2869: Run JSON schema validator from t...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/thrift/pull/299
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: THRIFT-2869: Run JSON schema validator from t...
Posted by nsuke <gi...@git.apache.org>.
Github user nsuke commented on a diff in the pull request:
https://github.com/apache/thrift/pull/299#discussion_r48102430
--- Diff: lib/json/schema.json ---
@@ -20,66 +21,78 @@
"map",
"union",
"struct",
- "exception",
"binary"
]
},
"base-type": {
- "title": "Base types",
+ "title": "Base type schema",
"type": "object",
"properties": {
"typeId": {
- "enum": [ "void", "string", "bool", "byte", "i16", "i32", "i64", "double", "binary" ]
+ "enum": ["void", "string", "bool", "byte", "i16", "i32", "i64", "double", "binary" ]
}
},
"required": [ "typeId" ]
},
"list-type": {
- "title": "List and set types",
+ "title": "List and set schema",
"type": "object",
"properties": {
- "typeId": { "enum": [ "list", "set" ] },
- "elemTypeId": { "$ref": "#/definitions/type-id" },
- "elemType": { "$ref": "#/definitions/type-spec" }
+ "typeId": {
+ "enum": [ "list", "set" ]
+ }
},
- "required": [ "typeId", "elemTypeId", "elemType" ]
+ "required": [ "typeId", "elemTypeId" ]
},
"map-type": {
- "title": "Map type",
+ "title": "Map schema",
"type": "object",
"properties": {
- "typeId": { "enum": [ "map" ] },
- "keyTypeId": { "$ref": "#/definitions/type-id" },
- "keyType": { "$ref": "#/definitions/type-spec" },
- "valueTypeId": { "$ref": "#/definitions/type-id" },
- "valueType": { "$ref": "#/definitions/type-spec" }
+ "typeId": {
+ "enum": [ "map" ]
+ }
},
"required": [ "typeId", "keyTypeId", "valueTypeId" ]
},
- "struct-spec": {
- "title": "Struct and union types",
+ "struct-type": {
+ "title": "Struct, union and exception schema",
"type": "object",
"properties": {
- "typeId": { "enum": [ "union", "struct" ] },
- "class": { "type": "string" }
+ "typeId": {
+ "enum": [ "union", "struct", "exception" ]
+ }
},
"required": [ "typeId", "class" ]
},
- "type-spec": {
+ "type-desc": {
+ "title": "Type descriptor schema",
"allOf": [
- { "type": "object" },
+ {
+ "type": "object",
+ "properties": {
+ "typeId": { "type": "string" },
+ "keyTypeId": { "$ref": "#/definitions/type-id" },
+ "keyType": { "$ref": "#/definitions/type-desc" },
+ "valueTypeId": { "$ref": "#/definitions/type-id" },
+ "valueType": { "$ref": "#/definitions/type-desc" },
+ "class": { "type": "string" },
+ "elemTypeId": { "$ref": "#/definitions/type-id" },
+ "elemType": { "$ref": "#/definitions/type-desc" }
--- End diff --
Thanks for the explanation.
I manually modified generated code but it seems it works fine.
I understand it may have been another issue but based on information we have, I'm excluding this part for now.
You can find update PR here: #749
It already found one regression (outdated schema) introduced after this PR :smile:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: THRIFT-2869: Run JSON schema validator from t...
Posted by stigsb <gi...@git.apache.org>.
Github user stigsb commented on a diff in the pull request:
https://github.com/apache/thrift/pull/299#discussion_r47478624
--- Diff: lib/json/schema.json ---
@@ -20,66 +21,78 @@
"map",
"union",
"struct",
- "exception",
"binary"
]
},
"base-type": {
- "title": "Base types",
+ "title": "Base type schema",
"type": "object",
"properties": {
"typeId": {
- "enum": [ "void", "string", "bool", "byte", "i16", "i32", "i64", "double", "binary" ]
+ "enum": ["void", "string", "bool", "byte", "i16", "i32", "i64", "double", "binary" ]
}
},
"required": [ "typeId" ]
},
"list-type": {
- "title": "List and set types",
+ "title": "List and set schema",
"type": "object",
"properties": {
- "typeId": { "enum": [ "list", "set" ] },
- "elemTypeId": { "$ref": "#/definitions/type-id" },
- "elemType": { "$ref": "#/definitions/type-spec" }
+ "typeId": {
+ "enum": [ "list", "set" ]
+ }
},
- "required": [ "typeId", "elemTypeId", "elemType" ]
+ "required": [ "typeId", "elemTypeId" ]
},
"map-type": {
- "title": "Map type",
+ "title": "Map schema",
"type": "object",
"properties": {
- "typeId": { "enum": [ "map" ] },
- "keyTypeId": { "$ref": "#/definitions/type-id" },
- "keyType": { "$ref": "#/definitions/type-spec" },
- "valueTypeId": { "$ref": "#/definitions/type-id" },
- "valueType": { "$ref": "#/definitions/type-spec" }
+ "typeId": {
+ "enum": [ "map" ]
+ }
},
"required": [ "typeId", "keyTypeId", "valueTypeId" ]
},
- "struct-spec": {
- "title": "Struct and union types",
+ "struct-type": {
+ "title": "Struct, union and exception schema",
"type": "object",
"properties": {
- "typeId": { "enum": [ "union", "struct" ] },
- "class": { "type": "string" }
+ "typeId": {
+ "enum": [ "union", "struct", "exception" ]
+ }
},
"required": [ "typeId", "class" ]
},
- "type-spec": {
+ "type-desc": {
+ "title": "Type descriptor schema",
"allOf": [
- { "type": "object" },
+ {
+ "type": "object",
+ "properties": {
+ "typeId": { "type": "string" },
+ "keyTypeId": { "$ref": "#/definitions/type-id" },
+ "keyType": { "$ref": "#/definitions/type-desc" },
+ "valueTypeId": { "$ref": "#/definitions/type-id" },
+ "valueType": { "$ref": "#/definitions/type-desc" },
+ "class": { "type": "string" },
+ "elemTypeId": { "$ref": "#/definitions/type-id" },
+ "elemType": { "$ref": "#/definitions/type-desc" }
--- End diff --
It's been a while, so my memory may be off here, but I seem to remember that it was because the validator wouldn't catch failures otherwise.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---