-
Notifications
You must be signed in to change notification settings - Fork 178
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
automatic generation of json schema for plugin config #537
Conversation
LGTM, modulo the failing CI, I think there should be some feature-gate for no_std. |
] | ||
}, | ||
"http_port": { | ||
"type": "string" |
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 generated schema here is incomplete: a number would also be accepted. Not sure it that's an issue for our purpose though
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.
This goes directly from definition of port field in zenoh-plugin-rest/config.rs. If we want to correct it, we should fix this type
pub struct Config {
#[serde(deserialize_with = "deserialize_http_port")]
pub http_port: String,
__path__: Option<String>,
__required__: Option<bool>,
}
"__required__": { | ||
"type": [ | ||
"boolean", | ||
"null" |
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 would have expected {"type": "boolean", "required": false}
. Is this equivalent, or is the _required_
key now mandatory, but nullable?
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 need to check json schema standard, but at least example file without __required__
field passes the check.
I've added additional verification of sample config file against generated schema. Seems that serde is much less strict than formal schema validation. Sometimes it seems completely wrong: schema requires volumes
and storages
to be vectors, but our config sample shows them as objects:
pub struct PluginConfig {
pub name: String,
pub required: bool,
pub backend_search_dirs: Option<Vec<String>>,
pub volumes: Vec<VolumeConfig>,
pub storages: Vec<StorageConfig>,
...
The schema is generated on build stage by
build.rs
based on rust config structure. When structure is changed, schema is regenerated and should be committed into git together with changes in Rust code.