Autolander created two attachments in a bug for the same PR

RESOLVED FIXED

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: emorley, Assigned: emorley)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
In bug 1237237, autolander created two identical attachments for a newly opened PR within a few seconds of each other.
(Assignee)

Comment 1

3 years ago
The same thing happened in bug 1240523.
(Assignee)

Comment 2

3 years ago
This happened again in bug 1240385.

Looking at the deliveries reported by the GitHub hook, it's showing two: one for action "opened" (096c6c80-c13b-11e5-951d-75e92b173dcb) and one for "assigned" ( 096c6c80-c13b-11e5-9688-32cb72afea14):
https://github.com/mozilla/treeherder/settings/hooks/6742001

When I create PRs I tend to:
* Visit the open PR page (SourceTree automatically opens this for me)
* Compose the description
* Press "assign to me" whilst still in draft mode
* Press open PR

It then appears that in this case GitHub fires off two notification payloads within short succession - one for each of opened and assigned.

I'm presuming there's a race condition such that if these two events occur too closely together, autolander isn't de-duping them (unlike if I use the "assign" feature as a separate action to opening the PR).
Interesting. So I believe autolander will check the bug to see if there is an attachment to the pull request already, but nothing to see if we are currently in the process of attaching one. I guess we should implement either some kind of "locking" on the bug when we are processing it.

I do believe we currently store bug state in azure table storage, a quick fix might be to check the azure table record before attaching the PR request, possibly storing additional information in azure if needed. Right now it looks like the azure write happens too late though: https://github.com/mozilla/autolander/blob/master/routes/pull_request.js#L43-L44
(Assignee)

Comment 4

3 years ago
The other option I guess would be to just filter the hook deliveries, so the action 'assigned' one is excluded.

Aside from the PR being opened, the only time the hook deliveries are relevant are when commits are added/updated, or the PR title changes.
(Assignee)

Comment 5

3 years ago
Yeah looks like "opened", "edited" or "synchronize" are the events of interest, with "assigned", "unassigned", "labeled", "unlabeled", "closed" and "reopened" being noise:
https://developer.github.com/v3/activity/events/types/#pullrequestevent
Comment on attachment 8756273 [details] [review]
[autolander] edmorley:filter-event-actions > mozilla:master

Drive-by R+. Thanks :)
Attachment #8756273 - Flags: review+
(Assignee)

Comment 7

3 years ago
https://github.com/mozilla/autolander/commit/5fbfcc0c9ae247c858df0405e14f0edc42636188
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Comment 8

3 years ago
Working :-)

May 25 09:12:52 autolander app/worker.1: Wed, 25 May 2016 16:12:51 GMT autolander:route:pull_request Skipping pull request event with action assigned
Comment on attachment 8756273 [details] [review]
[autolander] edmorley:filter-event-actions > mozilla:master

lgtm!
Attachment #8756273 - Flags: review?(pmoore) → review+
Created attachment 8756273 [details] [review]
[autolander] edmorley:filter-event-actions > mozilla:master
(Assignee)

Updated

3 years ago
Assignee: nobody → emorley
Status: NEW → ASSIGNED
(Assignee)

Comment 11

3 years ago
Comment on attachment 8756273 [details] [review]
[autolander] edmorley:filter-event-actions > mozilla:master

There doesn't seem to be an owner for Autolander any more, so would you mind taking a look? (Saw you in the contributors list)
Attachment #8756273 - Flags: review?(pmoore)
You need to log in before you can comment on or make changes to this bug.