| Message ID | 20230927112544.85011-9-victortoso@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qapi-go: add generator for Golang interface | expand |
On Wed, Sep 27, 2023 at 01:25:43PM +0200, Victor Toso wrote: > This patch adds a struct type in Go that will handle return values > for QAPI's command types. > > The return value of a Command is, encouraged to be, QAPI's complex > types or an Array of those. > > Every Command has a underlying CommandResult. The EmptyCommandReturn > is for those that don't expect any data e.g: `{ "return": {} }`. > > All CommandReturn types implement the CommandResult interface. > > Example: > qapi: > | { 'command': 'query-sev', 'returns': 'SevInfo', > | 'if': 'TARGET_I386' } > > go: > | type QuerySevCommandReturn struct { > | CommandId string `json:"id,omitempty"` > | Result *SevInfo `json:"return"` > | Error *QapiError `json:"error,omitempty"` > | } > > usage: > | // One can use QuerySevCommandReturn directly or > | // command's interface GetReturnType() instead. > | > | input := `{ "return": { "enabled": true, "api-major" : 0,` + > | `"api-minor" : 0, "build-id" : 0,` + > | `"policy" : 0, "state" : "running",` + > | `"handle" : 1 } } ` > | > | ret := QuerySevCommandReturn{} > | err := json.Unmarshal([]byte(input), &ret) > | if ret.Error != nil { > | // Handle command failure {"error": { ...}} > | } else if ret.Result != nil { > | // ret.Result.Enable == true > | } > > Signed-off-by: Victor Toso <victortoso@redhat.com> > --- > scripts/qapi/golang.py | 72 ++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 70 insertions(+), 2 deletions(-) > > diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py > index 52a9124641..48ca0deab0 100644 > --- a/scripts/qapi/golang.py > +++ b/scripts/qapi/golang.py > @@ -40,6 +40,15 @@ > ''' > > TEMPLATE_HELPER = ''' > +type QapiError struct { QAPIError as the name for this > + Class string `json:"class"` > + Description string `json:"desc"` > +} > + > +func (err *QapiError) Error() string { > + return fmt.Sprintf("%s: %s", err.Class, err.Description) > +} My gut feeling is that this should be just return err.Description on the basis that long ago we pretty much decided that the 'Class' field was broadly a waste of time except for a couple of niche use cases. The error description is always self contained and sufficient to diagnose problems, without knowing the Class. Keep the Class field in the struct though, as it could be useful to check in certain cases With regards, Daniel
Hi, On Thu, Sep 28, 2023 at 04:03:12PM +0100, Daniel P. Berrangé wrote: > On Wed, Sep 27, 2023 at 01:25:43PM +0200, Victor Toso wrote: > > This patch adds a struct type in Go that will handle return values > > for QAPI's command types. > > > > The return value of a Command is, encouraged to be, QAPI's complex > > types or an Array of those. > > > > Every Command has a underlying CommandResult. The EmptyCommandReturn > > is for those that don't expect any data e.g: `{ "return": {} }`. > > > > All CommandReturn types implement the CommandResult interface. > > > > Example: > > qapi: > > | { 'command': 'query-sev', 'returns': 'SevInfo', > > | 'if': 'TARGET_I386' } > > > > go: > > | type QuerySevCommandReturn struct { > > | CommandId string `json:"id,omitempty"` > > | Result *SevInfo `json:"return"` > > | Error *QapiError `json:"error,omitempty"` > > | } > > > > usage: > > | // One can use QuerySevCommandReturn directly or > > | // command's interface GetReturnType() instead. > > | > > | input := `{ "return": { "enabled": true, "api-major" : 0,` + > > | `"api-minor" : 0, "build-id" : 0,` + > > | `"policy" : 0, "state" : "running",` + > > | `"handle" : 1 } } ` > > | > > | ret := QuerySevCommandReturn{} > > | err := json.Unmarshal([]byte(input), &ret) > > | if ret.Error != nil { > > | // Handle command failure {"error": { ...}} > > | } else if ret.Result != nil { > > | // ret.Result.Enable == true > > | } > > > > Signed-off-by: Victor Toso <victortoso@redhat.com> > > --- > > scripts/qapi/golang.py | 72 ++++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 70 insertions(+), 2 deletions(-) > > > > diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py > > index 52a9124641..48ca0deab0 100644 > > --- a/scripts/qapi/golang.py > > +++ b/scripts/qapi/golang.py > > @@ -40,6 +40,15 @@ > > ''' > > > > TEMPLATE_HELPER = ''' > > +type QapiError struct { > > QAPIError as the name for this Agreed. I'll fix it. > > + Class string `json:"class"` > > + Description string `json:"desc"` > > +} > > > + > > +func (err *QapiError) Error() string { > > + return fmt.Sprintf("%s: %s", err.Class, err.Description) > > +} > > My gut feeling is that this should be just > > return err.Description > > on the basis that long ago we pretty much decided that the > 'Class' field was broadly a waste of time except for a > couple of niche use cases. The error description is always > self contained and sufficient to diagnose problems, without > knowing the Class. > > Keep the Class field in the struct though, as it could be > useful to check in certain cases I'll trust you on this. I'll change it. Cheers, Victor
diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py index 52a9124641..48ca0deab0 100644 --- a/scripts/qapi/golang.py +++ b/scripts/qapi/golang.py @@ -40,6 +40,15 @@ ''' TEMPLATE_HELPER = ''' +type QapiError struct { + Class string `json:"class"` + Description string `json:"desc"` +} + +func (err *QapiError) Error() string { + return fmt.Sprintf("%s: %s", err.Class, err.Description) +} + // Creates a decoder that errors on unknown Fields // Returns nil if successfully decoded @from payload to @into type // Returns error if failed to decode @from payload to @into type @@ -254,12 +263,17 @@ func (s *{type_name}) GetId() string {{ return s.MessageId }} + +func (s *{type_name}) GetReturnType() CommandReturn {{ + return &{cmd_ret_name}{{}} +}} ''' TEMPLATE_COMMAND = ''' type Command interface {{ GetId() string GetName() string + GetReturnType() CommandReturn }} func MarshalCommand(c Command) ([]byte, error) {{ @@ -292,6 +306,45 @@ }} ''' +TEMPLATE_COMMAND_RETURN = ''' +type CommandReturn interface { + GetId() string + GetCommandName() string + GetError() error +} +''' + +TEMPLATE_COMMAND_RETURN_METHODS = ''' +type {cmd_ret_name} struct {{ + MessageId string `json:"id,omitempty"` + Error *QapiError `json:"error,omitempty"` +{result} +}} + +func (r *{cmd_ret_name}) GetCommandName() string {{ + return "{name}" +}} + +func (r *{cmd_ret_name}) GetId() string {{ + return r.MessageId +}} + +func (r *{cmd_ret_name}) GetError() error {{ + return r.Error +}} + +{marshal_empty} +''' + +TEMPLATE_COMMAND_RETURN_MARSHAL_EMPTY = ''' +func (r {cmd_ret_name}) MarshalJSON() ([]byte, error) {{ + if r.Error != nil {{ + type Alias {cmd_ret_name} + return json.Marshal(Alias(r)) + }} + return []byte(`{{"return":{{}}}}`), nil +}}''' + def gen_golang(schema: QAPISchema, output_dir: str, prefix: str) -> None: @@ -327,7 +380,7 @@ def qapi_to_go_type_name(name: str, name += ''.join(word.title() for word in words[1:]) - types = ["event", "command"] + types = ["event", "command", "command return"] if meta in types: name = name[:-3] if name.endswith("Arg") else name name += meta.title().replace(" ", "") @@ -783,6 +836,7 @@ def generate_template_command(commands: dict[str, str]) -> str: return &command.Args, nil ''' content = TEMPLATE_COMMAND.format(cases=cases) + content += TEMPLATE_COMMAND_RETURN return content @@ -926,6 +980,15 @@ def visit_command(self, type_name = qapi_to_go_type_name(name, info.defn_meta) self.commands[name] = type_name + cmd_ret_name = qapi_to_go_type_name(name, "command return") + marshal_empty = TEMPLATE_COMMAND_RETURN_MARSHAL_EMPTY.format(cmd_ret_name=cmd_ret_name) + result = "" + if ret_type: + marshal_empty = "" + ret_type_name = qapi_schema_type_to_go_type(ret_type.name) + isptr = "*" if ret_type_name[0] not in "*[" else "" + result = f'''Result {isptr}{ret_type_name} `json:"return"`''' + content = "" if boxed or not arg_type or not qapi_name_is_object(arg_type.name): args = "" if not arg_type else "\n" + arg_type.name @@ -943,7 +1006,12 @@ def visit_command(self, arg_type.variants) content += TEMPLATE_COMMAND_METHODS.format(name=name, - type_name=type_name) + type_name=type_name, + cmd_ret_name=cmd_ret_name) + content += TEMPLATE_COMMAND_RETURN_METHODS.format(name=name, + cmd_ret_name=cmd_ret_name, + result=result, + marshal_empty=marshal_empty) self.target["command"] += content def visit_event(self, name, info, ifcond, features, arg_type, boxed):
This patch adds a struct type in Go that will handle return values for QAPI's command types. The return value of a Command is, encouraged to be, QAPI's complex types or an Array of those. Every Command has a underlying CommandResult. The EmptyCommandReturn is for those that don't expect any data e.g: `{ "return": {} }`. All CommandReturn types implement the CommandResult interface. Example: qapi: | { 'command': 'query-sev', 'returns': 'SevInfo', | 'if': 'TARGET_I386' } go: | type QuerySevCommandReturn struct { | CommandId string `json:"id,omitempty"` | Result *SevInfo `json:"return"` | Error *QapiError `json:"error,omitempty"` | } usage: | // One can use QuerySevCommandReturn directly or | // command's interface GetReturnType() instead. | | input := `{ "return": { "enabled": true, "api-major" : 0,` + | `"api-minor" : 0, "build-id" : 0,` + | `"policy" : 0, "state" : "running",` + | `"handle" : 1 } } ` | | ret := QuerySevCommandReturn{} | err := json.Unmarshal([]byte(input), &ret) | if ret.Error != nil { | // Handle command failure {"error": { ...}} | } else if ret.Result != nil { | // ret.Result.Enable == true | } Signed-off-by: Victor Toso <victortoso@redhat.com> --- scripts/qapi/golang.py | 72 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 70 insertions(+), 2 deletions(-)