Skip to content

Commit fd57d0c

Browse files
Alphareahal
authored andcommitted
feat: allow for both batched and single queries in task replacement
This patch removes the need for migrating all call sites, especially in `gecko`. This is the result of more discussion following the two previous patches¹. Turns out that while duck typing in interfaces is not great, leaking implementation details to the user is probably worse. [1] 41a10aa [1] 5207917
1 parent 41a10aa commit fd57d0c

File tree

3 files changed

+46
-16
lines changed

3 files changed

+46
-16
lines changed

src/taskgraph/docker.py

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,8 @@
1717

1818
from taskgraph.util import docker
1919
from taskgraph.util.taskcluster import (
20-
find_task_id_batched,
2120
get_artifact_url,
2221
get_session,
23-
status_task_batched,
2422
)
2523

2624
DEPLOY_WARNING = """
@@ -66,15 +64,7 @@ def load_image_by_name(image_name, tag=None):
6664
task = tasks[f"build-docker-image-{image_name}"]
6765

6866
indexes = task.optimization.get("index-search", [])
69-
index_to_taskid = {}
70-
task_id = False
71-
if indexes:
72-
indexes = list(indexes)
73-
index_to_taskid = find_task_id_batched(indexes)
74-
taskid_to_status = status_task_batched(list(index_to_taskid.values()))
75-
76-
arg = (indexes, index_to_taskid, taskid_to_status)
77-
task_id = IndexSearch().should_replace_task(task, {}, None, arg)
67+
task_id = IndexSearch().should_replace_task(task, {}, None, indexes)
7868

7969
if task_id in (True, False):
8070
print(

src/taskgraph/optimize/strategies.py

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
from taskgraph.optimize.base import OptimizationStrategy, register_strategy
55
from taskgraph.util.path import match as match_path
6+
from taskgraph.util.taskcluster import find_task_id, status_task
67

78
logger = logging.getLogger(__name__)
89

@@ -23,12 +24,28 @@ class IndexSearch(OptimizationStrategy):
2324

2425
def should_replace_task(self, task, params, deadline, arg):
2526
"Look for a task with one of the given index paths"
26-
index_paths, label_to_taskid, taskid_to_status = arg
27+
batched = False
28+
# Appease static checker that doesn't understand that this is not needed
29+
label_to_taskid = {}
30+
taskid_to_status = {}
31+
32+
if isinstance(arg, tuple) and len(arg) == 3:
33+
# allow for a batched call optimization instead of two queries
34+
# per index path
35+
index_paths, label_to_taskid, taskid_to_status = arg
36+
batched = True
37+
else:
38+
index_paths = arg
2739

2840
for index_path in index_paths:
2941
try:
30-
task_id = label_to_taskid[index_path]
31-
status = taskid_to_status[task_id]
42+
if batched:
43+
task_id = label_to_taskid[index_path]
44+
status = taskid_to_status[task_id]
45+
else:
46+
# 404 is raised as `KeyError` also end up here
47+
task_id = find_task_id(index_path)
48+
status = status_task(task_id)
3249
# status can be `None` if we're in `testing` mode
3350
# (e.g. test-action-callback)
3451
if not status or status.get("state") in ("exception", "failed"):

test/test_optimize_strategies.py

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Any copyright is dedicated to the public domain.
22
# http://creativecommons.org/publicdomain/zero/1.0/
33

4+
import os
45
from datetime import datetime
56
from test.fixtures.gen import make_task
67
from time import mktime
@@ -43,9 +44,29 @@ def params():
4344
),
4445
),
4546
)
46-
def test_index_search(state, expires, expected):
47+
def test_index_search(responses, params, state, expires, expected):
4748
taskid = "abc"
4849
index_path = "foo.bar.latest"
50+
51+
responses.add(
52+
responses.GET,
53+
f"{os.environ['TASKCLUSTER_ROOT_URL']}/api/index/v1/task/{index_path}",
54+
json={"taskId": taskid},
55+
status=200,
56+
)
57+
58+
responses.add(
59+
responses.GET,
60+
f"{os.environ['TASKCLUSTER_ROOT_URL']}/api/queue/v1/task/{taskid}/status",
61+
json={
62+
"status": {
63+
"state": state,
64+
"expires": expires,
65+
}
66+
},
67+
status=200,
68+
)
69+
4970
label_to_taskid = {index_path: taskid}
5071
taskid_to_status = {
5172
taskid: {
@@ -58,10 +79,12 @@ def test_index_search(state, expires, expected):
5879
deadline = "2021-06-07T19:03:20.482Z"
5980
assert (
6081
opt.should_replace_task(
61-
{}, params, deadline, ((index_path,), label_to_taskid, taskid_to_status)
82+
{}, params, deadline, ([index_path], label_to_taskid, taskid_to_status)
6283
)
6384
== expected
6485
)
86+
# test the non-batched variant as well
87+
assert opt.should_replace_task({}, params, deadline, [index_path]) == expected
6588

6689

6790
@pytest.mark.parametrize(

0 commit comments

Comments
 (0)