Closed Bug 1118774 Opened 5 years ago Closed 5 years ago

Add retries to build/upload.py

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(b2g-v2.0 fixed, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.1S fixed)

RESOLVED FIXED
mozilla37
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.0M --- fixed
b2g-v2.1 --- fixed
b2g-v2.1S --- fixed

People

(Reporter: mshal, Assigned: mshal)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Before the mach/mozharness integration, buildbot was responsible for retrying 'make upload'. Now that buildbot is no longer calling 'make upload' directly, we need to add retry logic to upload.py in order to work around connection timeouts to stage.m.o
Blocks: 1118782
I figure this might be useful, rather than writing yet another exponential backoff loop.
Attachment #8545490 - Flags: review?(gps)
I tested this a bit locally by replacing ssh/scp with a shell script that would randomly do an exit 1.

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ae1a992fcb39
Attachment #8545491 - Flags: review?(gps)
Attachment #8545490 - Flags: review?(gps) → review+
Comment on attachment 8545491 [details] [diff] [review]
0002-Bug-1118774-Add-retries-to-ssh-scp-in-upload.py.patch

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

First, how are you producing your patches? The diff summary in the commit message is totally unnecessary and adds little value: your version control system is much better at calculating these things.

Second, can we switch to the context manager redo foo? I think that would look cleaner.
(In reply to Gregory Szorc [:gps] from comment #3)
> Comment on attachment 8545491 [details] [diff] [review]
> 0002-Bug-1118774-Add-retries-to-ssh-scp-in-upload.py.patch
> 
> Review of attachment 8545491 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> First, how are you producing your patches? The diff summary in the commit
> message is totally unnecessary and adds little value: your version control
> system is much better at calculating these things.

That's git format-patch formatting, and it's a useful summary for reviewers, which is why reviewboard shows it too. Note that everything after -- is not part of the commit message.
(In reply to Gregory Szorc [:gps] from comment #3)
> First, how are you producing your patches? The diff summary in the commit
> message is totally unnecessary and adds little value: your version control
> system is much better at calculating these things.

Yeah, it's from git format-patch. The summary gets removed when I import it into hg. I'm aware I need to learn the new reviewing tools & git integration at some point, but at the moment goals are the priority (and this bug is already a distraction from that, since it's from last quarter's goals).

> Second, can we switch to the context manager redo foo? I think that would
> look cleaner.

Looks cleaner to me - patch on the way...
Attachment #8545491 - Attachment is obsolete: true
Attachment #8545491 - Flags: review?(gps)
Attachment #8546716 - Flags: review?(gps)
Attachment #8546716 - Flags: review?(gps) → review+
https://hg.mozilla.org/mozilla-central/rev/a1654fa1847d
https://hg.mozilla.org/mozilla-central/rev/fff1eae2346f
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.