Bug 1641971: Allow references to decision task via task-reference/artifact-reference; r=Callek

Differential Revision: https://phabricator.services.mozilla.com/D77547
This commit is contained in:
Tom Prince 2020-06-02 18:50:10 +00:00
parent de6126516d
commit 602da9cdcd
9 changed files with 61 additions and 22 deletions

View File

@ -185,8 +185,10 @@ using simple parameterized values, as follows:
The task definition may contain "task references" of this form. These will
be replaced during the optimization step, with the appropriate taskId for
the named dependency substituted for ``<dep-name>`` in the string.
Multiple labels may be substituted in a single string, and ``<<>`` can be
used to escape a literal ``<``.
Additionally, `decision` and `self` can be used a dependency names to refer
to the decision task, and the task itself. Multiple labels may be
substituted in a single string, and ``<<>`` can be used to escape a literal
``<``.
``{"artifact-reference": "..<dep-name/artifact/name>.."}``
Similar to a ``task-reference``, but this substitutes a URL to the queue's

View File

@ -434,7 +434,8 @@ class MachCommands(MachCommandBase):
tgg = taskgraph.generator.TaskGraphGenerator(
root_dir=options.get('root'),
parameters=parameters)
parameters=parameters,
)
actions = taskgraph.actions.render_actions_json(tgg.parameters, tgg.graph_config)
print(json.dumps(actions, sort_keys=True, indent=2, separators=(',', ': ')))

View File

@ -162,7 +162,8 @@ def create_tasks(graph_config, to_run, full_task_graph, label_to_taskid,
optimized_task_graph, label_to_taskid = optimize_task_graph(target_task_graph,
params,
to_run,
label_to_taskid)
label_to_taskid,
decision_task_id)
write_artifact('task-graph{}.json'.format(suffix), optimized_task_graph.to_json())
write_artifact('label-to-taskid{}.json'.format(suffix), label_to_taskid)
write_artifact('to-run{}.json'.format(suffix), list(to_run))

View File

@ -208,10 +208,14 @@ def taskgraph_decision(options, parameters=None):
lambda graph_config: get_decision_parameters(graph_config, options)
)
decision_task_id = os.environ['TASK_ID']
# create a TaskGraphGenerator instance
tgg = TaskGraphGenerator(
root_dir=options.get('root'),
parameters=parameters)
parameters=parameters,
decision_task_id=decision_task_id,
)
# write out the parameters used to generate this graph
write_artifact('parameters.yml', dict(**tgg.parameters))

View File

@ -107,7 +107,9 @@ class TaskGraphGenerator(object):
# each "phase" of generation. This allows some mach subcommands to short-
# circuit generation of the entire graph by never completing the generator.
def __init__(self, root_dir, parameters, target_kind=None):
def __init__(
self, root_dir, parameters, decision_task_id="<decision-task>", target_kind=None,
):
"""
@param root_dir: root directory, with subdirectories for each kind
@param paramaters: parameters for this task-graph generation, or callable
@ -119,6 +121,7 @@ class TaskGraphGenerator(object):
self.root_dir = ensure_text(root_dir)
self._parameters = parameters
self._target_kind = target_kind
self._decision_task_id = decision_task_id
# start the generator
self._run = self._run()
@ -341,6 +344,7 @@ class TaskGraphGenerator(object):
target_task_graph,
parameters,
do_not_optimize,
self._decision_task_id,
existing_tasks=existing_tasks,
strategy_override=strategies,
)

View File

@ -40,7 +40,7 @@ def register_strategy(name, args=()):
def optimize_task_graph(target_task_graph, params, do_not_optimize,
existing_tasks=None, strategy_override=None):
decision_task_id, existing_tasks=None, strategy_override=None):
"""
Perform task optimization, returning a taskgraph and a map from label to
assigned taskId, including replacement tasks.
@ -73,7 +73,7 @@ def optimize_task_graph(target_task_graph, params, do_not_optimize,
return get_subgraph(
target_task_graph, removed_tasks, replaced_tasks,
label_to_taskid), label_to_taskid
label_to_taskid, decision_task_id), label_to_taskid
def _get_optimizations(target_task_graph, strategies):
@ -186,7 +186,9 @@ def replace_tasks(target_task_graph, params, optimizations, do_not_optimize,
return replaced
def get_subgraph(target_task_graph, removed_tasks, replaced_tasks, label_to_taskid):
def get_subgraph(
target_task_graph, removed_tasks, replaced_tasks, label_to_taskid, decision_task_id,
):
"""
Return the subgraph of target_task_graph consisting only of
non-optimized tasks and edges between them.
@ -234,6 +236,7 @@ def get_subgraph(target_task_graph, removed_tasks, replaced_tasks, label_to_task
task.label,
task.task,
task_id=task.task_id,
decision_task_id=decision_task_id,
dependencies=named_task_dependencies,
)
deps = task.task.setdefault('dependencies', [])

View File

@ -240,7 +240,8 @@ class TestOptimize(unittest.TestCase):
optimize.slugid = ('tid{}'.format(i) for i in range(1, 10)).next
try:
got_subgraph = optimize.get_subgraph(graph, removed_tasks,
replaced_tasks, label_to_taskid)
replaced_tasks, label_to_taskid,
"DECISION-TASK")
finally:
optimize.slugid = slugid
self.assertEqual(got_subgraph.graph, exp_subgraph.graph)

View File

@ -53,7 +53,9 @@ class TestTaskRefs(unittest.TestCase):
def do(self, input, output):
taskid_for_edge_name = {'edge%d' % n: 'tid%d' % n for n in range(1, 4)}
self.assertEqual(
resolve_task_references("subject", input, "tid-self", taskid_for_edge_name),
resolve_task_references(
"subject", input, "tid-self", "tid-decision", taskid_for_edge_name,
),
output,
)
@ -97,13 +99,18 @@ class TestTaskRefs(unittest.TestCase):
self.do({'escape': {'task-reference': '<self>'}},
{'escape': 'tid-self'})
def test_decision(self):
"resolve_task_references resolves `decision` to the provided decision task id"
self.do({'escape': {'task-reference': '<decision>'}},
{'escape': 'tid-decision'})
def test_invalid(self):
"resolve_task_references raises a KeyError on reference to an invalid task"
self.assertRaisesRegexp(
KeyError,
"task 'subject' has no dependency named 'no-such'",
lambda: resolve_task_references(
"subject", {"task-reference": "<no-such>"}, "tid-self", {}
"subject", {"task-reference": "<no-such>"}, "tid-self", "tid-decision", {}
),
)
@ -118,7 +125,7 @@ class TestArtifactRefs(unittest.TestCase):
with mock.patch.dict(os.environ, {'TASKCLUSTER_ROOT_URL': 'https://tc-tests.localhost'}):
self.assertEqual(
resolve_task_references(
"subject", input, "tid-self", taskid_for_edge_name
"subject", input, "tid-self", "tid-decision", taskid_for_edge_name
),
output,
)
@ -155,17 +162,27 @@ class TestArtifactRefs(unittest.TestCase):
KeyError,
"task 'subject' can't reference artifacts of self",
lambda: resolve_task_references(
"subject", {"artifact-reference": "<self/path>"}, "tid-self", {}
"subject", {"artifact-reference": "<self/public/artifact>"},
"tid-self", "tid-decision", {}
),
)
def test_decision(self):
"resolve_task_references resolves `decision` to the provided decision task id"
self.do(
{'stuff': {'artifact-reference': '<decision/public/artifact>'}},
{'stuff': 'https://tc-tests.localhost/api/queue/v1/task/tid-decision/'
'artifacts/public/artifact'},
)
def test_invalid(self):
"resolve_task_references raises a KeyError on reference to an invalid task"
self.assertRaisesRegexp(
KeyError,
"task 'subject' has no dependency named 'no-such'",
lambda: resolve_task_references(
"subject", {"artifact-reference": "<no-such/path>"}, "tid-self", {}
"subject", {"artifact-reference": "<no-such/public/artifact>"},
"tid-self", "tid-decision", {}
),
)
@ -173,7 +190,7 @@ class TestArtifactRefs(unittest.TestCase):
"resolve_task_references ignores badly-formatted artifact references"
for inv in ['<edge1>', 'edge1/foo>', '<edge1>/foo', '<edge1>foo']:
resolved = resolve_task_references(
"subject", {"artifact-reference": inv}, "tid-self", {}
"subject", {"artifact-reference": inv}, "tid-self", "tid-decision", {}
)
self.assertEqual(resolved, inv)

View File

@ -35,7 +35,7 @@ def resolve_timestamps(now, task_def):
})
def resolve_task_references(label, task_def, task_id, dependencies):
def resolve_task_references(label, task_def, task_id, decision_task_id, dependencies):
"""Resolve all instances of
{'task-reference': '..<..>..'}
and
@ -49,6 +49,8 @@ def resolve_task_references(label, task_def, task_id, dependencies):
key = match.group(1)
if key == 'self':
return task_id
elif key == 'decision':
return decision_task_id
try:
return dependencies[key]
except KeyError:
@ -67,11 +69,15 @@ def resolve_task_references(label, task_def, task_id, dependencies):
raise KeyError(
"task '{}' can't reference artifacts of self".format(label)
)
try:
task_id = dependencies[dependency]
except KeyError:
raise KeyError("task '{}' has no dependency named '{}'".format(label, dependency))
elif dependency == 'decision':
task_id = decision_task_id
else:
try:
task_id = dependencies[dependency]
except KeyError:
raise KeyError(
"task '{}' has no dependency named '{}'".format(label, dependency)
)
assert artifact_name.startswith('public/'), \
"artifact-reference only supports public artifacts, not `{}`".format(artifact_name)