1088590 - Add support for timeouts in tools

Status: RESOLVED FIXED

(Release Engineering :: Release Automation: Other, defect, P2)

Description

[Massimo Gervasini [:massimo]]

Recently, during the release process for Firefox 34.0b3, we had two hg timeouts checking out locales. 

We have retry() function around most of the hg operations but if a request takes too long, the process is terminated by buildbot, so retry() never happens.

We should add an optional timeout parameter in retry(), so if a retried operation is not completed after a given amount of time, it raises an exception and retry takes care of re-running it.

retry code in tools: https://mxr.mozilla.org/build/source/tools/lib/python/util/retry.py

Comment 1

[Massimo Gervasini [:massimo]]

Attachment: [tools] Bug 1088590 - Add a timeout for retries.patch

Actually, we have run_cmd_periodic_poll() which can be used to manipulate a process if it runs for longer than 'warning_interval' seconds, so there's no need to update retry()

This patch enables the use of run_cmd_periodic_poll() in apply_and_push() so now tag-release.py can terminate a hg push if it takes longer than 120 seconds.

Comment 2

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8514478 [details] [diff] [review]
[tools] Bug 1088590 - Add a timeout for retries.patch

Seems fine to me, and 120 seconds should be plenty for the small number of changesets we push in tagging.

catlee, you know this code well, could you take a look too ?

Comment 3

[Chris AtLee [:catlee]]

Comment on attachment 8514478 [details] [diff] [review]
[tools] Bug 1088590 - Add a timeout for retries.patch

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

::: lib/python/util/commands.py
@@ +259,5 @@
> +def terminate_on_timeout(start_time, elapsed, proc):
> +    """a callback to use with run_cmd_periodic_poll, that terminates the process
> +       after a given timeout"""
> +    log.debug("operation timeout after %s seconds)" % (str(elapsed)))
> +    log.debug("terminating %s" % (proc))

I would use something like log.warn or even log.error here.

Also a small style nit: using log.debug("terminating %s", proc) is preferred over log.debug("terminating %s" % (proc)) if we're using standard python logging.

Comment 4

[Massimo Gervasini [:massimo]]

Comment on attachment 8514478 [details] [diff] [review]
[tools] Bug 1088590 - Add a timeout for retries.patch

Thanks Nick & Chris for the reviews.

Landed with changes in comment #3

Comment 5

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8514478 [details] [diff] [review]
[tools] Bug 1088590 - Add a timeout for retries.patch

I backed this out at 71c12bc85a1c. hg.m.o seems to accept changes quickly, then be slow to sync them out to the webheads. If we retry before it's done we get multiple tags like this http://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=8e812440658b&tochange=ee3571fc463c 
(pushlog isn't showing relbranches at the moment)

Reading back on this bug, it was originally filed for cloning timeouts and we patched pushing. Something changed ?

Comment 6

[Massimo Gervasini [:massimo]]

(In reply to Nick Thomas [:nthomas] from comment #5)
> Comment on attachment 8514478 [details] [diff] [review]
> [tools] Bug 1088590 - Add a timeout for retries.patch
> 
> I backed this out at 71c12bc85a1c. hg.m.o seems to accept changes quickly,
> then be slow to sync them out to the webheads. If we retry before it's done
> we get multiple tags like this
> http://hg.mozilla.org/releases/mozilla-beta/
> pushloghtml?fromchange=8e812440658b&tochange=ee3571fc463c 
> (pushlog isn't showing relbranches at the moment)
> 
> Reading back on this bug, it was originally filed for cloning timeouts and
> we patched pushing. Something changed ?

Sorry, I had it wrong in the first comment, we had timeouts pushing, not cloning.

I haven't thought about the use case where the push actually works but there's no answer from the server, in this case retry will create multiple tags.

This patch was meant to mitigate the issue where we cannot complete the tagging job because pushing a locale repository gets stuck and buildbot terminates the full job. 

How about increasing the warning_interval to a much higher value, close to the timeout without output limit? In this case we might end up with multiple tags but we could save the tagging job.

In any case we should improve the tagging process with Bug 607392 - "split tagging into en-US and other" and Bug 1072079 - 'stop blocking on tagging official builds"

Comment 7

[Massimo Gervasini [:massimo]]

Attachment: [tools] Bug 1088590 - add a timeout for clone opertations.patch

Using the same push timeout logic for cloning operations.
Now the clone will be terminated if the operation takes more than 7200 seconds (not sure what's the best timeout value, here)

Comment 8

[Nick Thomas [:nthomas] (UTC+12)]

I'm sorry I haven't gotten to this feedback request. At this point I won't be able to until mid-january, so feel free to ask someone like rail or ben in the meantime.

Comment 9

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8527868 [details] [diff] [review]
[tools] Bug 1088590 - add a timeout for clone opertations.patch

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

Seems reasonable to me, based on similar code already in these files.

::: lib/python/mozilla_buildtools/test/test_util_hg.py
@@ +1175,5 @@
> +        # The operation will be interrupted by the timeout. Retry should
> +        # intercept the failure and re-start the clone a few times but,
> +        # at the end, the clone must fail.
> +        self.assertRaises(subprocess.CalledProcessError, clone, self.repodir,
> +                          self.wc, timeout=0.01, poll_interval=0.05)

Seems strange to have a timeout shorter than the poll_interval, might lead to incomplete testing. Are there other tests we could add ?

::: lib/python/util/commands.py
@@ +211,5 @@
> +
> +        now = time.time()
> +        if now - start_time > timeout:
> +            elapsed = now - start_time
> +            log.info("clone process, is taking too long: %ss (timeout %s). Terminating" %

Could be used for other things, so 'process took too long ...'.

@@ +217,5 @@
> +            proc.terminate()
> +            proc.wait()
> +            raise subprocess.CalledProcessError(proc.returncode, cmd,
> +                                                output='timeout, process terminated')
> +

Missing the time.sleep(poll_interval) here.

::: lib/python/util/hg.py
@@ +87,4 @@
>      return get_output(['hg'] + cmd, env=env, **kwargs)
>  
>  
> +def hg_terminate_on_timeout(cmd, timeout=7200, poll_interval=0.25,

maybe get_hg_output_with_timeout().

2 hours is a pretty long timeout, even for cloning a big gecko repo. What do we set for max job time, and max time without output in tagging jobs ?

Comment 10

[Hal Wine [:hwine] use NI!]

I believe :gps wants "--traceback" added to all of the hg invocations, especially in a case like this. ni to him for any additional changes.

Comment 11

[Gregory Szorc [:gps]]

Yes, adding --traceback is useful to track down exactly what caused some failures in Mercurial. We shouldn't see many Mercurial failures, so adding --traceback globally shouldn't hurt.

Comment 12

[Massimo Gervasini [:massimo]]

Attachment: [tools] Bug 1088590 - add a timeout for hg clone; add --extend option to hg operations.patch

Hi Nick,

changes in this patch:
* updated poll_interval in tests to 0.01s. Instead of setting a very short timeout, we could actually have a clone operation that takes some seconds, but I haven't found a decent way to do it. Any idea?
* updated terminate process log message
* added missing time.sleep(poll_interval)
* renamed hg_terminate_on_timeout => get_hg_output_with_timeout
* about the timeout: on buildbot we have script_timeout = 3h and script_max_time = 40 mins. A fresh clone of m-c takes around 20 mins on a windows slave, so I have set the new default for this script to 30 mins
* added --traceback option to all clone, pull and push operations as requested in comment #10 and #11

Comment 13

[Nick Thomas [:nthomas] (UTC+12)]

(In reply to Massimo Gervasini [:mgerva|pto => 02.02.2015] from comment #12)
> changes in this patch:
> * updated poll_interval in tests to 0.01s. Instead of setting a very short
> timeout, we could actually have a clone operation that takes some seconds,
> but I haven't found a decent way to do it. Any idea?

The way the changes are structured you need a 'hg sleep 2', or to invent some long running operation (which is still quick to setup, to keep test runtime as short as possible). 'hg serve' could be a (hacky) substitute. 

The sort of tests I was hoping for was for poll_interval to be shorter than the timeout, and check the number of time.sleep calls (which would have caught it being missing, I think).  test_util_commands.py has tests for run_cmd() and run_cmd_periodic_poll() with the sort of thing I mean. Perhaps it isn't much work to modify run_cmd_periodic_poll to add a timeout, as well as the existing warning. The existing usage is at http://mxr.mozilla.org/build/source/tools/buildfarm/release/release-runner.py#179, so we'd have to have a timeout=None in the function definition.

Ben, do you have any opinion on this at all ?

> * about the timeout: on buildbot we have script_timeout = 3h and
> script_max_time = 40 mins. A fresh clone of m-c takes around 20 mins on a
> windows slave, so I have set the new default for this script to 30 mins

These seems a fine value for timeout on hg clone's.

Comment 14

[[e-mail] (:bhearsum)]

(In reply to Nick Thomas [:nthomas] from comment #13)
> (In reply to Massimo Gervasini [:mgerva|pto => 02.02.2015] from comment #12)
> > changes in this patch:
> > * updated poll_interval in tests to 0.01s. Instead of setting a very short
> > timeout, we could actually have a clone operation that takes some seconds,
> > but I haven't found a decent way to do it. Any idea?
> 
> The way the changes are structured you need a 'hg sleep 2', or to invent
> some long running operation (which is still quick to setup, to keep test
> runtime as short as possible). 'hg serve' could be a (hacky) substitute. 
> 
> The sort of tests I was hoping for was for poll_interval to be shorter than
> the timeout, and check the number of time.sleep calls (which would have
> caught it being missing, I think).  test_util_commands.py has tests for
> run_cmd() and run_cmd_periodic_poll() with the sort of thing I mean. Perhaps
> it isn't much work to modify run_cmd_periodic_poll to add a timeout, as well
> as the existing warning. The existing usage is at
> http://mxr.mozilla.org/build/source/tools/buildfarm/release/release-runner.
> py#179, so we'd have to have a timeout=None in the function definition.
> 
> Ben, do you have any opinion on this at all ?

Sounds totally fine to me. Maybe it's a bad idea to have run_cmd_periodic_poll default to killing things after a period of time? Ie, make timeout=None the default, and pass an int to opt-in to that behaviour.

Comment 15

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8549678 [details] [diff] [review]
[tools]  Bug 1088590 - add a timeout for hg clone; add --extend option to hg operations.patch

OK. mgerva, please look at extending run_cmd_periodic_poll to optionally terminate after a timeout. Sorry for the change of direction at this point.

Comment 16

[Massimo Gervasini [:massimo]]

Attachment: [tools] Bug 1088590 - add a timeout for hg clone; add --traceback option to hg operations II.patch

Thanks Ben, thanks Nick!

changes in this patch:
* added a timeout to run_cmd_periodic_poll() and get_hg_output(). Default timeout is set to 1 day. I have decided to add a very high value for timeout rather than set it to None, to avoid another code branch ("if timeout is None:") - so by default, the above functions are terminating the process if it takes too long but in reality this will never happen (buildbot will take care of terminating stuck commands)
* removed get_output_and_terminate_on_timeout() 
* tests: added testOutputTimeout() and testSuccessTerminateAfterTimeout(); removed testCloneTimeout() - there's no need to simulate a slow clone with testSuccessTerminateAfterTimeout()

Comment 17

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8557965 [details] [diff] [review]
[tools] Bug 1088590 - add a timeout for hg clone; add --traceback option to hg operations II.patch

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

f+ as I think it needs a little more.

::: lib/python/mozilla_buildtools/test/test_util_commands.py
@@ +72,4 @@
>                                    warning_interval=2),
>              0)
>  
> +    def testSuccessTerminateAfterTimeout(self):

How about testTerminatedByTimeout() ?  Would be good to add a 'class TestGetOutput' just before 'def testOutput' too.

@@ +74,5 @@
>  
> +    def testSuccessTerminateAfterTimeout(self):
> +        self.assertRaises(
> +            subprocess.CalledProcessError, run_cmd_periodic_poll,
> +            ['bash', '-c', 'sleep 3 && true'], warning_interval=1, timeout=2)

In the interests of test running faster, how about sleep 1 and timeout=0.5

::: lib/python/util/commands.py
@@ +117,3 @@
>              # reset last_check to avoid spamming callback
>              last_check = now
>              elapsed = now - start_time

I'd calculate elapsed just before the warning test, and use it wherever the now - start_time is needed.

@@ +139,5 @@
> +                     (now - start_time, timeout))
> +            proc.terminate()
> +            proc.wait()
> +            raise subprocess.CalledProcessError(proc.returncode, cmd,
> +                                                output=TERMINATED_PROCESS_MSG)

Won't see anything from the process output here, I think. See comments for get_output().

@@ +200,5 @@
> +
> +        now = time.time()
> +        if now - start_time > timeout:
> +            elapsed = now - start_time
> +            log.info("process, is taking too long: %ss (timeout %s). Terminating" %

Please use '(timeout %ss)' too., so they both have units.

@@ +203,5 @@
> +            elapsed = now - start_time
> +            log.info("process, is taking too long: %ss (timeout %s). Terminating" %
> +                     (now - start_time, timeout))
> +            proc.terminate()
> +            proc.wait()

Hopefully we won't need to do proc.kill() after a while, lets see what happens. There's a potential deadlock in .wait() if the buffer is full though.

@@ +205,5 @@
> +                     (now - start_time, timeout))
> +            proc.terminate()
> +            proc.wait()
> +            raise subprocess.CalledProcessError(proc.returncode, cmd,
> +                                                output='timeout, process terminated')

Should be TERMINATED_PROCESS_MSG, I think. Also will be useful to see the output from the process, so .communicate() is probably the way to go.

::: lib/python/util/hg.py
@@ +68,4 @@
>          return urlsplit(repo).path.lstrip("/")
>  
>  
> +def get_hg_output(cmd, timeout=1800, poll_interval=0.25, **kwargs):

poll_interval probably isn't needed here, just timeout.

@@ +162,5 @@
>  
>  
>  def clone(repo, dest, branch=None, revision=None, update_dest=True,
> +          clone_by_rev=False, mirrors=None, bundles=None, timeout=7200,
> +          poll_interval=0.25):

poll_interval probably isn't needed here, just timeout.

@@ +188,5 @@
> +    subprocess will be terminated. This features allows to terminate hung clones
> +    before buildbot kills the full jobs. When a timeout terminates the process,
> +    the exception is caught by `retrier`.
> +
> +    `poll_interval` determines how frequently the hg clone process is polled.

... so this comment could be removed.

@@ +381,4 @@
>  
>  
>  def push(src, remote, push_new_branches=True, force=False, **kwargs):
> +    cmd = ['hg', 'push', '--traceback']

unbundle() needs --traceback, and a longer timeout than 30 minutes.

Comment 18

[Massimo Gervasini [:massimo]]

Attachment: [tools] Bug 1088590 - add a timeout in get_output and in run_cmd_periodic_poll; add --traceback option to hg operations III.patch

Thanks for the feedback, Nick!

Changes in this version:
* fixes requested in comment 17
* get_output logs the output of the terminated process
* new tests

Comment 19

[Massimo Gervasini [:massimo]]

Attachment: [tools] interdiffs

Changes from previous and current patch

Comment 20

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8566140 [details] [diff] [review]
[tools] Bug 1088590 - add a timeout in get_output and in run_cmd_periodic_poll; add --traceback option to hg operations III.patch

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

Just minor stuff now, f+

In test_util_commands.py I'd really like to see three classes: TestRunCmd, TestRunCmdPeriodicPoll, and TestGetOutput; then only test the matching function in each. They're a bit mixed up at the moment.

It would be really good to get a list of timeouts on each of the hg operations, as not all use get_hg_output() and some call run_cmd directly. Please update the bug summary with that in mind, since I don't believe push() will be changed here (that's fine!).

::: lib/python/mozilla_buildtools/test/test_util_commands.py
@@ +83,5 @@
> +        cmd = ['bash', '-c', 'echo "%s" && echo "%s" 1>&2 && sleep 1 && true' % (text_s, text_e)]
> +        try:
> +            get_output(cmd, timeout=0.5)
> +        except subprocess.CalledProcessError as error:
> +            print error

Nit, leftover debugging ?

@@ +85,5 @@
> +            get_output(cmd, timeout=0.5)
> +        except subprocess.CalledProcessError as error:
> +            print error
> +            self.assertTrue(text_s in error.output)
> +            self.assertTrue(text_e not in error.output)

Don't you need an else to fail if the exception doesn't happen, like testOutputTimeout does ?

@@ +94,5 @@
> +        cmd = ['bash', '-c', 'echo "%s" && echo "%s" 1>&2 && sleep 1 && true' % (text_s, text_e)]
> +        try:
> +            get_output(cmd, include_stderr=True, timeout=0.5)
> +        except subprocess.CalledProcessError as error:
> +            print error

Nit, leftover debugging ?

@@ +96,5 @@
> +            get_output(cmd, include_stderr=True, timeout=0.5)
> +        except subprocess.CalledProcessError as error:
> +            print error
> +            self.assertTrue(text_s in error.output)
> +            self.assertTrue(text_e in error.output)

Missing else again.

::: lib/python/util/commands.py
@@ -107,3 @@
>              # reset last_check to avoid spamming callback
>              last_check = now
> -            elapsed = now - start_time

I wasn't very clear about this, but meant to put it just after |now = time.time()|.

@@ +153,5 @@
>      is set, stderr will be included in the output, otherwise it will be sent to
>      the caller's stderr stream.
>  
>      Warning that you shouldn't use this function to capture a large amount of
> +    output (> a few thousand bytes), it will deadlock.

Is this still true ? You're using Popen and communicate as https://docs.python.org/2/library/subprocess.html recommends.

@@ +197,5 @@
> +            else:
> +                raise subprocess.CalledProcessError(proc.returncode, cmd, output=output)
> +
> +        now = time.time()
> +        if now - start_time > timeout:

As above, please calculate elapsed between the now assignment and if, then adjust later code.

@@ +199,5 @@
> +
> +        now = time.time()
> +        if now - start_time > timeout:
> +            elapsed = now - start_time
> +            log.info("process, is taking too long: %ss (timeout %ss).Terminating" %

Nit, a space before Terminating

::: lib/python/util/hg.py
@@ +161,4 @@
>  
>  
>  def clone(repo, dest, branch=None, revision=None, update_dest=True,
> +          clone_by_rev=False, mirrors=None, bundles=None, timeout=7200, ):

Nit, remove trailing comma+space.

@@ +251,4 @@
>      exc = None
>      for _ in retrier(attempts=RETRY_ATTEMPTS):
>          try:
> +            get_hg_output(cmd=cmd, include_stderr=True, timeout=timeout, )

Nit, remove trailing comma+space.

Comment 21

[Massimo Gervasini [:massimo]]

Attachment: [tools] Bug 1088590 - add a timeout in get_output and in run_cmd_periodic_poll; add --traceback option to hg operations IV.patch

Updated as requested in comment 20

* added a new test to check get_output() does not hangs on huge output
* set hg unbundle timeout to 2700 seconds

Comment 22

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8567161 [details] [diff] [review]
[tools] Bug 1088590 - add a timeout in get_output and in run_cmd_periodic_poll; add --traceback option to hg operations IV.patch

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

r+, thanks for your patience!

(In reply to Nick Thomas [:nthomas] from comment #20)
> In test_util_commands.py I'd really like to see three classes: TestRunCmd,
> TestRunCmdPeriodicPoll, and TestGetOutput; then only test the matching
> function in each. They're a bit mixed up at the moment.
> 
> It would be really good to get a list of timeouts on each of the hg
> operations, as not all use get_hg_output() and some call run_cmd directly.
> Please update the bug summary with that in mind, since I don't believe
> push() will be changed here (that's fine!).

Would still love to see these happen if you have time (before landing, and bug comment respectively).

Comment 23

[Massimo Gervasini [:massimo]]

Attachment: Bug 1088590 - add a timeout in get_output and in run_cmd_periodic_poll; add --traceback option to hg operations V.patch

Thanks for the review, Nick.

changes in this patch:
* now we have TestRunCmd TestRunCmdPeriodicPoll and TestGetOutput
* set unbundle timeout to 1h

Some timeouts introduced by this patch:

command  |  timeout [s]
---------+-------------
clone    |   3600
pull     |   1800
push     |   ----
unbundle |   3600
update   |   3600

Comment 24

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8571386 [details] [diff] [review]
Bug 1088590 - add a timeout in get_output and in run_cmd_periodic_poll; add --traceback option to hg operations V.patch

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

Please move these tests from TestRunCmd to TestGetOutput: testOutput, testStdErr, testNoStdErr, testBadOutput. They all use get_output().

Comment 25

[Massimo Gervasini [:massimo]]

Comment on attachment 8571386 [details] [diff] [review]
Bug 1088590 - add a timeout in get_output and in run_cmd_periodic_poll; add --traceback option to hg operations V.patch

landed with changes requested in comment 24

https://hg.mozilla.org/build/tools/rev/056a5dcb6e72

Comment 26

[Jordan Lund (:jlund)]

Comment on attachment 8571386 [details] [diff] [review]
Bug 1088590 - add a timeout in get_output and in run_cmd_periodic_poll; add --traceback option to hg operations V.patch

backed out
remote:   https://hg.mozilla.org/build/tools/rev/899065ceb7a4
remote:   https://hg.mozilla.org/build/tools/rev/d6d28635bcc6

suspected as the culprit in:
https://treeherder.mozilla.org/logviewer.html#?job_id=1106855&repo=mozilla-central

snippet:
command: START
command: hg pull --traceback -r 21f00dd7bda6 https://hg.mozilla.org/build/mozharness
command: cwd: /builds/hg-shared/build/mozharness
command: env: {'HGPLAIN': '1'}
Traceback (most recent call last):
  File "/tools/checkouts/build-tools/buildfarm/utils/hgtool.py", line 94, in <module>
    autoPurge=options.auto_purge)
  File "/tools/checkouts/build-tools/buildfarm/utils/../../lib/python/util/hg.py", line 503, in mercurial
    mirrors=mirrors, bundles=bundles, autoPurge=False)
  File "/tools/checkouts/build-tools/buildfarm/utils/../../lib/python/util/hg.py", line 463, in mercurial
    mirrors=mirrors)
  File "/tools/checkouts/build-tools/buildfarm/utils/../../lib/python/util/hg.py", line 329, in pull
    get_hg_output(cmd=cmd, cwd=dest, include_stderr=True)
  File "/tools/checkouts/build-tools/buildfarm/utils/../../lib/python/util/hg.py", line 86, in get_hg_output
    return get_output(['hg'] + cmd, timeout=timeout, env=env, **kwargs)
  File "/tools/checkouts/build-tools/buildfarm/utils/../../lib/python/util/commands.py", line 204, in get_output
    raise subprocess.CalledProcessError(proc.returncode, cmd, output=output)
TypeError: __init__() got an unexpected keyword argument 'output'
program finished with exit code 1
elapsedTime=16.752785
========= Finished 'python /tools/checkouts/build-tools/buildfarm/utils/hgtool.py ...' failed

Comment 27

[Nick Thomas [:nthomas] (UTC+12)]

Apparently the output argument was added subprocess.CalledProcessError between Python 2.6 and 2.7, not that https://docs.python.org/2/library/subprocess.html#subprocess.CalledProcessError will tell you so.

Comment 28

[Massimo Gervasini [:massimo]]

Attachment: [tools] Bug 1088590 - add a timeout in get_output and in run_cmd_periodic_poll; add --traceback option to hg operations VII.patch

Fixes for python 2.6

Changes from last patch:

* changed subprocess.CalledProcessError(proc.returncode, cmd, option=option) calls to:

error = subprocess.CalledProcessError(proc.returncode, cmd)
error.output = output
raise error

* using: nose.plugins.skip.SkipTest instead of @unittest.skipIf, (skipIf is not available in python 2.6)

Comment 29

[Rail Aliiev [:rail]]

Comment on attachment 8572590 [details] [diff] [review]
[tools] Bug 1088590 - add a timeout in get_output and in run_cmd_periodic_poll; add --traceback option to hg operations VII.patch

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

I looked at the interdiff with the previous patch and it looks good to me.

Comment 30

[Massimo Gervasini [:massimo]]

Comment on attachment 8572590 [details] [diff] [review]
[tools] Bug 1088590 - add a timeout in get_output and in run_cmd_periodic_poll; add --traceback option to hg operations VII.patch

Thanks rail!


https://hg.mozilla.org/build/tools/rev/46721e25c5fc

Comment 31

[Pete Moore [:pmoore][:pete]]

Attachment: bug1088590_tools_spelling_corrections.patch

Minor spelling corrections

Comment 32

[Massimo Gervasini [:massimo]]

Comment on attachment 8573207 [details] [diff] [review]
bug1088590_tools_spelling_corrections.patch

Thanks Pete!

Comment 33

[Pete Moore [:pmoore][:pete]]

Comment on attachment 8573207 [details] [diff] [review]
bug1088590_tools_spelling_corrections.patch

https://hg.mozilla.org/build/tools/rev/f80d81584851

Comment 34

[Pete Moore [:pmoore][:pete]]

I'm assuming this is ok, since there has been no backout or reports of problems etc. Therefore closing.