739368 - Talos --noisy and --debug switches should work together

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Jeff Hammel]

run_tests.py has two switches, --noisy (-n) and --debug (-d).

http://hg.mozilla.org/build/talos/file/c6013a2f09ce/talos/run_tests.py#l651

These both serve the same basic idea: to increase output (to screen)
if specified.  However, they do not talk to each other at all:

http://hg.mozilla.org/build/talos/file/c6013a2f09ce/talos/utils.py#l63

Instead, we should use a more conventional approach.  Many unix
programs use e.g. `-v` for verbose and `-v -v` for more verbose.
Python logging (http://docs.python.org/library/logging.html ) uses
levels which we can front-end to specify which level(s) go to console (e.g.):

https://github.com/mozautomation/mozmill/blob/master/mozmill/mozmill/logger.py#L19

Probably the best solution is:

1. get Talos to use mozlog (talos.logger):
https://github.com/mozilla/mozbase/blob/master/mozlog/mozlog/logger.py
1.5. Improve mozlog since its probably not adequate for what we want
to do
2. have a specifiable level from the command line that controls what
is output to screen

However, significant improve may be made here without saving the
entire world.  FWIW, I wouldn't mind if noisy was default (or debug
for that matter) for the time being. For one, when diagnosing software
issues I would prefer too much information (and while Talos has been
in production for years, in many ways it is still immature) vs not
enough.  For another, this is how it is run via buildbot, so the lack
of the --noisy flag is purely for developers that want to save screen
realestate.

Comment 1

[Fahim Zahur]

Attachment: Perform logging to screen using mozlog

I've made an attempt at a solution similar to what Jeff described. I added a global logger object from mozlog to utils.py which is then called to perform screen logging. I used the 'info' level as the 'noisy' output throughout but we can change that to some other level if it's more appropriate.

Comment 2

[Fahim Zahur]

Attachment: This adds the shutdown import to mozlog which was missing

I am not sure if this patch is correctly made, but it imports logging.shutdown() to logger.py in mozlog which was missing

Comment 3

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

Comment on attachment 614656 [details] [diff] [review]
Perform logging to screen using mozlog

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

do we have mozlog available to talos?  I believe we need to modify install.py or setup.py to include the mozlog stuff.  Also mozlog might need to define a talos instance.

I really wish we could make this simpler, instead of:
utils.moz_logger.debug(msg)

we could keep it as:
utils.debug(msg)

then in utils, define:
def debug(msg):
    moz_logger.debug(msg)


I would like to resolve the commandline arguments to run_tests.py as well.  We have --debug and --noisy.  I think we should make noisy by default and have a --debug flag as optional.  For talos, we have traditionally had noise lines as:
"NOISE: <msg>"

and debug lines as:
"DEBUG: <msg>"

I believe we can add setLevelName calls to mozlog which would resolve our need for that.  

Overall, this is a great start.  Lets get some of these details sorted out and give it another try!

::: talos/run_tests.py
@@ +54,4 @@
>  import post_file
>  from ttest import TTest
>  from results import PageloaderResults
> +import mozlog

do we need this import here?

::: talos/utils.py
@@ +51,5 @@
>  START_TIME = 0
>  saved_environment = {}
>  
> +def init_logger(level,name='Talos'):
> +	global moz_logger 

where is this really defined?

@@ +52,5 @@
>  saved_environment = {}
>  
> +def init_logger(level,name='Talos'):
> +	global moz_logger 
> +	moz_logger = mozlog.getLogger(name)

should we set a logfile name here?

Comment 4

[Fahim Zahur]

Thanks for the feedback! Where is the setLevelName method by the way? I couldn't find it. One approach for the 'debug' and 'noisy' switches might be to define them as new levels in mozlog/logger.py. What do you guys think of that?

> do we need this import here?
You're right, I forgot to remove that before making the patch

>where is this really defined?
I defined it in the line below. Should I define it in the same line like:
   global moz_logger = mozlog.getLogger(name)

>should we set a logfile name here?
It seems the logger objects always have a name('root' if nothing is specified) and I wasn't able to find a way to suppress the name from being output.

Comment 5

[Burak Yiğit Kaya [:BYK]]

Comment on attachment 614656 [details] [diff] [review]
Perform logging to screen using mozlog

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

A very good start but still needs some polishing ;)

::: talos/run_tests.py
@@ +686,4 @@
>      test_file(arg, options, parser.parsed)
> +	
> +  #stop logger
> +  utils.stop_logger()

If we can do these automatically in utils.py or somewhere else it would be better and make it harder to forget using these. (These=start/stop)

::: talos/utils.py
@@ +58,5 @@
> +		moz_level = mozlog.WARNING
> +	elif(level == 'noisy'):
> +		moz_level = mozlog.INFO
> +	elif(level == 'debug'):
> +		moz_level = mozlog.DEBUG

I think we should use a dict object here instead of multiple if statements. Or may be getattr(mozlog, level.upper()).

Comment 6

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

if you could attach the mozlog that you are using (or explain how you get it).  I want to make sure I am referencing the same bits.

Comment 7

[Fahim Zahur]

I was looking at this- https://github.com/mozilla/mozbase/tree/master/mozlog

Comment 8

[Jeff Hammel]

Comment on attachment 614656 [details] [diff] [review]
Perform logging to screen using mozlog

Pretty much what jmaher and BYK said.  It'd be nice to have global noisy + debug functions in utils.  Alternatively, we could have a new logging module.  init_logger should be done with a dictionary. the 'NOISE' level will have to print noise, and we should add this to setup.py.  (I'd also prefer logger or mozlogger vs moz_logger, but that's just a nit).

If you'd like to have any features in mozlogger's API to make this easier, please let me know and I'm happy to add.

Comment 9

[Jeff Hammel]

Comment on attachment 614658 [details] [diff] [review]
This adds the shutdown import to mozlog which was missing

Looks fine.

Comment 10

[Jeff Hammel]

(In reply to Jeff Hammel [:jhammel] from comment #9)
> Comment on attachment 614658 [details] [diff] [review]
> This adds the shutdown import to mozlog which was missing
> 
> Looks fine.

jmaher, BYK: any objections if I push?

Comment 11

[Burak Yiğit Kaya [:BYK]]

None from BYK

Comment 12

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

Comment on attachment 614658 [details] [diff] [review]
This adds the shutdown import to mozlog which was missing

lets land this!

Comment 13

[Jeff Hammel]

(In reply to Joel Maher (:jmaher) from comment #12)
> Comment on attachment 614658 [details] [diff] [review]
> This adds the shutdown import to mozlog which was missing
> 
> lets land this!

pushed: https://github.com/mozilla/mozbase/commit/c54c6b05383965af2af525cae1e9ff7ba0125cd3

Comment 14

[Fahim Zahur]

Attachment: Perform logging to screen using mozlog

Here's an updated patch. This time I've tried the implementation with a new logger module.

Issues I wasn't sure about:

1. Right now noisy is on by default but there's no way to turn it off. I could use the -n flag for turning it off.
2. I wasn't able to think of a way to avoid calling the start/stop functions in main() in run_tests.py. Do you guys have any ideas?
3. I wasn't sure about the change needed for setup.py.

Thanks for the feedback!

Comment 15

[Fahim Zahur]

Attachment: New module to do logging

Comment 16

[Jeff Hammel]

> Issues I wasn't sure about:
> 
> 1. Right now noisy is on by default but there's no way to turn it off. I
> could use the -n flag for turning it off.

We probably don't want to do this since that reverses the current meaning.  I'm not sure if there's much of a use-case at the moment no to be noisy, so it is probably fine just to leave on.

Comment 17

[Jeff Hammel]

Comment on attachment 615981 [details] [diff] [review]
Perform logging to screen using mozlog

+  else:
+	logger.start_logger('noisy')

The rest of the file uses 2-space indentation.  While I'd like to move to 4-space indentation, we shouldn't do it here.

+  # stop logger
+  logger.stop_logger()

Do we really need to stop the logger?  Why?  

Without the logger patch its hard to give much of a review, but this looks fine except the indentation issue.

Comment 18

[Fahim Zahur]

(In reply to Jeff Hammel [:jhammel] from comment #17)
> Comment on attachment 615981 [details] [diff] [review]
> Perform logging to screen using mozlog
> 
> +  else:
> +	logger.start_logger('noisy')
> 
> The rest of the file uses 2-space indentation.  While I'd like to move to
> 4-space indentation, we shouldn't do it here.
> 
> +  # stop logger
> +  logger.stop_logger()
> 
> Do we really need to stop the logger?  Why?  
> 
> Without the logger patch its hard to give much of a review, but this looks
> fine except the indentation issue.

There's a second patch with the logger module :)

Comment 19

[Burak Yiğit Kaya [:BYK]]

Comment on attachment 615981 [details] [diff] [review]
Perform logging to screen using mozlog

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

I didn't write inline comments since there were too much but the logging strings are sometimes inline formatted and sometimes done via string concatenation. They should be done as explained here: http://docs.python.org/howto/logging.html#logging-variable-data

I know the above criticism is not directly against this patch but with a new "more capable" logger, I think this should be addressed too. This is the main reason for r-, otherwise the patch is pretty good.

::: talos/run_tests.py
@@ +723,4 @@
>    if options.debug:
> +    logger.start_logger('debug') 
> +  else:
> +	logger.start_logger('noisy')

logging.basicConfig(.... level=logging.DEBUG) might be better in here.

@@ +731,4 @@
>      test_file(arg, options, parser.parsed)
> +	
> +  # stop logger
> +  logger.stop_logger()

This is not necessary.

Comment 20

[Fahim Zahur]

You guys are right, stopping the logger isn't needed. I somehow had a misconception that it gave an error message otherwise but hadn't checked it out properly. I can add an updated patch later once I have the feedback about the logger.

Comment 21

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

Comment on attachment 615981 [details] [diff] [review]
Perform logging to screen using mozlog

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

no additional comments.  Sorry for the delay on a review.  Looking forward to this landing.

Comment 22

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

Comment on attachment 615982 [details] [diff] [review]
New module to do logging

># ***** BEGIN LICENSE BLOCK *****
># Version: MPL 1.1/GPL 2.0/LGPL 2.1
>#
># ***** END LICENSE BLOCK *****

please use mpl 2.0: http://www.mozilla.org/MPL/headers/.  You are more than welcome to add your name as the initial contributor/author.

>
># Define Talos specific log level
>NOISE = mozlog.DEBUG + 1

should this be the other way where debug>noise?

>
>def start_logger(level,name='Talos'):
>	global logger 

I really would like a comment here indicating where 'logger' is defined.  

>	logger = mozlog.getLogger(name)
>	logger.setLevel(log_levels[level])	

why do we set the level here?  I assume it is so we can selectively print out messages, just below we print to noise/debug.
	

Overall this seems way to simple for a file to wrap mozlog.  If there are plans to increase this to be more robust I am game for making this file, otherwise we should modify mozlog to have an extra function or two that we need.

Comment 23

[Fahim Zahur]

> >
> ># Define Talos specific log level
> >NOISE = mozlog.DEBUG + 1
> 
> should this be the other way where debug>noise?

> 
> >	logger = mozlog.getLogger(name)
> >	logger.setLevel(log_levels[level])	
> 
> why do we set the level here?  I assume it is so we can selectively print
> out messages, just below we print to noise/debug.

The logger only prints out messages with levels >= its own level. So I set noisy at a higher level so that debug is not printed unless the logger's level is set to DEBUG. The logger's level is set to control whether debug messages will be printed.

Comment 24

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

thanks for clarifying, this makes sense!

Comment 25

[Burak Yiğit Kaya [:BYK]]

Comment on attachment 614658 [details] [diff] [review]
This adds the shutdown import to mozlog which was missing

LGTM

Comment 26

[Burak Yiğit Kaya [:BYK]]

Comment on attachment 615982 [details] [diff] [review]
New module to do logging

I am against another layer of wrapping over mozlog since it already is a layer on top of the built-in logging module. I think we should handle this behavior inside talos at the initialization level or inside mozlog itself which does not sound very good.

I think the main problem here is the additional "NOISE" logging level which I am not sure if really necessary or not. (this is of course independent from the patch)

Comment 27

[Fahim Zahur]

(In reply to Burak Yiğit Kaya [:BYK] from comment #26)
> I think the main problem here is the additional "NOISE" logging level which
> I am not sure if really necessary or not. (this is of course independent
> from the patch)

We could replace 'NOISE' with 'INFO' which is one of the default levels in the logging module if everyone thinks it's appropriate.

Comment 28

[Burak Yiğit Kaya [:BYK]]

(In reply to Fahim Zahur from comment #27)
> We could replace 'NOISE' with 'INFO' which is one of the default levels in
> the logging module if everyone thinks it's appropriate.

Makes great sense to me but someone else should also confirm this.

Comment 29

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

I am not sure what the talos log parsers require, but I would be up for changing it to INFO.  We can run through try server to ensure compatibility.

Comment 30

[Fahim Zahur]

Attachment: Perform logging to screen using mozlog

Replaced 'NOISE' with 'INFO', removed the new module and added global functions for logging in 'utils.py'

Comment 31

[Fahim Zahur]

Attachment: Perform logging to screen using mozlog

My previous patch was faulty, was missing the inline string formatting.

Comment 32

[Fahim Zahur]

Attachment: Perform logging to screen using mozlog

Sorry for repeatedly changing the patch, I realized after uploading that using logging.basicConfig would probably give a nicer solution.

Comment 33

[Burak Yiğit Kaya [:BYK]]

Comment on attachment 618896 [details] [diff] [review]
Perform logging to screen using mozlog

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

Almost perfect! You made all the "utils.noisy -> utils.info" changes, all of the string formatting changes for logging which made the code much nicer to eyes and so on. Great work!
If we can make sure removing --noisy argument makes no harm, I would be happy to give an r+ to this one and save other improvements for another bug.

::: talos/run_tests.py
@@ +613,4 @@
>    parser.add_option('-d', '--debug', dest='debug',
>                      action='store_true', default=False,
>                      help="enable debug")
> +

Is this the right thing to do, drop the --noisy argument? Because if it is in use, then it may break things by throwing "unknown argument" or similar errors.

::: talos/ttest.py
@@ +328,4 @@
>                      total_time += resolution
>                      fileData = self._ffprocess.getFile(b_log)
>                      if (len(fileData) > 0):
> +                        utils.info("%s", fileData.replace(dumpResult, ''))

This is a bit unnecessary abstraction, don't you think? =) Though instead of blatantly dumping the date we can make it something like "File data: %s".

@@ +410,4 @@
>                      results_file = open(browser_config['browser_log'], "r")
>                      results_raw = results_file.read()
>                      results_file.close()
> +                    utils.info("%s", results_raw)

Same as above: either directly use the variable value or do something like "Raw results: %s"

::: talos/utils.py
@@ +70,2 @@
>    """
> +  logging.debug(message, *args, **kwargs)

You can actually make something like debug = logging.debug ;)

@@ +73,5 @@
> +def info(message, *args, **kwargs):
> +  """Prints messages from the browser/application that are generated, otherwise
> +     these are ignored.  
> +  """
> +  logging.info(message, *args, **kwargs)  

Same thing as the debug function goes for this too.

Comment 34

[Mozilla RelEng Bot]

Try run for 993179c59a84 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=993179c59a84
Results (out of 83 total builds):
    success: 9
    failure: 74
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-993179c59a84

Comment 35

[Burak Yiğit Kaya [:BYK]]

(In reply to Mozilla RelEng Bot from comment #34)
> Try run for 993179c59a84 is complete.
> Detailed breakdown of the results available here:
>     https://tbpl.mozilla.org/?tree=Try&rev=993179c59a84
> Results (out of 83 total builds):
>     success: 9
>     failure: 74
> Builds (or logs if builds failed) available at:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-
> 993179c59a84

I think this supports my thesis about removing the --noisy argument.

Comment 36

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

all the automation failed due to:
<error>
python run_tests.py --noisy 20120426_2046_config.yml
Usage: run_tests.py [options] manifest.yml [manifest.yml ...]

run_tests.py: error: no such option: --noisy
program finished with exit code 2
elapsedTime=0.329679
</error>

I think for now we need to keep the --noisy commandline option, but make it do nothing.  This is my opinion.  Once we have the infrastructure in place we can work on cleaning up all the scripts that run these tests.  It gets complicated since the scripts are different for all the different branches, but that is something we live with for other bugs as well.

Comment 37

[Jeff Hammel]

Yeah, all of buildbot currently uses --noisy so we should keep it and label it something like "(DEPRECATED: this is now the default)"

Comment 38

[Mozilla RelEng Bot]

Try run for 30990933900e is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=30990933900e
Results (out of 85 total builds):
    success: 83
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-30990933900e

Comment 39

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

this passing try server run includes:
  parser.add_option('-n', '--noisy', dest='noisy',
                    action='store_true', default=False,
                    help="does nothing!")

Comment 40

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

Comment on attachment 618896 [details] [diff] [review]
Perform logging to screen using mozlog

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

I think we are really close.  Pretty much what BYK said.  The try server run yielded great results.

::: talos/ffprocess_linux.py
@@ +175,4 @@
>      def copyFile(self, fromfile, toDir):
>          if not os.path.isfile(os.path.join(toDir, os.path.basename(fromfile))):
>              shutil.copy(fromfile, toDir)
> +            utils.debug("installed %s", fromfile)

thanks for correcting the spelling here!

Comment 41

[Fahim Zahur]

Attachment: Perform logging to screen using the Python logging module

Added back the noisy flag and removed the debug and info functions and simply imported the 2 functions from logging

Comment 42

[Jeff Hammel]

So I'm probably being dense but why is passing the variables, as supported here --
http://docs.python.org/howto/logging.html#logging-variable-data
-- better than formatting the string yourself?

Comment 43

[Burak Yiğit Kaya [:BYK]]

(In reply to Jeff Hammel [:jhammel] from comment #42)
> So I'm probably being dense but why is passing the variables, as supported
> here --
> http://docs.python.org/howto/logging.html#logging-variable-data
> -- better than formatting the string yourself?

It is "the right way" to do it if you know what I mean in Python terms. Besides it makes the code much easier to read and follow. I hate the % operator for strings and now it is obselete too.

Comment 44

[Burak Yiğit Kaya [:BYK]]

Comment on attachment 619164 [details] [diff] [review]
Perform logging to screen using the Python logging module

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

Perfect!

::: talos/run_tests.py
@@ +615,4 @@
>                      help="enable debug")
>    parser.add_option('-n', '--noisy', dest='noisy',
>                      action='store_true', default=False,
> +                    help="(DEPRECATED: this is now the default)")

Good explanation.

Comment 45

[Jeff Hammel]

Not to further complicate things, but PerfConfigurator additionally features a --verbose switch. IMHO, we probably only need one switch total if --noisy is now the default

Comment 46

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

Comment on attachment 619164 [details] [diff] [review]
Perform logging to screen using the Python logging module

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

Thanks for writing this, lets work on getting this landed.

Comment 47

[Jeff Hammel]

Comment on attachment 619164 [details] [diff] [review]
Perform logging to screen using the Python logging module

Sorry forgot that this was in my review queue.  r+.  This doesn't look like it uses mozlog?  Are we comfortable with that?

Comment 48

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

lets land this and file a followup to use mozlog

Comment 49

[Jeff Hammel]

Currently bitrotted. :(

Comment 50

[Fahim Zahur]

Attachment: Perform logging to screen using mozlog

I've rebased the patch on the latest talos revision. Also using mozlog instead of logging, but because of that logger.py in mozlog needs a couple of additional imports.

Comment 51

[Fahim Zahur]

Attachment: Added some additional imports to mozlog to allow them to be used in utils.py in talos

Comment 52

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

Comment on attachment 623003 [details] [diff] [review]
Added some additional imports to mozlog to allow them to be used in utils.py in talos

simple!

Comment 53

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

Comment on attachment 623002 [details] [diff] [review]
Perform logging to screen using mozlog

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

Thanks for the unbitrot'd patch.

Comment 54

[Jeff Hammel]

Comment on attachment 623002 [details] [diff] [review]
Perform logging to screen using mozlog

   parser.add_option('-n', '--noisy', dest='noisy',
                     action='store_true', default=False,
-                    help="enable noisy output")
+                    help="(DEPRECATED: this is now the default)")
   options, args = parser.parse_args(args)
 
   # set variables
   if options.debug:
     print 'setting debug'
-    utils.setdebug(1)
+    utils.startLogger('debug')
   if options.noisy:
-    utils.setnoisy(1)
+    utils.startLogger('info')
 
Shouldn't the default for --noisy be True (and maybe also 

level = 'info'
if options.debug:
   print "setting debug"
   level = 'debug'
utils.startLogger(level)

Comment 55

[Fahim Zahur]

Attachment: Perform logging to screen using mozlog

Fixed run_tests.py so that 'info' the default level. My bad on missing that in the last patch.

Comment 56

[Jeff Hammel]

Comment on attachment 623216 [details] [diff] [review]
Perform logging to screen using mozlog

looks good to me.  We'll need a setup.py change and a create_talos_zip.py change for the mozlog import

Comment 57

[Burak Yiğit Kaya [:BYK]]

Comment on attachment 623216 [details] [diff] [review]
Perform logging to screen using mozlog

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

LGTM and +1 to jhammel's setup.py comment. Though I'm not sure if that should be a part of this bug. May be creating another bug and setting that as a blocker for this one is a good idea.

::: talos/run_tests.py
@@ +610,2 @@
>    if options.debug:
>      print 'setting debug'

Is this line really necessary?

::: talos/ttest.py
@@ +183,1 @@
>                                                                os.path.basename(dump)))

Minor indent problem here.

Comment 58

[Jeff Hammel]

> @@ +610,2 @@
> >    if options.debug:
> >      print 'setting debug'
> 
> Is this line really necessary?

FWIW, its what we do currently.  I don't really care whether we keep the line or not.

Comment 59

[Jeff Hammel]

Attachment: add mozlog to setup.py and create_talos_zip.py

tested locally; needs to be tested on try

Comment 60

[Jeff Hammel]

sadly, this fails with qimport, i believe because of trailing CRs:

│hg import https://bug739368.bugzilla.mozilla.org/attachment.cgi?id=623216 
applying https://bug739368.bugzilla.mozilla.org/attachment.cgi?id=623216
patching file talos/ffprocess_linux.py
Hunk #1 FAILED at 174
1 out of 1 hunks FAILED -- saving rejects to file talos/ffprocess_linux.py.rej
patching file talos/ffprocess_mac.py
Hunk #1 FAILED at 186
1 out of 1 hunks FAILED -- saving rejects to file talos/ffprocess_mac.py.rej
patching file talos/ffprocess_win32.py
Hunk #1 FAILED at 191
1 out of 1 hunks FAILED -- saving rejects to file talos/ffprocess_win32.py.rej
patching file talos/run_tests.py
Hunk #1 FAILED at 94
Hunk #2 FAILED at 164
Hunk #3 FAILED at 509
Hunk #4 FAILED at 556
Hunk #5 FAILED at 601
5 out of 5 hunks FAILED -- saving rejects to file talos/run_tests.py.rej
patching file talos/ttest.py
Hunk #1 FAILED at 166
Hunk #2 FAILED at 178
Hunk #3 FAILED at 221
Hunk #4 FAILED at 243
Hunk #5 FAILED at 260
Hunk #6 FAILED at 293
Hunk #7 FAILED at 327
Hunk #8 FAILED at 419
Hunk #9 FAILED at 430
9 out of 9 hunks FAILED -- saving rejects to file talos/ttest.py.rej
patching file talos/utils.py
Hunk #1 FAILED at 42
Hunk #2 FAILED at 59
2 out of 2 hunks FAILED -- saving rejects to file talos/utils.py.rej
abort: patch failed to apply

If anyone is more of a hg wizard than I I'd love to know how to do this.  FWIW, `curl https://bug739368.bugzilla.mozilla.org/attachment.cgi?id=623216 | patch -p1` does work

Comment 61

[Jeff Hammel]

Attachment: combination of both patches hopefully in a form palatable to hg import

Comment 62

[Jeff Hammel]

pushed talos with attachment 623515 [details] [diff] [review] to try: https://tbpl.mozilla.org/?tree=Try&rev=2e1ac94e6011

Comment 63

[Jeff Hammel]

Someone's messed with Try server:

 using PTY: False
INFO: talos.json URL: http://hg.mozilla.org/try/raw-file/2e1ac94e6011/testing/talos/talos.json
INFO: Downloading http://people.mozilla.com/~jhammel/talos.bug-739368.zip as talos.zip
ERROR: HTTP Error 403: Forbidden
program finished with exit code 1
elapsedTime=0.840871

from

https://tbpl.mozilla.org/php/getParsedLog.php?id=11717941&tree=Try&full=1

Comment 64

[Jeff Hammel]

(In reply to Jeff Hammel [:jhammel] from comment #63)
> Someone's messed with Try server:
> 
>  using PTY: False
> INFO: talos.json URL:
> http://hg.mozilla.org/try/raw-file/2e1ac94e6011/testing/talos/talos.json
> INFO: Downloading http://people.mozilla.com/~jhammel/talos.bug-739368.zip as
> talos.zip
> ERROR: HTTP Error 403: Forbidden
> program finished with exit code 1
> elapsedTime=0.840871
> 
> from
> 
> https://tbpl.mozilla.org/php/getParsedLog.php?id=11717941&tree=Try&full=1

Hmm, this is a legitimate forbidden.  Since my upload script hasn't changed this means that the way people.m.o works has changed :(  Will investigate

Comment 65

[Jeff Hammel]

(In reply to Jeff Hammel [:jhammel] from comment #64)
> (In reply to Jeff Hammel [:jhammel] from comment #63)
> > Someone's messed with Try server:
> > 
> >  using PTY: False
> > INFO: talos.json URL:
> > http://hg.mozilla.org/try/raw-file/2e1ac94e6011/testing/talos/talos.json
> > INFO: Downloading http://people.mozilla.com/~jhammel/talos.bug-739368.zip as
> > talos.zip
> > ERROR: HTTP Error 403: Forbidden
> > program finished with exit code 1
> > elapsedTime=0.840871
> > 
> > from
> > 
> > https://tbpl.mozilla.org/php/getParsedLog.php?id=11717941&tree=Try&full=1
> 
> Hmm, this is a legitimate forbidden.  Since my upload script hasn't changed
> this means that the way people.m.o works has changed :(  Will investigate

Noted at https://bugzilla.mozilla.org/show_bug.cgi?id=754703

Comment 66

[Jeff Hammel]

pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=6774a185f87b

Hopefully IT can advise on bug 754703 as the current situation makes it difficult for me to push talos changes to try

Comment 67

[Jeff Hammel]

problems on e.g. https://tbpl.mozilla.org/php/getParsedLog.php?id=11730574&tree=Try&full=1 :(

Comment 68

[Jeff Hammel]

It looks like OSX64 opt fails...not sure why it would be any different :/

Comment 69

[David Chan [:dchan]]

Apparently the umask change to 0027. This removes rwx from the other group on newly created files / directories. Until IT changes it, a workaround is to edit your ~/.bashrc and add
umask 0022

This will restore umask when you logon. You may have to change your program to set the appropriate umask depending on how it is called.

Comment 70

[Jeff Hammel]

fwiw, i added umask 0000 to my .bashrc but the resulting files are still forbidden :(

Comment 71

[Jeff Hammel]

Given the failure on try, maybe the thing to do is to try the non-mozlog patch and see if that fares any better.

Comment 72

[Burak Yiğit Kaya [:BYK]]

Comment on attachment 623003 [details] [diff] [review]
Added some additional imports to mozlog to allow them to be used in utils.py in talos

I noticed this was waiting for my review. Sorry for the late response. LGTM.

Comment 73

[Jeff Hammel]

Hmmm, so I dropped the ball on getting this checked in.  Lemme look at this and see what there is to do

Comment 74

[Jeff Hammel]

Sadly, though not surprisingly, this has bitrotted :(

Comment 75

[Jeff Hammel]

This is in practice not a good entry level bug.  I'm happy to help with any of this, but unmarking as a good first bug.

Comment 76

[cmtalbert]

jmaher could we investigate what it would take to land the work already done here or if it is too bitrotted, then maybe we just push back and wait for new patches?

Comment 77

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

Attachment: updated patch to use mozlog and noisy by default (1.0)

Comment 78

[Jeff Hammel]

Comment on attachment 740868 [details] [diff] [review]
updated patch to use mozlog and noisy by default (1.0)

lgtm if this has been tested

Comment 79

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

http://hg.mozilla.org/build/talos/rev/ba0ef3d82bdb