Skip to content

Commit

Permalink
feat(queue): allow to configure checks timeout (Mergifyio#3761)
Browse files Browse the repository at this point in the history
This inject a new condition that can unqueue pull request automatically.

Since this condition is a timedelta, the pull request is automatically
refresh when the timeout is reached.

Fixes MRGFY-640

Change-Id: I175b07c8cb0a4cb582b75aae6c7bd27dfcae4636
  • Loading branch information
sileht committed Dec 15, 2021
1 parent 56684c6 commit 7e5f214
Show file tree
Hide file tree
Showing 25 changed files with 2,433 additions and 7 deletions.
6 changes: 6 additions & 0 deletions docs/source/actions/queue.rst
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,12 @@ A ``queue_rules`` takes the following parameter:
between 1 and 20.
See :ref:`speculative checks`.

* - ``checks_timeout``
- :ref:`Duration <duration>`
-
- The amount of time Mergify waits for pending checks to return before unqueueing pull requests.
This cannot be less than 60 seconds.

.. note::

|premium plan tag|
Expand Down
3 changes: 3 additions & 0 deletions docs/source/conditions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@ Here's the list of pull request attribute that can be used in conditions:
* - ``queued-at``
- :ref:`Timestamp <iso timestamp>` or :ref:`Relative timestamp <relative timestamp>`
- The time the pull request was queued at for merge.
* - ``queue-merge-started-at``
- :ref:`Timestamp <iso timestamp>` or :ref:`Relative timestamp <relative timestamp>`
- The time the pull request mergeability checks has started at.
* - ``commits-behind``
- list of string
- The list of commits sha between the head of the base branch and the base
Expand Down
16 changes: 16 additions & 0 deletions docs/source/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,22 @@ Examples
This pull request looks stale. Feel free to reopen it if you think it's a mistake.
.. _duration:

Duration
~~~~~~~~

Duration can be expressed as ``quantity unit [quantity unit...]`` where
quantity is a number (possibly signed); unit is microsecond, millisecond,
second, minute, hour, day, week, or abbreviations or plurals of these units;

.. code-block::
1 day 15 hours 6 minutes 42 seconds
1 d 15 h 6 m 42 s
Disabling Rules
~~~~~~~~~~~~~~~

Expand Down
1 change: 1 addition & 0 deletions docs/source/spelling_wordlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ templating
wasn
th
queueing
unqueueing
workflow
workflows
checkbox
Expand Down
1 change: 1 addition & 0 deletions mergify_engine/actions/merge.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ def validate_config(self, mergify_config: "rules.MergifyConfig") -> None:
"batch_size": 1,
"allow_inplace_speculative_checks": True,
"allow_speculative_checks_interruption": True,
"checks_timeout": None,
}
)

Expand Down
10 changes: 9 additions & 1 deletion mergify_engine/actions/merge_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from mergify_engine import branch_updater
from mergify_engine import check_api
from mergify_engine import config
from mergify_engine import constants
from mergify_engine import context
from mergify_engine import github_types
from mergify_engine import json as mergify_json
Expand Down Expand Up @@ -167,6 +168,13 @@ async def get_rule_checks_status(
if rule.conditions.match:
return check_api.Conclusion.SUCCESS

for condition in rule.conditions.walk():
if (
condition.label == constants.CHECKS_TIMEOUT_CONDITION_LABEL
and not condition.match
):
return check_api.Conclusion.FAILURE

conditions_without_checks = rule.conditions.copy()
for condition_without_check in conditions_without_checks.walk():
attr = condition_without_check.get_attribute_name()
Expand Down Expand Up @@ -345,7 +353,7 @@ async def _run(
return check_api.Result(
check_api.Conclusion.CANCELLED,
"The pull request has been removed from the queue",
"The queue conditions cannot be satisfied due to failing checks. "
"The queue conditions cannot be satisfied due to failing checks or checks timeout. "
f"{self.UNQUEUE_DOCUMENTATION}",
)

Expand Down
1 change: 1 addition & 0 deletions mergify_engine/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
MERGE_QUEUE_BRANCH_PREFIX = "mergify/merge-queue"
MERGE_QUEUE_SUMMARY_NAME = "Queue: Embarked in merge train"

CHECKS_TIMEOUT_CONDITION_LABEL = "checks-are-on-time"

MERGIFY_OPENSOURCE_SPONSOR_DOC = (
"<hr />\n"
Expand Down
20 changes: 20 additions & 0 deletions mergify_engine/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,7 @@ class ContextCaches:
datetime.time,
date.PartialDatetime,
datetime.datetime,
datetime.timedelta,
date.RelativeDatetime,
typing.List[github_types.SHAType],
typing.List[github_types.GitHubLogin],
Expand Down Expand Up @@ -1091,6 +1092,23 @@ async def _get_consolidated_data(self, name: str) -> ContextAttributeType:
else:
return date.RelativeDatetime(queued_at)

elif name in ("queue-merge-started-at", "queue-merge-started-at-relative"):
# Only used with QueuePullRequest
q = await merge_train.Train.from_context(self)
car = q.get_car_by_tmp_pull(self)
if car is None:
car = q.get_car(self)
if car is None:
started_at = date.DT_MAX
else:
started_at = car.creation_date
else:
started_at = car.creation_date
if name == "queue-merge-started-at":
return started_at
else:
return date.RelativeDatetime(started_at)

elif name == "label":
return [label["name"] for label in self.pull["labels"]]

Expand Down Expand Up @@ -1946,6 +1964,8 @@ class QueuePullRequest(BasePullRequest):
"current-day-of-week",
"current-timestamp",
"schedule",
"queue-merge-started-at",
"queue-merge-started-at-relative",
)

async def __getattr__(self, name: str) -> ContextAttributeType:
Expand Down
32 changes: 32 additions & 0 deletions mergify_engine/date.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,11 @@ def from_string(cls, value: str) -> "PartialDatetime":


class TimedeltaRegexResultT(typing.TypedDict):
filled: typing.Optional[str]
days: typing.Optional[str]
hours: typing.Optional[str]
minutes: typing.Optional[str]
seconds: typing.Optional[str]


@dataclasses.dataclass(order=True)
Expand Down Expand Up @@ -261,3 +263,33 @@ def fromtimestamp(timestamp: float) -> datetime.datetime:

def pretty_datetime(dt: datetime.datetime) -> str:
return dt.strftime("%Y-%m-%d %H:%M %Z")


_INTERVAL_RE = re.compile(
r"""
(?P<filled>
((?P<days>[-+]?\d+) \s+ d(ays?)? \s* )?
((?P<hours>[-+]?\d+) \s+ h(ours?)? \s* )?
((?P<minutes>[-+]?\d+) \s+ m(inutes?)? \s* )?
((?P<seconds>[-+]?\d+) \s+ s(econds?)? \s* )?
)
""",
re.VERBOSE,
)


def interval_from_string(value: str) -> datetime.timedelta:
m = _INTERVAL_RE.match(value)
if m is None:
raise InvalidDate("Invalid date interval")

kw = typing.cast(TimedeltaRegexResultT, m.groupdict())
if not kw or not kw["filled"]:
raise InvalidDate("Invalid date interval")

return datetime.timedelta(
days=int(kw["days"] or 0),
hours=int(kw["hours"] or 0),
minutes=int(kw["minutes"] or 0),
seconds=int(kw["seconds"] or 0),
)
8 changes: 8 additions & 0 deletions mergify_engine/json.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ def default(self, v: typing.Any) -> typing.Any:
"class": type(v).__name__,
"name": v.name,
}
elif isinstance(v, datetime.timedelta):
return {
"__pytype__": "datetime.timedelta",
"value": str(v.total_seconds()),
}

elif isinstance(v, datetime.datetime):
return {
"__pytype__": "datetime.datetime",
Expand Down Expand Up @@ -64,6 +70,8 @@ def _decode(v: typing.Dict[typing.Any, typing.Any]) -> typing.Any:
enum_cls = _JSON_TYPES[cls_name]
enum_name = v["name"]
return enum_cls[enum_name]
elif v.get("__pytype__") == "datetime.timedelta":
return datetime.timedelta(seconds=float(v["value"]))
elif v.get("__pytype__") == "datetime.datetime":
return datetime.datetime.fromisoformat(v["value"])
elif v.get("__pytype__") == "set":
Expand Down
12 changes: 12 additions & 0 deletions mergify_engine/queue/merge_train.py
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,16 @@ async def update_summaries(
f"The pull requests {refs} cannot be merged and will be split"
)

checks_timeout_summary = ""
if conclusion == check_api.Conclusion.FAILURE:
for condition in evaluated_queue_rule.conditions.walk():
if (
condition.label == constants.CHECKS_TIMEOUT_CONDITION_LABEL
and not condition.match
):
checks_timeout_summary = "\n\n⏲️ The checks have timed out ⏲️"
break

queue_summary = "\n\nRequired conditions for merge:\n\n"
queue_summary += evaluated_queue_rule.conditions.get_summary()

Expand Down Expand Up @@ -744,6 +754,7 @@ async def update_summaries(

if self.creation_state == "created":
summary = f"Embarking {self._get_embarked_refs(markdown=True)} together"
summary += checks_timeout_summary
summary += queue_summary + "\n" + batch_failure_summary

if self.queue_pull_request_number is None:
Expand Down Expand Up @@ -871,6 +882,7 @@ async def update_summaries(
conclusion,
title=original_pull_title,
summary=unexpected_change_summary
+ checks_timeout_summary
+ queue_summary
+ "\n"
+ checks_copy_summary
Expand Down
1 change: 1 addition & 0 deletions mergify_engine/queue/naive.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ async def get_config(
"batch_size": 1,
"allow_inplace_speculative_checks": True,
"allow_speculative_checks_interruption": True,
"checks_timeout": None,
},
}
)
Expand Down
37 changes: 33 additions & 4 deletions mergify_engine/rules/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import dataclasses
import datetime
import functools
import itertools
import logging
Expand All @@ -25,7 +26,9 @@
import yaml

from mergify_engine import actions
from mergify_engine import constants
from mergify_engine import context
from mergify_engine import date
from mergify_engine import github_types
from mergify_engine.rules import conditions
from mergify_engine.rules import live_resolvers
Expand Down Expand Up @@ -73,6 +76,7 @@ class QueueConfig(typing.TypedDict):
batch_size: int
allow_inplace_speculative_checks: bool
allow_speculative_checks_interruption: bool
checks_timeout: typing.Optional[datetime.timedelta]


EvaluatedQueueRule = typing.NewType("EvaluatedQueueRule", "QueueRule")
Expand Down Expand Up @@ -105,14 +109,29 @@ async def get_pull_request_rule(
logger: logging.LoggerAdapter,
log_schedule_details: bool,
) -> EvaluatedQueueRule:
branch_protection_conditions = (
await conditions.get_branch_protection_conditions(repository, ref)
extra_conditions = await conditions.get_branch_protection_conditions(
repository, ref
)
if self.config["checks_timeout"] is not None:
extra_conditions += (
conditions.RuleCondition(
{
">": (
"queue-merge-started-at-relative",
date.RelativeDatetime(
date.utcnow() - self.config["checks_timeout"]
),
)
},
label=constants.CHECKS_TIMEOUT_CONDITION_LABEL,
description=f"⏲️ Checks timeout setting ({self.config['checks_timeout']} seconds)",
),
)

queue_rule_with_branch_protection = QueueRule(
self.name,
conditions.QueueRuleConditions(
self.conditions.condition.copy().conditions
+ branch_protection_conditions
extra_conditions + self.conditions.condition.copy().conditions
),
self.config,
)
Expand Down Expand Up @@ -442,6 +461,13 @@ def get_pull_request_rules_schema(partial_validation: bool = False) -> voluptuou
)


def ChecksTimeout(v: str) -> datetime.timedelta:
td = date.interval_from_string(v)
if td < datetime.timedelta(seconds=60):
raise voluptuous.Invalid("Interval must be greater than 60 seconds")
return td


QueueRulesSchema = voluptuous.All(
[
voluptuous.All(
Expand All @@ -463,6 +489,9 @@ def get_pull_request_rules_schema(partial_validation: bool = False) -> voluptuou
voluptuous.Required(
"allow_speculative_checks_interruption", default=True
): bool,
voluptuous.Required("checks_timeout", default=None): voluptuous.Any(
None, voluptuous.All(str, voluptuous.Coerce(ChecksTimeout))
),
},
voluptuous.Coerce(QueueRule.from_dict),
)
Expand Down
8 changes: 6 additions & 2 deletions mergify_engine/rules/conditions.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class RuleCondition:
"""

condition: typing.Union[str, FakeTreeT]
label: typing.Optional[str] = None
description: typing.Optional[str] = None
partial_filter: filter.Filter[bool] = dataclasses.field(init=False)
match: bool = dataclasses.field(init=False, default=False)
Expand All @@ -55,6 +56,7 @@ def __post_init__(self) -> None:

def update(self, condition_raw: typing.Union[str, FakeTreeT]) -> None:
self.condition = condition_raw

try:
if isinstance(condition_raw, str):
condition = parser.parse(condition_raw)
Expand Down Expand Up @@ -84,13 +86,15 @@ def update_attribute_name(self, new_name: str) -> None:
self.update(new_tree)

def __str__(self) -> str:
if isinstance(self.condition, str):
if self.label is not None:
return self.label
elif isinstance(self.condition, str):
return self.condition
else:
return str(self.partial_filter)

def copy(self) -> "RuleCondition":
rc = RuleCondition(self.condition, self.description)
rc = RuleCondition(self.condition, self.label, self.description)
rc.partial_filter.value_expanders = self.partial_filter.value_expanders
return rc

Expand Down
1 change: 1 addition & 0 deletions mergify_engine/rules/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ class Parser(enum.Enum):
"closed-at": Parser.TIMESTAMP_OR_TIMEDELTA,
"merged-at": Parser.TIMESTAMP_OR_TIMEDELTA,
"queued-at": Parser.TIMESTAMP_OR_TIMEDELTA,
"queue-merge-started-at": Parser.TIMESTAMP_OR_TIMEDELTA,
"current-timestamp": Parser.TIMESTAMP,
"locked": Parser.BOOL,
"merged": Parser.BOOL,
Expand Down

0 comments on commit 7e5f214

Please sign in to comment.