-
Notifications
You must be signed in to change notification settings - Fork 72
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
Introduced a JSON schema of a leapp report #681
Conversation
Can one of the admins verify this patch? |
Thank you for contributing to the Leapp project!Please note that every PR needs to comply with the If you want to re-run tests or request review, you can use following commands as a comment:
Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please consider rerunning the CI by commenting leapp-ci build (might require several comments). If the problem persists, contact leapp-infra. |
@fernflower We should refer to the schema from the upstream documentation. |
@pirat89 I will put the reference once the schema is merged. Are you ok with the placing in leapp's root? |
@fernflower I am thinking where to put it as we want to even reduce the main topic menu - it's already too long. We should discuss it in future with @mportman12 probably. But temporary it could be ok. Btw, why to do it separately? Why not in this PR? As these two things are two sides of the same coin. I guess it's ok if the jira task is kept in development or if you create the followup task, so we do not forget to finish it. |
@fernflower aah...I thought you are talking about readthedocs menu. Now I see where the file is located :) I think it should be in a subdir. As well, haven't we talked about version schema? As we plan to change it in future, so I believe there should be something like version 1.0 .. maybe the file should reflect that as well and should be called |
leapp-ci build |
@pirat89 what about putting 'version' in the schema itself (did it in the latest commit)? I'd rather keep the filename as it is |
@fernflower And how you will point people to various versions of schemas then? we will need keep several versions of schemas documented in the same time.. |
I really believe we need to have info about the version inside the schema and reflect that in the filename as well. |
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.
giving my approval to not block the pr. I will try to think where to move the file. in the worst case, we can still move it after the PR merge.
replaced tabs by whitespaces in the fixup commits |
"properties": { | ||
"url": { | ||
"$id": "#/properties/entries/items/anyOf/0/properties/detail/properties/external/items/anyOf/0/properties/url", | ||
"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.
What about to add the uri
format to make validation even more strict?"
"type": "string", | |
"type": "string", | |
"format": "uri", |
ref: https://json-schema.org/understanding-json-schema/reference/string.html#resource-identifiers
8655ef8
to
73e7f59
Compare
Fixing the indentation as it was broken on several places (maybe after may edit) |
[ | ||
"rm -rf /tmp/42" | ||
] | ||
], |
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 is wrong actually. E.g.
{
"type": "command",
"context": [
"leapp", "answer", "--section", "remove_pam_pkcs11_module_check.confirm=True"
]
}
We should add more examples to be able to validate the json schema better. Let's do it the next morning.
report-schema-v100.json
Outdated
"properties": { | ||
"anyOf": [ | ||
{ | ||
"type": { | ||
"$id": "#/properties/detail/properties/remediations/items/anyOf/0/properties/type", |
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.
and this is probably not possible as well. properties should be always key-value dicts as it looks like.
Generated by https://jsonschema.net and adjusted manually. The patch contains 2 schema files: - 1.0.0 that matches the version that foreman_leapp relies on - 1.1.0 that has the stable report key. Basic unit test coverage is presented in test_reporting.py - the sample leapp-report.json from vagrant box is checked against json schema v 1.0.0 and 1.1.0. OAMG-4263 Co-Authored-By: Petr Stodůlka <[email protected]>
0c56624
to
a3d426a
Compare
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.
🥳
## Packaging - Add JSON schema of leapp reports for validation (oamg#681) - Bump leapp-framework capability to 1.4 (oamg#684) ## Framework ### Fixes - Fix Py2/Py3 issues for unit-tests relying on __wrapped__ for decorators (oamg#674) ### Enhancements - Add a stable report identifier for each generated report (oamg#669) ## Additional changes interesting for devels - Add a possibility to overwrite virtualenv name for testing `$VENVNAME` (oamg#675) - Add a way to override default python version using `$PYTHON_VENV` (oamg#675) Related leapp-repository release: https://github.com/oamg/leapp-repository/releases/tag/v0.13.0
## Packaging - Add JSON schema of leapp reports for validation (oamg#681) - Bump leapp-framework capability to 1.4 (oamg#684) ## Framework ### Fixes - Fix Py2/Py3 issues for unit-tests relying on __wrapped__ for decorators (oamg#674) ### Enhancements - Add a stable report identifier for each generated report (oamg#669) ## Additional changes interesting for devels - Add a possibility to overwrite virtualenv name for testing `$VENVNAME` (oamg#675) - Add a way to override default python version using `$PYTHON_VENV` (oamg#675) Related leapp-repository release: https://github.com/oamg/leapp-repository/releases/tag/v0.13.0
## Packaging - Add JSON schema of leapp reports for validation (#681) - Bump leapp-framework capability to 1.4 (#684) ## Framework ### Fixes - Fix Py2/Py3 issues for unit-tests relying on __wrapped__ for decorators (#674) ### Enhancements - Add a stable report identifier for each generated report (#669) ## Additional changes interesting for devels - Add a possibility to overwrite virtualenv name for testing `$VENVNAME` (#675) - Add a way to override default python version using `$PYTHON_VENV` (#675) Related leapp-repository release: https://github.com/oamg/leapp-repository/releases/tag/v0.13.0
Generated by https://jsonschema.net and adjusted manually.
OAMG-4263