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

automatic generation of json schema for plugin config #537

Merged
merged 7 commits into from
Aug 21, 2023
Merged

Conversation

milyin
Copy link
Contributor

@milyin milyin commented Aug 20, 2023

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.

@milyin milyin requested review from Mallets, gabrik and p-avital August 20, 2023 19:11
@gabrik
Copy link
Contributor

gabrik commented Aug 21, 2023

LGTM, modulo the failing CI, I think there should be some feature-gate for no_std.

]
},
"http_port": {
"type": "string"
Copy link
Contributor

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

Copy link
Contributor Author

@milyin milyin Aug 21, 2023

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"
Copy link
Contributor

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?

Copy link
Contributor Author

@milyin milyin Aug 21, 2023

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>,
...

@milyin milyin merged commit 846825e into master Aug 21, 2023
@milyin milyin deleted the plugin_schema branch August 21, 2023 14:45
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.

3 participants