From 6fbdc1ff040faabada96a08a124907dfa475c739 Mon Sep 17 00:00:00 2001 From: Alex Franchuk Date: Fri, 1 Nov 2024 09:46:50 -0400 Subject: [PATCH 1/3] Do not apply skip-unless-changed for cron triggers. --- docs/reference/optimization-strategies.rst | 10 +++------- src/taskgraph/optimize/strategies.py | 5 +++-- test/test_optimize_strategies.py | 2 +- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/docs/reference/optimization-strategies.rst b/docs/reference/optimization-strategies.rst index ef5d5444b..a48912ae4 100644 --- a/docs/reference/optimization-strategies.rst +++ b/docs/reference/optimization-strategies.rst @@ -10,15 +10,11 @@ Strategies for Removing Tasks skip-unless-changed ~~~~~~~~~~~~~~~~~~~ -.. note:: - - This strategy is only implemented for Mercurial repositories hosted on - ``hg.mozilla.org``. - The :class:`skip-unless-changed ` strategy will optimize the -target task away *unless* a specified file is modified. Glob patterns are -supported. +target task away *unless* a specified file is modified. When there is no +difference between head and base revs (for example, cron or action tasks), this +optimization will not apply. Glob patterns are supported. Example: diff --git a/src/taskgraph/optimize/strategies.py b/src/taskgraph/optimize/strategies.py index 78836e4f4..f1944fc4a 100644 --- a/src/taskgraph/optimize/strategies.py +++ b/src/taskgraph/optimize/strategies.py @@ -80,8 +80,9 @@ def check(self, files_changed, patterns): return False def should_remove_task(self, task, params, file_patterns): - # pushlog_id == -1 - this is the case when run from a cron.yml job or on a git repository - if params.get("repository_type") == "hg" and params.get("pushlog_id") == -1: + # skip-unless-changed should not apply when there is no commit delta, + # such as for cron and action tasks (there will never be file changes) + if params.get("head_rev") == params.get("base_rev"): return False changed = self.check(params["files_changed"], file_patterns) diff --git a/test/test_optimize_strategies.py b/test/test_optimize_strategies.py index 5ec2faef1..6322ca83c 100644 --- a/test/test_optimize_strategies.py +++ b/test/test_optimize_strategies.py @@ -135,7 +135,7 @@ def test_index_search(caplog, responses, params, state, expires, expected, logs) {"files_changed": ["bar.txt"]}, ["foo.txt"], True, id="files don't match" ), pytest.param( - {"repository_type": "hg", "pushlog_id": -1, "files_changed": ["bar.txt"]}, + {"head_rev": "8843d7f92416211de9ebb963ff4ce28125932878", "base_rev": "8843d7f92416211de9ebb963ff4ce28125932878", "files_changed": ["bar.txt"]}, ["foo.txt"], False, id="cron task", From f2f4fa279986415e5f2690d2e01db185258be741 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 4 Nov 2024 16:31:03 +0000 Subject: [PATCH 2/3] style: pre-commit.ci auto fixes [...] --- test/test_optimize_strategies.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/test_optimize_strategies.py b/test/test_optimize_strategies.py index 6322ca83c..9d08dcfaa 100644 --- a/test/test_optimize_strategies.py +++ b/test/test_optimize_strategies.py @@ -135,7 +135,11 @@ def test_index_search(caplog, responses, params, state, expires, expected, logs) {"files_changed": ["bar.txt"]}, ["foo.txt"], True, id="files don't match" ), pytest.param( - {"head_rev": "8843d7f92416211de9ebb963ff4ce28125932878", "base_rev": "8843d7f92416211de9ebb963ff4ce28125932878", "files_changed": ["bar.txt"]}, + { + "head_rev": "8843d7f92416211de9ebb963ff4ce28125932878", + "base_rev": "8843d7f92416211de9ebb963ff4ce28125932878", + "files_changed": ["bar.txt"], + }, ["foo.txt"], False, id="cron task", From 0c472068404dbbd528b5892bff32f19f614ee076 Mon Sep 17 00:00:00 2001 From: Alex Franchuk Date: Tue, 5 Nov 2024 13:56:34 -0500 Subject: [PATCH 3/3] Apply head/base comparison only when base is non-empty (i.e., there are valid revs). --- src/taskgraph/optimize/strategies.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/taskgraph/optimize/strategies.py b/src/taskgraph/optimize/strategies.py index f1944fc4a..5d03ae603 100644 --- a/src/taskgraph/optimize/strategies.py +++ b/src/taskgraph/optimize/strategies.py @@ -82,7 +82,7 @@ def check(self, files_changed, patterns): def should_remove_task(self, task, params, file_patterns): # skip-unless-changed should not apply when there is no commit delta, # such as for cron and action tasks (there will never be file changes) - if params.get("head_rev") == params.get("base_rev"): + if params.get("base_rev") and params.get("head_rev") == params.get("base_rev"): return False changed = self.check(params["files_changed"], file_patterns)