Make mercurial closure hooks fail closed once treestatus.mozilla.org proven reliable

RESOLVED FIXED

Status

Developer Services
Mercurial: hg.mozilla.org
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: emorley, Assigned: emorley)

Tracking

({sheriffing-P1})

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
(Chris AtLee [:catlee] from bug 754132 comment #4)
> > If we turn treestatus into a production service, we should consider making
> > this fail closed. treestatus seems simple enough that we could make that
> > work.
> 
> Let's deploy with fail open first and see how it handles things.

Once bug 754132 has been resolved and we're happy with treestatus.mozilla.org, we should switch to failing closed rather than open - especially given that people can always override the hook using CLOSED TREE anyway, were there to be issues with treestatus.

This will also alleviate the "we want to keep ancient trees closed, but don't want them cluttering up the treestatus UI" problem of bug 758882 comment 0.
Depends on: 759170
(Assignee)

Updated

5 years ago
Summary: Make mercurial closure hooks fail closed (not open), once treestatus.mozilla.org proven reliable → Make mercurial closure hooks fail closed once treestatus.mozilla.org proven reliable
(Assignee)

Updated

5 years ago
Whiteboard: [sheriff-want]
(Assignee)

Comment 1

4 years ago
We should do this.

People managed to land on aurora today due to treestatus HTTP 500 errors :-(
Assignee: nobody → edmorley.bz
Status: NEW → ASSIGNED
Keywords: sheriffing-P1
Whiteboard: [sheriff-want]
(Assignee)

Comment 2

4 years ago
Created attachment 692978 [details] [diff] [review]
Patch v1
Attachment #692978 - Flags: review?(ted)
Comment on attachment 692978 [details] [diff] [review]
Patch v1

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

::: mozhghooks/treeclosure.py
@@ +52,5 @@
>      except (ValueError, IOError), (err):
>          # fail open. no sense making hg unavailable
>          # if treestatus is down
>          print "Error: %s" % err, url
> +        return 1

Please make the error message more verbose while you're here, so that people have an understanding of why their push is failing. Also, this print statement is written oddly. It's actually doing print ("Error: %s" % err), url. Maybe tweak that to be less confusing while you're at it?
Attachment #692978 - Flags: review?(ted) → review+
(Assignee)

Comment 4

4 years ago
Created attachment 693131 [details] [diff] [review]
Patch v2

I know r+ already, but a few more changes, just wanted to make sure you're ok with making the except a bit more complex.
Attachment #692978 - Attachment is obsolete: true
Attachment #693131 - Flags: review?(ted)
Attachment #693131 - Flags: review?(ted) → review+
(Assignee)

Comment 5

4 years ago
Cheers :-)

https://hg.mozilla.org/hgcustom/hghooks/rev/4a9228a54a7d
(Assignee)

Updated

4 years ago
Depends on: 822648
(Assignee)

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering

Comment 6

3 years ago
https://hg.mozilla.org/hgcustom/version-control-tools/rev/2bd6696e11b5
Product: Release Engineering → Developer Services
You need to log in before you can comment on or make changes to this bug.