Code review comment for lp:~gary-lasker/software-center/fix-lp920741

Revision history for this message
Michael Vogt (mvo) wrote :

Thanks for this update.

The logging with "app" is only a problem in a context where both string and unicode objects are passed to the logger. In this case it will try to convert the application to a str first (via __str__) and then convert that to unicode. But because there is no __unicode__ method it will assume ascii encoding for the resulting string and fail.

This means that in the places where a single app argument is used its fine and will not crash and its also fine if all strings are of type "str". I mentioned this on irc:

Jun 27 17:15:17 <mvo> tremolux: the only thing that needs invesitgation is if we use a mixed unicode/str in a logger elsewhere that might crash in the same way

but because of the rush to work on the paypal bug I did not put that in the bugreport (I'm sorry for that).

I will revert the parts where there is just a single app argument in the logger and add it to the 5.2 branch
(the added benefit is that the diff also smaller which will make the SRU team happy :)

« Back to merge proposal