Closed Bug 1059453 Opened 5 years ago Closed 5 years ago

Mozlog's setup_logging should convert unicode objects appropriately

Categories

(Testing :: Mozbase, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla34

People

(Reporter: cmanchester, Assigned: cmanchester)

Details

Attachments

(1 file, 1 obsolete file)

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.
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
Attachment #8480303 - Flags: review?(james)
Assignee: nobody → cmanchester
Status: NEW → ASSIGNED
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.
Attachment #8480303 - Flags: review?(james) → review+
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.
Attachment #8480303 - Attachment is obsolete: true
Comment on attachment 8480572 [details] [diff] [review]
Treat either str or unicode as a file in mozlog's commandline.py

r=jgraham
Attachment #8480572 - Flags: review+
I can confirm that this fixes the issue locally.  Thanks for the quick turn-around, team!
https://hg.mozilla.org/mozilla-central/rev/0dca174fbc10
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.