Closed
Bug 848227
Opened 12 years ago
Closed 12 years ago
make mozharness retries generic
Categories
(Release Engineering :: Applications: MozharnessCore, defect, P3)
Release Engineering
Applications: MozharnessCore
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mozilla, Assigned: mozilla)
References
Details
(Whiteboard: [mozharness])
Attachments
(1 file)
68.85 KB,
patch
|
rail
:
review+
mozilla
:
checked-in+
|
Details | Diff | Splinter Review |
Currently we have a lot of retry logic in various places in mozharness, but they each have their own logic to do those retries.
We should make this generic.
Assignee | ||
Comment 1•12 years ago
|
||
Run a python call (that can be a self.run_command() or other), up to max retries.
A check for "good" (status 0 could be a default check, but parsing output [whitelist/blacklist], expecting a value or something in a set of values, etc.).
A time.sleep() between retries that can be set to 0.
A max_retries as well as a final step.
The final step could be used to set buildbot RETRY status if applicable.
Assignee | ||
Comment 2•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → aki
Assignee | ||
Comment 3•12 years ago
|
||
Ok, this is another big one, sorry about that.
My work is here: https://github.com/escapewindow/mozharness/compare/generic-retry
I can break this patch up if that helps.
This patch:
* merges OSMixin and ShellMixin. They had interdependencies, and I don't think we ever inherited one without the other. Hello ScriptMixin.
* creates a generic ScriptMixin.retry() method that's based off of build/tools' util.retry.retry().
* ports various methods to use retry() instead of homegrown versions.
** This may result in different error messages, different num_retries and sleep times, etc.
** download_file() will now do an nslookup on every URLError, not just the final one.
** vcs_checkout() will rmtree(dest) on every failure, even the final attempt
* also gets a few more files pep8-whitespace-compliant.
* updates setup.py to add test dependencies. (maybe I should add mercurial==1.7.3 until bug 828029 is fixed?)
* adds the retry tests from build/tools into test_base_script.py
This is not only a large patch, but it touches a lot of critical infrastructure methods. I'd like to land on default, and get a green run on all Cedar platforms, before merging to production.
Attachment #722993 -
Flags: review?(rail)
Comment 4•12 years ago
|
||
Comment on attachment 722993 [details] [diff] [review]
generic retry()
Review of attachment 722993 [details] [diff] [review]:
-----------------------------------------------------------------
::: mozharness/base/script.py
@@ +154,5 @@
> + def _download_file(self, url, file_name):
> + """ Helper script for download_file()
> + """
> + try:
> + f = urllib2.urlopen(url)
Not a blocker, probably could be done as a separate bug/patch. It would be great to add and pass timeout to urlopen to avoid buildbot killing the process. Even None is OK.
Attachment #722993 -
Flags: review?(rail) → review+
Assignee | ||
Comment 5•12 years ago
|
||
(In reply to Rail Aliiev [:rail] from comment #4)
> Comment on attachment 722993 [details] [diff] [review]
> generic retry()
>
> Review of attachment 722993 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: mozharness/base/script.py
> @@ +154,5 @@
> > + def _download_file(self, url, file_name):
> > + """ Helper script for download_file()
> > + """
> > + try:
> > + f = urllib2.urlopen(url)
>
> Not a blocker, probably could be done as a separate bug/patch. It would be
> great to add and pass timeout to urlopen to avoid buildbot killing the
> process. Even None is OK.
Makes sense; that may help the downloads that hit bug 840305.
Looks like we need to make sure there are no python 2.5 test machines first, though.
Assignee | ||
Comment 6•12 years ago
|
||
Comment on attachment 722993 [details] [diff] [review]
generic retry()
http://hg.mozilla.org/build/mozharness/rev/0815e7ebbd57
Merging m-c to cedar to make sure nothing is egregiously broken before merging to production.
Attachment #722993 -
Flags: checked-in+
Assignee | ||
Comment 7•12 years ago
|
||
Gaia UI tests look broken, but I *think* those are mozpool-client retries.
https://tbpl.mozilla.org/php/getParsedLog.php?id=20537486&tree=Cedar&full=1#error0
Assignee | ||
Comment 8•12 years ago
|
||
Merged to production.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
Updated•11 years ago
|
Component: General Automation → Mozharness
You need to log in
before you can comment on or make changes to this bug.
Description
•