Closed Bug 1041533 Opened 6 years ago Closed 6 years ago

Instantiating b2gpopulate instance invalidates device logging setting

Categories

(Testing :: Mozbase, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla34

People

(Reporter: wlach, Assigned: wlach)

Details

Attachments

(1 file, 1 obsolete file)

Because the logger instances of devicemanager are shared, when b2gpopulate instantiates a new logger (with a default logging setting of error), any existing instances of devicemanager's logging level is reset to error. This makes it impossible to enable verbose logging throughout the eideticker harness, which we sometimes want to do for debugging purposes.
I suspect a better solution would be switch b2gpopulate to structured logging, or perhaps to remove the notion of "logging level" from devicemanager altogether. Need to think about it. In the mean time, this should fix the issues we're seeing.
Attachment #8459593 - Flags: review?(dave.hunt)
Comment on attachment 8459593 [details] [diff] [review]
Patch to allow passing in an existing instance of a logger to b2gpopulate

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

Couldn't we fix this in mozdevice, so that any new instances inherit the existing log level unless it's explicitly set? In the near future we should be able to let b2gpopulate use the device manager available from marionette.runner.device.dm, so we shouldn't need to instantiate a new one.

::: b2gpopulate/b2gpopulate.py
@@ +55,4 @@
>      handler.setFormatter(mozlog.MozFormatter(include_timestamp=True))
>      logger = mozlog.getLogger('B2GPopulate', handler)
>  
> +    def __init__(self, marionette, dm=None, log_level='INFO',

Shouldn't new keyword arguments be added to the end, in case they're being provided in order without their keywords?
Attachment #8459593 - Flags: review?(dave.hunt) → review-
After discussions with davehunt, I think the proper fix here is to set the logging level to None by default in mozdevice, then only set it to a new value if that's passed in.
Component: Eideticker → Mozbase
QA Contact: hskupin
This should prevent the log level in mozdevice being accidentally reset when using old-style logging.
Attachment #8459593 - Attachment is obsolete: true
Attachment #8460792 - Flags: review?(armenzg)
Comment on attachment 8460792 [details] [diff] [review]
[checked-in] Update mozdevice to not reset global log level

I see what you're doing here.
WFM.
Attachment #8460792 - Flags: review?(armenzg) → review+
Comment on attachment 8460792 [details] [diff] [review]
[checked-in] Update mozdevice to not reset global log level

https://hg.mozilla.org/integration/mozilla-inbound/rev/10dc968f23ea
Attachment #8460792 - Attachment description: Update mozdevice to not reset global log level → [checked-in] Update mozdevice to not reset global log level
https://hg.mozilla.org/mozilla-central/rev/10dc968f23ea
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.