[Kimchi-devel] [PATCH] [Wok 5/7] Save log entry IDs for requests that generate tasks

[Aline Manera]

On 09/01/2016 01:15 PM, Lucio Correia wrote:
> On 01-09-2016 11:09, Aline Manera wrote:
>>
>>
>> On 08/31/2016 06:06 PM, Lucio Correia wrote:
>>> Signed-off-by: Lucio Correia <luciojhc at linux.vnet.ibm.com>
>>> ---
>>>   src/wok/control/base.py | 28 +++++++++++++++++++++++++---
>>>   1 file changed, 25 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/wok/control/base.py b/src/wok/control/base.py
>>> index 213e6b5..06f1f08 100644
>>> --- a/src/wok/control/base.py
>>> +++ b/src/wok/control/base.py
>>> @@ -26,6 +26,7 @@ import urllib2
>>>
>>>
>>>   import wok.template
>>> +from wok.asynctask import save_request_log_id
>>>   from wok.auth import USER_GROUPS, USER_NAME, USER_ROLES
>>>   from wok.control.utils import get_class_name, internal_redirect,
>>> model_fn
>>>   from wok.control.utils import parse_request, validate_method
>>> @@ -162,6 +163,7 @@ class Resource(object):
>>>                        self.info['persistent'] is True):
>>>                       result = render_fn(self, action_result)
>>>                       status = cherrypy.response.status
>>> +
>>>                       return result
>>>               except WokException, e:
>>>                   details = e
>>> @@ -171,7 +173,9 @@ class Resource(object):
>>>                   # log request
>>>                   code = self.getRequestMessage(method, action_name)
>>>                   reqParams = utf8_dict(self.log_args, request)
>>> -                log_request(code, reqParams, details, method, status)
>>
>>> +                log_id = log_request(code, reqParams, details,
>>> method, status)
>>> +                if status == 202:
>>> +                    save_request_log_id(log_id, action_result['id'])
>>
>> This should be only on generate_action_handler_task(), right? As this
>> function is specific to handle Tasks.
>>
>
> Yes, I did it originally. But I need request information and it's not 
> possible to call parse_request() again. So I needed to move it to here.
>

You can change the function to return the log_id as parameter.

Having an if statement like when we have specific function to handle 
tasks looks weird to me.

>
>
>>>
>>>           wrapper.__name__ = action_name
>>>           wrapper.exposed = True
>>> @@ -223,7 +227,7 @@ class Resource(object):
>>>               raise
>>>           finally:
>>>               # log request
>>
>>> -            if method not in LOG_DISABLED_METHODS:
>>> +            if method not in LOG_DISABLED_METHODS and status != 202:
>>
>> Why status 202 has a different behavior?
>
> It's logged separately in AsyncResource class methods, since it needs 
> log entry id and task id, which is available only there.

That is not true.
Each resource element has an ID, ie, when the resource is a Task, you 
have the Task ID there too. You can simplify use self.ident in the base 
Resource class.

>
>
>>
>>>                   code = self.getRequestMessage(method)
>>>                   log_request(code, self.log_args, details, method,
>>> status)
>>>
>>> @@ -307,6 +311,15 @@ class AsyncResource(Resource):
>>>               raise cherrypy.HTTPError(405, e.message)
>>>
>>>           cherrypy.response.status = 202
>>
>>> +
>>> +        # log request
>>> +        method = 'DELETE'
>>> +        code = self.getRequestMessage(method)
>>> +        reqParams = utf8_dict(self.log_args)
>>> +        log_id = log_request(code, reqParams, None, method,
>>> +                             cherrypy.response.status)
>>> +        save_request_log_id(log_id, task['id'])
>>> +
>>
>> Is this result of the "if status != 202" added above?
>
> Yes this is the "separate log" for requests that generate tasks 
> (return 202).

I don't think we need that.

>
>
>>
>>>           return wok.template.render("Task", task)
>>>
>>>
>>> @@ -454,7 +467,7 @@ class Collection(object):
>>>               status = e.status
>>>               raise
>>>           finally:
>>
>>> -            if method not in LOG_DISABLED_METHODS:
>>> +            if method not in LOG_DISABLED_METHODS and status != 202:
>>>                   # log request
>>
>> Same question I did before.
> Same answer above :)
>
>
>>
>>>                   code = self.getRequestMessage(method)
>>>                   reqParams = utf8_dict(self.log_args, params)
>>> @@ -480,6 +493,15 @@ class AsyncCollection(Collection):
>>>           args = self.model_args + [params]
>>>           task = create(*args)
>>>           cherrypy.response.status = 202
>>
>>> +
>>> +        # log request
>>> +        method = 'POST'
>>> +        code = self.getRequestMessage(method)
>>> +        reqParams = utf8_dict(self.log_args, params)
>>> +        log_id = log_request(code, reqParams, None, method,
>>> +                             cherrypy.response.status)
>>> +        save_request_log_id(log_id, task['id'])
>>
>> Is this result of the "if status != 202" added above?
>>
>> Unless to save the log_id to the AsyncTask instance, shouldn't the log
>> record be the same of the other HTTP return codes?
>
> the task ID is only available here, and the log ID is generated when 
> the message is logged. Here is the only place both are available.
>
>

You can access the task ID by self.ident in the base Resource class

>>
>>> +
>>>           return wok.template.render("Task", task)
>>>
>>>
>>
>
>