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

[Sheldon]

On 07/17/2014 08:20 PM, Aline Manera wrote:
>
> 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.
OK , I can add an "error" parameter.

But the UI developers should double check the format of "passwd".
They should not let users type an valid input.

Do you agree?

That means really kimchi user should not get this error message from 
backend.
This message is just for developers.
"KCHAPI0008E: Parameters does not match requirement in schema: 1234 is 
not of type u'string'\"


>
>>>
>>>>
>>>>> + },
>>>>> + "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
>>>>
>>>>
>>>>
>>>
>>>
>>
>
>
>


-- 
Thanks and best regards!

Sheldon Feng(冯少合)<shaohef at linux.vnet.ibm.com>
IBM Linux Technology Center