JSON API - stage I#934
Conversation
|
Can one of the admins verify this patch? |
|
ok to test |
|
add to whitelist |
…rshaling json responses, RESTful API early prototype Signed-off-by: Piotr Debski <piotr.debski@intel.com>
amd-pdebski
changed the title
| /** Reads Request structure from file "request.json" */ | ||
|
|
||
| func (req *Request) Read_req() { | ||
| data, err := ioutil.ReadFile("request.json") |
There was a problem hiding this comment.
We'd prefer to read input json from stdin instead of forcing user to create file containing the request.
|
|
||
| func (req *Request) Write_req() { | ||
| file, err := json.MarshalIndent(req, "", " ") | ||
| _ = ioutil.WriteFile("request.json", file, 0644) |
There was a problem hiding this comment.
Same here. We'd prefer to print output to stdout. It's much easier to integrate with other tools.
| type Request struct { | ||
| Request_list_caches bool `json:"request list_caches"` | ||
| Request_get_stats bool `json:"request statistics"` | ||
| Get_stats_info Stats_request_info `json:"statistics request info"` | ||
| } |
There was a problem hiding this comment.
I think it would be better to have common request header (it may consist of a single field like "command", which would contain name of the command) and the rest of the request structure would be different for each command. Otherwise we will end up with request structure containing all the input for all variants of all commands.
That would require preparsing request json as a Header struct which may look like this:
type Header struct {
Command string `json:"command"`
}That would allow us to determine the command type, and then we would be able to parse the request as a specific command.
|
Please squash commits to hide review history (e.g. remove "Review fixes" commit" from the log) |
|
General comment: I would suggest to make json application output to entirely conform to json syntax. Especially error information might be included in the json output itself. |
| "core_id": 0, | ||
| "io_class": 0, |
There was a problem hiding this comment.
It would make sense to make "core_id" and "io_class" parameters optional
There was a problem hiding this comment.
Problem addressed trough increase granulation and making current parameters obligatory (different commands for each statistics level)
|
retest this please |
| @@ -0,0 +1,165 @@ | |||
| /* | |||
| * Copyright(c) 2012-2021 Intel Corporation | |||
| * SPDX-License-Identifier: BSD-3-Clause-Clear | |||
There was a problem hiding this comment.
Please update license tag to SPDX-License-Identifier: BSD-3-Clause
| json_get_stats_core(*params) | ||
| } | ||
|
|
||
| if command == "opencas.ioclass.stats.get" { |
There was a problem hiding this comment.
It should be opencas.cache.ioclass.stats.get.
| json_get_stats_cache(*params) | ||
| } | ||
|
|
||
| if command == "opencas.core.stats.get" { |
There was a problem hiding this comment.
It should be opencas.cache.core.stats.get.
| json_get_cache_info(*params) | ||
| } | ||
|
|
||
| if command == "opencas.core.info.get" { |
There was a problem hiding this comment.
opencas.cache.core.info.get
| json_get_core_info(*params) | ||
| } | ||
|
|
||
| if command == "opencas.ioclass.info.get" { |
There was a problem hiding this comment.
opencas.cache.ioclass.info.get
|
Hello @pdebski21! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2022-01-04 09:28:29 UTC |
Signed-off-by: Piotr Debski <piotr.debski@intel.com>
Signed-off-by: Piotr Debski <piotr.debski@intel.com>
9 participants
JSON API - ioctl handling, json requests handling, structuring and marshaling json responses, RESTful API early prototype