-
Notifications
You must be signed in to change notification settings - Fork 9
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
Implemented command validate
#24
Conversation
Ticket: None Changelog: Title Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>
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.
So did you check https://json-schema.org/?
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.
Cool, thanks!
index = Index(index)._get() | ||
|
||
try: | ||
validate_index(index) | ||
except CFBSIndexException as e: | ||
print(e) | ||
return 1 | ||
return 0 |
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.
index = Index(index)._get() | |
try: | |
validate_index(index) | |
except CFBSIndexException as e: | |
print(e) | |
return 1 | |
return 0 | |
index = Index(index)._get() | |
try: | |
validate_index(index) | |
except CFBSIndexException as e: | |
print(e) | |
return 1 | |
return 0 |
Not a big issue, but I suggest not using the Index
class for this. It is conceivable that Index()
might fail on invalid index, and then you don't get the error handling you want. I think validate_index
should just take a normal dict. It could also be useful to run this function from within Index()
.
Maybe make a helper function get_json()
which reads from file or HTTP request, depending on the path and use that instead?
Index
also has caching, which maybe you don't want here.
We can merge this anyway, feel free to address in a follow-up.
@vpodzime I told him to check it out, but since this was already done, without dependencies, and relatively small (~100) lines, we can use it like this as well. |
Ticket: None
Changelog: Title
Signed-off-by: Lars Erik Wik lars.erik.wik@northern.tech