[Kimchi-devel] [PATCH V2 2/5] vm ticket in backend: update controller and API.json

[Aline Manera]

On 07/17/2014 07:04 AM, Royce Lv wrote:
> On 2014年07月17日 11:10, Sheldon wrote:
>> On 07/17/2014 08:44 AM, Aline Manera wrote:
>>>
>>> On 07/15/2014 12:45 PM, shaohef at linux.vnet.ibm.com wrote:
>>>> From: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
>>>>
>>>> we set ticket as sub-resource of vm resource.
>>>>
>>>> Signed-off-by: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
>>>> ---
>>>> src/kimchi/API.json | 14 ++++++++++++++
>>>> src/kimchi/control/vms.py | 13 +++++++++++++
>>>> 2 files changed, 27 insertions(+)
>>>>
>>>> diff --git a/src/kimchi/API.json b/src/kimchi/API.json
>>>> index 3616509..f5087e7 100644
>>>> --- a/src/kimchi/API.json
>>>> +++ b/src/kimchi/API.json
>>>> @@ -328,6 +328,20 @@
>>>> }
>>>> }
>>>> },
>>>> + "vmticket_update": {
>>>> + "type": "object",
>>>> + "properties": {
>>>> + "passwd": {
>>>> + "description": "the password of ticket.",
>>>> + "minLength": 1,
>>>> + "type": "string"
>>>
>>> You need to set the "error" string for invalid inputs
>> during your vocation, we have discuss this problem:
>> here is the log of our discussion.
>> http://lists.ovirt.org/pipermail/kimchi-devel/2014-May/005306.html
>> and it is improved for this "error" string
>> http://lists.ovirt.org/pipermail/kimchi-devel/2014-May/005377.html
>>
>> we think for some schema item, we do not need "error". we let kimchi
>> catch this error, and generates an common exception string.
>>
>> The generated exception string can not be translated by kimchi, it 
>> just depends on
>> jsonschema to translate it.
> According to me, params type errors and other obvious error can be 
> handled by json schema validator automatically.
> For errors need special explaination, such as "patter not match 
> :"pattern": "^[^ ]+( +[^ ]+)*$""
> which cannot be understood from schema error report, I think we can 
> specify an error number to it.
>
> So for this one the error is:
>
> {
> "reason":"KCHAPI0008E: Parameters does not match requirement in 
> schema: 1234 is not of type u'string'",
> "code":"400 Bad Request",
> "call_stack":"Traceback (most recent call last):\n File 
> \"/usr/lib/python2.7/site-packages/cherrypy/_cprequest.py\", line 656, 
> in respond\n response.body = self.handler()\n File 
> \"/usr/lib/python2.7/site-packages/cherrypy/lib/encoding.py\", line 
> 188, in __call__\n self.body = self.oldhandler(*args, **kwargs)\n File 
> \"/usr/lib/python2.7/site-packages/cherrypy/_cpdispatch.py\", line 34, 
> in __call__\n return self.callable(*self.args, **self.kwargs)\n File 
> \"/home/shhfeng/work/workdir/kimchi/src/kimchi/control/base.py\", line 
> 129, in index\n raise cherrypy.HTTPError(400, e.message)\nHTTPError: 
> (400, u\"KCHAPI0008E: Parameters does not match requirement in schema: 
> 1234 is not of type u'string'\")\n"
> }
>
> For me,
> Parameters does not match requirement in schema: 1234 is not of type 
> u'string'", is a quite clear error msg which does not need manual 
> handling.
> What do you think?

The idea behind "error" parameter is to always display a translated 
message to the user.
Without it, we will have in Portuguese:

KCHAPI0008E: Parametros não satisfazem o requisito: 1234 is not of type 
u'string'

Look, this is not a Portuguese message.
If I am using Kimchi in Portuguese I want to see it in Portuguese 
instead of part in Portuguese and part in English.

Please, keep using the "error" parameter on jsonschema to avoid those 
kind of untranslated messages.

>>
>>>
>>>> + },
>>>> + "expire": {
>>>> + "description": "lifetime of a ticket.",
>>>> + "type": "number"
>>>
>>> Same for expire
>>>
>>>> + }
>>>> + }
>>>> + },
>>>> "templates_create": {
>>>> "type": "object",
>>>> "error": "KCHTMPL0016E",
>>>> diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py
>>>> index 508f478..acc4a1f 100644
>>>> --- a/src/kimchi/control/vms.py
>>>> +++ b/src/kimchi/control/vms.py
>>>> @@ -34,6 +34,7 @@ def __init__(self, model, ident):
>>>> super(VM, self).__init__(model, ident)
>>>> self.update_params = ["name", "users", "groups", "cpus", "memory"]
>>>> self.screenshot = VMScreenShot(model, ident)
>>>> + self.ticket = VMTicket(model, ident)
>>>> self.uri_fmt = '/vms/%s'
>>>> for ident, node in sub_nodes.items():
>>>> setattr(self, ident, node(model, self.ident))
>>>> @@ -55,3 +56,15 @@ def __init__(self, model, ident):
>>>> def get(self):
>>>> self.lookup()
>>>> raise internal_redirect(self.info)
>>>> +
>>>> +
>>>> +class VMTicket(Resource):
>>>> + def __init__(self, model, ident):
>>>> + super(VMTicket, self).__init__(model, ident)
>>>> + self.update_params = ["passwd", "expire"]
>>>> + self.uri_fmt = '/vms/%s/ticket'
>>>> + self.reset = self.generate_action_handler('reset')
>>>> +
>>>> + @property
>>>> + def data(self):
>>>> + return self.info
>>>
>>>
>>>
>>
>>
>