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

Fix autodiscovery jsonschema #1108

Merged
merged 5 commits into from
Jan 24, 2023
Merged

Conversation

olblak
Copy link
Member

@olblak olblak commented Jan 23, 2023

This pullrequest fix the jsonschema generated for the autodiscovery.

Test

To test this pull request, you can run the following commands:

cd pkg/core/jsonschema/
go test

Additional Information

Tradeoff

Potential improvement

As of today, Updatecli codebase, relies on the function New to initiate object. it uses a map of plugin to define the behavior and due to that, the invopop/jsonschema requires to implement the interface func (ConfigType) JSONSchema() *jschema.Schema to generate the jsonschema corresponding to the map of plugin. Hence the current pullrequest

Another approach would be to implement a json (un)marshaller for each config that works with plugins. If we get rid of the New function in favor of the unmarshaller approach, then we would also need to implement of yaml.
Based on my initial experimentation, it makes the codebase more difficult to write and maintain

* Rename function `GenerateJsonSchema` to `AppendOneOfToJsonSchema` to
  better describe its meaning
* Fix deprecation by replacing ioutil.Writefile to os.Writefile
* Add function `AppendMapToJsonSchema` to handle the situation where we
  need to append properties to an existing object

Signed-off-by: Olblak <me@olblak.com>
Signed-off-by: Olblak <me@olblak.com>
@olblak olblak added the bug Something isn't working label Jan 23, 2023
@olblak olblak added this to the 0.44.0 milestone Jan 23, 2023
@olblak olblak added the jsonschema Anything related to updatecli jsonschema label Jan 24, 2023
@olblak olblak enabled auto-merge (squash) January 24, 2023 17:48
@olblak olblak merged commit 04d7a0a into updatecli:main Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working jsonschema Anything related to updatecli jsonschema
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant