logbook handler str.format {0} style expansion

[temoto]

Thanks for built-in logbook handler.

logbook allows str.format style parameter expansion, for example: log.error(u"foo: {0} bar: {bar}", 7, bar="ok").

But it breaks with TypeError("not all arguments converted during string formatting"):

    # raven.events.Message.to_string 110:
        return msg['message'] % msg['params']

I haven't yet found best solution, but it would seem that one should use logbook's built-in expansion and not apply % operator later. I would also think that same logic applies to logging handler, since logging also has expansion built-in (although only % style one, probably that's why usually=when-using-logging everything "just works").

[temoto]

Actually, please see the patch, works for me.

[dcramer]

This will fix your error but it will break grouping on the message.

The reason we pass both the unformatted message and the params is because the grouping key is actually based on the unformatted message

[temoto]

Yes, i noticed it now, indeed, grouping is better. Have to find some other way.

I will try more sophisticated approach next time.

[dcramer]

We could change the message interface so it accepts three args instead of two:

message, params, formatted

[temoto]

Doesn't seem that you would need params then :)

[dcramer]

We still show them on the page, but maybe that's not very important

David Cramer

On Thursday, February 14, 2013 at 12:09 PM, Sergey Shepelev wrote:

Doesn't seem that you would need params then :)

—
Reply to this email directly or view it on GitHub (#260 (comment)).

[temoto]

Oh i'm sorry, i thought those were passing separately as data.

[temoto]

Okay, as i was writing next proposal i realized it wouldn't work. So maybe passing params separately is indeed the only solution.

Just consider str.format also accepts both args tuple and kwargs dict.

[dcramer]

The problem with str.format is it doesnt seem to handle the old style of formatting:

'hello %s'.format('world')
'hello %s'

David Cramer

On Thursday, February 14, 2013 at 12:14 PM, Sergey Shepelev wrote:

Okay, as i was writing next proposal i realized it wouldn't work. So maybe passing params separately is indeed the only solution.
Just consider str.format also accepts both args tuple and kwargs dict.

—
Reply to this email directly or view it on GitHub (#260 (comment)).

[dcramer]

Also FWIW, I'm not too familiar with logbook, so if it ONLY supports new-style formatting, then we could make some changes so that's easier.

Also I think the way way we format params is technically invalid with logging, so we should probably change the interface either way to accept the already-formatted message.

David Cramer

On Thursday, February 14, 2013 at 12:15 PM, David Cramer wrote:

The problem with str.format is it doesnt seem to handle the old style of formatting:

'hello %s'.format('world')
'hello %s'

David Cramer

On Thursday, February 14, 2013 at 12:14 PM, Sergey Shepelev wrote:

Okay, as i was writing next proposal i realized it wouldn't work. So maybe passing params separately is indeed the only solution.
Just consider str.format also accepts both args tuple and kwargs dict.

—
Reply to this email directly or view it on GitHub (#260 (comment)).

[temoto]

Indeed, logbook supports only new style formatting. What is quick fix in this case? I'd rather return grouping.

[temoto]

Thank you very much.

I wonder if handler_kwargs['params'] is missing record kwargs...

handler_kwargs = {
    'message': record.msg,
    'params': record.args + record.kwargs.values() #?
}