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

[Aline Manera]

On 07/17/2014 11:27 AM, Sheldon wrote:
> 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?

Yeap! You should have input verification in both - backend and frontend

>
> 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'\"
>

For developer and whoever using the Kimchi REST API.

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