[Kimchi-devel] [PATCH V2] [Wok 0/6] Log failed user requests

Aline Manera alinefm at linux.vnet.ibm.com
Tue Jun 21 21:06:00 UTC 2016



On 06/16/2016 02:01 PM, Lucio Correia wrote:
> Hi Aline, thanks for the feedback. See my comments below.
>
> On 16-06-2016 10:18, Aline Manera wrote:
>> Hi Lucio,
>>
>> IMO we should add the original exception raised to fail a request to a
>> 'details' parameter in the JSON.
>> So the user can check the reason the requested failed.
>>
>> The exception will be raised right after you record the log entry. That
>> exception would be translate to the time of it was raised.
>> So we will need to add a logic to record only its code and when
>> requesting the user logs entries, it may be translated again to avoid
>> showing a exception in Portuguese to other user requesting the log
>> entries in Chinese, for example.
>> Do you know what I mean?
>
> Today we save only text in wok-req.log file, in json-friendly format. 
> That file is the source for any request to /logs API.
>
> Today the translation is done at the moment of logging. So the above 
> is a design change for the functionality because, in order to 
> translate the message when /logs API is called, we need to save and 
> recover the data differently.
>
>
>>
>> I have talked to you offline about the message, if there is need to have
>> separated messages for success and failed action.
>> For this first attempt, I recommend to use the same message (as you are
>> doing in this  patch set). There is no need to add a "FAILED" warning in
>> the begging of the message as it can be easily added by the UI according
>> to the status code.
>> For example, on UI, if status code different than 20X it will prefix the
>> message with "FAILED".
>> What do you think about it?
> I'm OK with it.
>
>
>>
>> Also other point we need to think about is related to those actions that
>> will generated a Task.
>> For example, when creating/cloning a guest, the POST action will, on
>> most of time, succeed but it will start a Task on background.
>> But that Task may fail, ie, the guest creation/cloning failed by some
>> reason but the POST action succeeded. How can we track that?
>> Maybe we need to differentiate the actions which rely on Task to only
>> record its success or failure when the Task has completed.
>> Any ideas on how to do that?
> It will need further investigation, but one idea is to log the result 
> of the task as well, and change the logging of requests that originate 
> tasks to something like "task created for...".
>
>
> Considering all the above, I think this patch can be commited as is 
> and then we work on the redesign, since they are not conflicting to 
> each other.
>

OK. I have open 3 new issues to cover what I have proposed above:

https://github.com/kimchi-project/wok/issues/140
https://github.com/kimchi-project/wok/issues/141
https://github.com/kimchi-project/wok/issues/142

Please, take a look when you have a chance.

Thanks,
Aline Manera

>
>>
>> Regards,
>> Aline Manera
>>
>> On 06/09/2016 05:59 PM, Lucio Correia wrote:
>>> Important:
>>> * This patchset depends on [PATCH V2] [Wok 0/3] User Request Log
>>> improvements.
>>> * This patch requires "[Kimchi] Do not break the logging of failed
>>> requests".
>>> Otherwise, some tests will be broken.
>>>
>>> Changes in V2:
>>> * Applied review suggestions
>>>
>>> Lucio Correia (6):
>>>    Revert "Use past verbs"
>>>    Parse request before authorization check
>>>    Use status code 200 for PUT requests on resource
>>>    Log failed login/logout attempts
>>>    Log failed user requests
>>>    Add status code to request log message
>>>
>>>   docs/API/logs.md        |   1 +
>>>   src/wok/control/base.py | 130
>>> ++++++++++++++++++++++--------------------------
>>>   src/wok/exception.py    |  28 ++++++++---
>>>   src/wok/i18n.py         |   4 +-
>>>   src/wok/reqlogger.py    |  10 ++--
>>>   src/wok/root.py         |  44 +++++++++++-----
>>>   tests/test_api.py       |   2 +-
>>>   7 files changed, 122 insertions(+), 97 deletions(-)
>>>
>>
>
>




More information about the Kimchi-devel mailing list