Closed Bug 1220231 Opened 9 years ago Closed 9 years ago

Prevent triggering autoland while autoland is in progress

Categories

(MozReview Graveyard :: General, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mcote, Assigned: mdoglio)

References

Details

Attachments

(3 files)

If someone triggers autoland for a commit series, we need to be sure no one else can trigger one while it's in progress.  This state (autoland in progress) needs to be displayed in the UI as well.
Status: NEW → ASSIGNED
mozreview: prevent concurrent autoland requests (bug 1220231); r?dminor
Attachment #8685476 - Flags: review?(dminor)
Comment on attachment 8685476 [details]
MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r=glob

https://reviewboard.mozilla.org/r/24797/#review22335

This looks good to me, but I'm not familiar enough with django's cache behaviour to feel comfortable giving a "ship-it". I'll flag smacleod for review in bugzilla.

::: pylib/mozreview/mozreview/autoland/resources.py:167
(Diff revision 1)
>              # Rather than hard coding the destination it would make sense
>              # to extract it from metadata about the repository. That will have
>              # to wait until we fix Bug 1168486.

This comment is no longer relevant, please remove it while you're here.
Attachment #8685476 - Flags: review?(dminor)
Attachment #8685476 - Flags: review?(smacleod)
Comment on attachment 8685476 [details]
MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r=glob

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24797/diff/1-2/
Attachment #8685476 - Attachment description: MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r?dminor → MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r?smacleod
mozreview: remove outdated comment; r?dminor
Attachment #8685944 - Flags: review?(dminor)
Attachment #8685944 - Flags: review?(dminor) → review+
Comment on attachment 8685944 [details]
MozReview Request: mozreview: remove outdated comment; r=dminor

https://reviewboard.mozilla.org/r/24925/#review22447

thanks!
:mdoglio, one way of testing this would be to use the treestatus container to close the trees so that Autoland can't land anything while the test is running.

There are mach commands for this, check out autoland/tests/test-autoland-closed-tree.t, but I guess you'll have to adapt the mach stuff for selenium.
Comment on attachment 8685476 [details]
MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r=glob

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24797/diff/2-3/
Comment on attachment 8685944 [details]
MozReview Request: mozreview: remove outdated comment; r=dminor

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24925/diff/1-2/
mozreview: add test for concurrent autoland requests (bug 1220231); r?dminor
Attachment #8688476 - Flags: review?(dminor)
Comment on attachment 8688476 [details]
MozReview Request: mozreview: add test for concurrent autoland requests (bug 1220231); r=dminor

https://reviewboard.mozilla.org/r/25329/#review22849

We could also put the treestatus functions in the superclass in unittest.py but I'm ok with not doing that until we have a second test which uses them.
Attachment #8688476 - Flags: review?(dminor) → review+
Comment on attachment 8685476 [details]
MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r=glob

https://reviewboard.mozilla.org/r/24797/#review22843

looks good, comments can be addressed on commit.

::: pylib/mozreview/mozreview/autoland/resources.py:42
(Diff revision 3)
> +    return cache.add(lock_id, "true")

as per irc, we should explictly set the cache entry to expire after 24 hours

::: pylib/mozreview/mozreview/autoland/resources.py:51
(Diff revision 3)
> +def get_autoland_lock(review_request_id, repository_url, revision):

nit: i think this would be clearer if it were named "get_autoland_lock_id"
Attachment #8685476 - Flags: review+
https://reviewboard.mozilla.org/r/24797/#review22857

::: pylib/mozreview/mozreview/autoland/resources.py:42
(Diff revision 3)
> +    return cache.add(lock_id, "true")

just marking this as an issue

::: pylib/mozreview/mozreview/autoland/resources.py:51
(Diff revision 3)
> +def get_autoland_lock(review_request_id, repository_url, revision):

just marking this as an issue
Comment on attachment 8685476 [details]
MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r=glob

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24797/diff/3-4/
Comment on attachment 8685944 [details]
MozReview Request: mozreview: remove outdated comment; r=dminor

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24925/diff/2-3/
Comment on attachment 8688476 [details]
MozReview Request: mozreview: add test for concurrent autoland requests (bug 1220231); r=dminor

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25329/diff/1-2/
Comment on attachment 8685476 [details]
MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r=glob

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24797/diff/4-5/
Attachment #8685476 - Attachment description: MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r?smacleod → MozReview Request: mozreview: prevent concurrent autoland requests (bug 1220231); r=glob
Attachment #8685476 - Flags: review?(smacleod)
Comment on attachment 8685944 [details]
MozReview Request: mozreview: remove outdated comment; r=dminor

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24925/diff/3-4/
Attachment #8685944 - Attachment description: MozReview Request: mozreview: remove outdated comment; r?dminor → MozReview Request: mozreview: remove outdated comment; r=dminor
Attachment #8688476 - Attachment description: MozReview Request: mozreview: add test for concurrent autoland requests (bug 1220231); r?dminor → MozReview Request: mozreview: add test for concurrent autoland requests (bug 1220231); r=dminor
Comment on attachment 8688476 [details]
MozReview Request: mozreview: add test for concurrent autoland requests (bug 1220231); r=dminor

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25329/diff/2-3/
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Product: Developer Services → MozReview
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: