1040079 - Adjust logging levels for devicemanager*.py

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Armen [:armenzg]]

This seems like an easy first bug.

The intention of this bug is to get informative messages into the tbpl logs and determine if on mozdevice we should changes some messages to the right level.

The steps to follow would be:
* change the default mozlog.ERROR to mozlog.INFO [1]
* review the logging messages and see if they need to be change between info/debug/error
** change directory to testing/mozbase/mozdevice/mozdevice
** run "grep -E "(info|debug|error)\(" devicemanager*py" to find all instances
* attach your patch and request me to have a look
** once we have a good patch we can test it on try
* we can push to try with try: -b o -p linux64,android,emulator -u xpcshell,mochitest-1,crashtest-1,reftest-1,marionette-webapi -t none
** that would generate a series of jobs that would put your changes to the test
** you would have to have a 

[1] http://hg.mozilla.org/mozilla-central/file/f92e4d13b3c6/testing/mozbase/mozdevice/mozdevice/devicemanager.py#l49

Comment 1

[William Lachance (:wlach)]

This is not too difficult, but is probably a bit involved for a good first bug. Let's make it a good next bug.

Comment 2

[abhi12ravi]

Hi Armen, 

I would like to work on this bug. Can you please assign it to me?

Comment 3

[abhi12ravi]

Attachment: 1040079.patch

Hi Armen, 

I have followed all the steps that you have given. Do let me know if what I have done is what you wanted. 

Thank you.

Comment 4

[Armen [:armenzg]]

Hello abhi12ravi,
My apologies for the late reply. I've been out for holidays.
I should be able to look into this with detail on Tuesday.

Thanks for the patch!

Comment 5

[Armen [:armenzg]]

Comment on attachment 8477934 [details] [diff] [review]
1040079.patch

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

Thanks abhi12ravi for the patch. Overall the patch is heading on the right direction, however, somethings have changed since the bug got filed. I need to figure those out.

Comment 6

[Armen [:armenzg]]

Hi wlach,
Since I filed the bug it seems that few things have changed.
At this point, we don't have a default logLevel as we used to [1].
From looking at tbpl, I can't see any output of mozdevice anymore.
I know that jgraham was excited to be able to see so much more output of mozdevice since it allowed him to know what was going on.

I think we have to discuss what we did on bug 1041533 and see if we have to change anything.
Otherwise we would have to go around the tree and have to set a level for each call of DeviceManagerADB.

What do you think?

On the other hand, this is not blocking anything but rather trying to improve our logging status.

[1] http://hg.mozilla.org/mozilla-central/rev/10dc968f23ea

Comment 7

[William Lachance (:wlach)]

Hey Armen, sorry about not giving feedback on this earlier.

I don't want to revisit bug 1041533, as that approach caused some pretty weird behaviour that was hard to track down. Also, I think most of the existing logging in mozdevice.DeviceManagerADB is at about the right level, I don't think we should change debug->info in most of the cases of [e-mail]'s patch, the only thing that stuck out as the right thing was maybe the reverse switch from info->debug in the chmod calls.

I think at this point it would be best to focus on making mozdevice work well with structured logging, it's not really clear to me that further adjustments on how we're doing old-style logging in tree would provide a good return on time invested at this point. I suspect James or Chris would be the best people to ask about that.

Comment 8

[abhi12ravi]

Hey Armen and William, 

Thanks for taking a look at my patch. Do I wait till you guys figure out what's supposed to be done? Learning about the log levels was interesting, thanks for that!

Comment 9

[William Lachance (:wlach)]

Hey Armen, I think I'd like to just apply the two changes I mentioned in comment 7 and then close out this bug. Is that ok with you?

Comment 10

[Armen [:armenzg]]

(In reply to William Lachance (:wlach) from comment #9)
> Hey Armen, I think I'd like to just apply the two changes I mentioned in
> comment 7 and then close out this bug. Is that ok with you?

Yes, both proposed changes make change. Thanks for keeping this alive!

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

armen, can you wrap this up?

Comment 12

[Armen [:armenzg]]

Attachment: change chmod from info to debug

Comment 13

[William Lachance (:wlach)]

Comment on attachment 8555406 [details] [diff] [review]
change chmod from info to debug

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

lgtm

Comment 14

[Armen [:armenzg]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fabb44ffd5d0

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fabb44ffd5d0