1059453 - Mozlog's setup_logging should convert unicode objects appropriately

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Chris Manchester (limited bugmail, email directly)]

The instance check at http://dxr.mozilla.org/mozilla-central/source/testing/mozbase/mozlog/mozlog/structured/commandline.py?from=commandline.py&case=true#161 is no good for unicode objects. Could change to isinstance(value, basestring), but as I'm looking at it possibly conversion should always happen.

Comment 1

[Chris Manchester (limited bugmail, email directly)]

Attachment: Treat either str or unicode as a file in mozlog's commandline.py

nalexander, could you link me to your patch using this?

Try for windows check tests:

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=1aa8aca8dcfb

Comment 2

[James Graham [:jgraham]]

Comment on attachment 8480303 [details] [diff] [review]
Treat either str or unicode as a file in mozlog's commandline.py

Review of attachment 8480303 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine. I don't know why it was an issue though; data coming from the shell is surely always a byte string?

::: testing/mozbase/mozlog/tests/test_structured.py
@@ +1,4 @@
>  # -*- coding: utf-8 -*-
>  import argparse
> +import json
> +import mozfile

mozfile should be after the stdlib arguments, separated by a blank line.

Comment 3

[Chris Manchester (limited bugmail, email directly)]

Attachment: Treat either str or unicode as a file in mozlog's commandline.py

This fixes the import order. It sounded like nalexander was passing in arguments from another script, which should work, but isn't our expected use.

Comment 4

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8480572 [details] [diff] [review]
Treat either str or unicode as a file in mozlog's commandline.py

r=jgraham

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0dca174fbc10

Comment 6

[Nick Alexander :nalexander [he/him]]

I can confirm that this fixes the issue locally.  Thanks for the quick turn-around, team!

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0dca174fbc10