Skip to content

Commit

Permalink
Add an expiry to not create try pushes for phabricator revisions that…
Browse files Browse the repository at this point in the history
… are too old
  • Loading branch information
mlbonhomme committed Oct 22, 2024
1 parent a41fad3 commit e286e1c
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 1 deletion.
20 changes: 19 additions & 1 deletion libmozevent/mercurial.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import os
import tempfile
import time
from datetime import datetime
from datetime import datetime, timedelta

import hglib
import requests
Expand All @@ -34,6 +34,9 @@
MAX_PUSH_RETRIES = 4
# Wait successive exponential delays: 6sec, 36sec, 3.6min, 21.6min
PUSH_RETRY_EXPONENTIAL_DELAY = 6
# Time after which a Phabricator revision should be considered as expired and the
# try push should no longer be retried
DIFF_EXPIRY = timedelta(hours=1)


class TryMode(enum.Enum):
Expand Down Expand Up @@ -448,6 +451,21 @@ async def handle_build(self, repository, build):
build,
{"message": error_log, "duration": time.time() - start},
)
if (
build.diff.get("fields")
and build.diff["fields"].get("dateCreated")
and (
datetime.now()
- datetime.fromtimestamp(build.diff["fields"]["dateCreated"])
> DIFF_EXPIRY
)
):
error_log = "This build is too old to push to try repository"
return (
"fail:mercurial",
build,
{"message": error_log, "duration": time.time() - start},
)
elif build.retries:
logger.warning(
"Trying to apply build's diff after a remote push error "
Expand Down
62 changes: 62 additions & 0 deletions tests/test_mercurial.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import json
import os.path
from unittest.mock import MagicMock
from datetime import timedelta

import hglib
import pytest
Expand Down Expand Up @@ -917,3 +918,64 @@ def test_get_base_identifier(mock_mc):
assert (
mock_mc.get_base_identifier(stack) == "tip"
), "`tip` commit should be used when `use_latest_revision` is `True`."


@responses.activate
@pytest.mark.asyncio
async def test_push_failure_diff_expiry(PhabricatorMock, mock_mc):
diff = {
"revisionPHID": "PHID-DREV-badutf8",
"baseRevision": "missing",
"phid": "PHID-DIFF-badutf8",
"id": 555,
"fields": {"dateCreated": 1510251135},
}
build = MockBuild(4444, "PHID-REPO-mc", 5555, "PHID-build-badutf8", diff)
with PhabricatorMock as phab:
phab.load_patches_stack(build)

bus = MessageBus()
bus.add_queue("phabricator")

from libmozevent import mercurial

mercurial.DIFF_EXPIRY = timedelta(hours=24)
mercurial.TRY_STATUS_URL = "http://test.status/try"

sleep_history = []

class AsyncioMock(object):
async def sleep(self, value):
nonlocal sleep_history
sleep_history.append(value)

mercurial.asyncio = AsyncioMock()

responses.get(
"http://test.status/try", status=200, json={"result": {"status": "open"}}
)

repository_mock = MagicMock(spec=Repository)

worker = MercurialWorker(
"mercurial", "phabricator", repositories={"PHID-REPO-mc": repository_mock}
)
worker.register(bus)

await bus.send("mercurial", build)
assert bus.queues["mercurial"].qsize() == 1
task = asyncio.create_task(worker.run())

# Check the treeherder link was queued
mode, out_build, details = await bus.receive("phabricator")
task.cancel()

assert build.retries == 0

assert mode == "fail:mercurial"
assert out_build == build
assert details["duration"] > 0
assert details["message"] == "This build is too old to push to try repository"

# no call sent to TRY_STATUS_URL
assert len(responses.calls) == 0

0 comments on commit e286e1c

Please sign in to comment.