From ae9676d1f1006a7a1c9d7dc11e43b996eb93c9e1 Mon Sep 17 00:00:00 2001 From: mlbonhomme Date: Wed, 23 Oct 2024 14:53:38 +0200 Subject: [PATCH 1/5] Bump mypy in CI --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a17de9f..34cc64c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -34,7 +34,7 @@ repos: hooks: - id: yesqa - repo: https://github.com/pre-commit/mirrors-mypy - rev: v0.950 + rev: v1.12.1 hooks: - id: mypy additional_dependencies: From bb26e2934261cc8aa650ca3751c0353e370e4ce1 Mon Sep 17 00:00:00 2001 From: mlbonhomme Date: Tue, 22 Oct 2024 11:48:02 +0200 Subject: [PATCH 2/5] Update Phabricator mock to have user agent set --- tests/conftest.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index 39eb9a5..6b0916d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,8 +5,10 @@ import asyncio import collections +from configparser import ConfigParser import json import os.path +import tempfile import urllib.parse from contextlib import contextmanager from datetime import datetime, timedelta @@ -345,6 +347,17 @@ def mock_taskcluster(): class MockBuild(PhabricatorBuild): def __init__(self, diff_id, repo_phid, revision_id, target_phid, diff): + config_file = tempfile.NamedTemporaryFile() + with open(config_file.name, "w") as f: + custom_conf = ConfigParser() + custom_conf.add_section("User-Agent") + custom_conf.set("User-Agent", "name", "libmozdata") + custom_conf.write(f) + f.seek(0) + from libmozdata import config + + config.set_config(config.ConfigIni(config_file.name)) + self.diff_id = diff_id self.repo_phid = repo_phid self.revision_id = revision_id From 365c78899b5d948e61efb484eb6b45f9c5558bc3 Mon Sep 17 00:00:00 2001 From: mlbonhomme Date: Tue, 22 Oct 2024 11:40:52 +0200 Subject: [PATCH 3/5] Add an expiry to not create try pushes for phabricator revisions that are too old --- libmozevent/mercurial.py | 20 ++++++++++++- tests/test_mercurial.py | 62 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/libmozevent/mercurial.py b/libmozevent/mercurial.py index 4521908..886f28a 100644 --- a/libmozevent/mercurial.py +++ b/libmozevent/mercurial.py @@ -11,7 +11,7 @@ import os import tempfile import time -from datetime import datetime +from datetime import datetime, timedelta import hglib import requests @@ -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): @@ -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 " diff --git a/tests/test_mercurial.py b/tests/test_mercurial.py index dd74dfd..d89d501 100644 --- a/tests/test_mercurial.py +++ b/tests/test_mercurial.py @@ -3,6 +3,7 @@ import json import os.path from unittest.mock import MagicMock +from datetime import timedelta import hglib import pytest @@ -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 From 8a907e72ee98e2fb1751ab99116db560e483759a Mon Sep 17 00:00:00 2001 From: mlbonhomme Date: Tue, 22 Oct 2024 13:14:53 +0200 Subject: [PATCH 4/5] make diff expiry a parameter of the MercurialWorker class + default to 24h --- libmozevent/mercurial.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/libmozevent/mercurial.py b/libmozevent/mercurial.py index 886f28a..84a0302 100644 --- a/libmozevent/mercurial.py +++ b/libmozevent/mercurial.py @@ -34,9 +34,6 @@ 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): @@ -338,12 +335,20 @@ class MercurialWorker(object): ] ] - def __init__(self, queue_name, queue_phabricator, repositories, skippable_files=[]): + def __init__( + self, + queue_name, + queue_phabricator, + repositories, + diff_expiry=timedelta(hours=24), + skippable_files=[], + ): assert all(map(lambda r: isinstance(r, Repository), repositories.values())) self.queue_name = queue_name self.queue_phabricator = queue_phabricator self.repositories = repositories self.skippable_files = skippable_files + self.diff_expiry = diff_expiry def register(self, bus): self.bus = bus @@ -457,7 +462,7 @@ async def handle_build(self, repository, build): and ( datetime.now() - datetime.fromtimestamp(build.diff["fields"]["dateCreated"]) - > DIFF_EXPIRY + > self.diff_expiry ) ): error_log = "This build is too old to push to try repository" From 78e0fb882fd9ad1c764e7e85bb94f0a20719acbd Mon Sep 17 00:00:00 2001 From: mlbonhomme Date: Tue, 22 Oct 2024 14:31:46 +0200 Subject: [PATCH 5/5] update test --- tests/test_mercurial.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_mercurial.py b/tests/test_mercurial.py index d89d501..25a48e3 100644 --- a/tests/test_mercurial.py +++ b/tests/test_mercurial.py @@ -3,7 +3,6 @@ import json import os.path from unittest.mock import MagicMock -from datetime import timedelta import hglib import pytest @@ -928,6 +927,7 @@ async def test_push_failure_diff_expiry(PhabricatorMock, mock_mc): "baseRevision": "missing", "phid": "PHID-DIFF-badutf8", "id": 555, + # a date in 2017 "fields": {"dateCreated": 1510251135}, } build = MockBuild(4444, "PHID-REPO-mc", 5555, "PHID-build-badutf8", diff) @@ -939,7 +939,6 @@ async def test_push_failure_diff_expiry(PhabricatorMock, mock_mc): from libmozevent import mercurial - mercurial.DIFF_EXPIRY = timedelta(hours=24) mercurial.TRY_STATUS_URL = "http://test.status/try" sleep_history = []