If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Mozlog's setup_logging should convert unicode objects appropriately

RESOLVED FIXED in mozilla34

Status

Testing
Mozbase
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: chmanchester, Assigned: chmanchester)

Tracking

unspecified
mozilla34
x86_64
Linux
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
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.
(Assignee)

Comment 1

3 years ago
Created attachment 8480303 [details] [diff] [review]
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
Attachment #8480303 - Flags: review?(james)
(Assignee)

Updated

3 years ago
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+
(Assignee)

Comment 3

3 years ago
Created attachment 8480572 [details] [diff] [review]
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.
(Assignee)

Updated

3 years ago
Attachment #8480303 - Attachment is obsolete: true
(Assignee)

Comment 4

3 years ago
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+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/0dca174fbc10
Flags: in-testsuite+
Keywords: checkin-needed
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
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.