1413196 - Almost permafailing gecko decision tasks on Try: TypeError: argument of type 'NoneType' is not iterable during Generating tasks for release-bouncer-sub fennec

Status: RESOLVED FIXED

(Taskcluster :: General, defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Try will get closed for this:

https://treeherder.mozilla.org/#/jobs?repo=try&filter-searchStr=gecko%20decision%20task&tochange=194b7e74297d6ef9cb1cee53ff92b6a10b06009d

https://treeherder.mozilla.org/logviewer.html#?job_id=141055130&repo=try

[task 2017-10-31T13:46:06.762585Z] Generated 7 tasks for kind docker-image
[task 2017-10-31T13:46:06.762738Z] Loading tasks for kind release-bouncer-sub
[task 2017-10-31T13:46:06.794308Z] Generating tasks for release-bouncer-sub fennec
[task 2017-10-31T13:46:06.796469Z] Traceback (most recent call last):
[task 2017-10-31T13:46:06.796543Z]   File "/builds/worker/checkouts/gecko/taskcluster/mach_commands.py", line 165, in taskgraph_decision
[task 2017-10-31T13:46:06.796669Z]     return taskgraph.decision.taskgraph_decision(options)
[task 2017-10-31T13:46:06.796736Z]   File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/decision.py", line 113, in taskgraph_decision
[task 2017-10-31T13:46:06.796785Z]     full_task_json = tgg.full_task_graph.to_json()
[task 2017-10-31T13:46:06.797065Z]   File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/generator.py", line 130, in full_task_graph
[task 2017-10-31T13:46:06.797315Z]     return self._run_until('full_task_graph')
[task 2017-10-31T13:46:06.797404Z]   File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/generator.py", line 287, in _run_until
[task 2017-10-31T13:46:06.797451Z]     k, v = self._run.next()
[task 2017-10-31T13:46:06.797514Z]   File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/generator.py", line 217, in _run
[task 2017-10-31T13:46:06.797570Z]     new_tasks = kind.load_tasks(self.parameters, list(all_tasks.values()))
[task 2017-10-31T13:46:06.797630Z]   File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/generator.py", line 65, in load_tasks
[task 2017-10-31T13:46:06.797676Z]     for task_dict in transforms(trans_config, inputs)]
[task 2017-10-31T13:46:06.797742Z]   File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/task.py", line 1433, in check_run_task_caches
[task 2017-10-31T13:46:06.797776Z]     for task in tasks:
[task 2017-10-31T13:46:06.797985Z]   File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/task.py", line 1366, in build_task
[task 2017-10-31T13:46:06.798061Z]     resolve_keyed_by(v, 'ids', 'notifications', **config.params)
[task 2017-10-31T13:46:06.798122Z]   File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/util/schema.py", line 107, in resolve_keyed_by
[task 2017-10-31T13:46:06.798168Z]     if subfield not in container:
[task 2017-10-31T13:46:06.798218Z] TypeError: argument of type 'NoneType' is not iterable

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Is there any way to add test coverage for this?

Comment 2

[Rok Garbas [:garbas]]

:RyanVM I can not find where taskgraph task transform is tested. I assume none of the code in taskcluster/taskgraph/tranforms/task.py is tested

:aki :rail Since I'm new to taskcluster/taskgraph/* code, do you maybe know if there are some tests for this? I'm sure we can add test for this part, but we should also have tests for other parts of task transforms and also tests for other transforms.

Comment 3

[Rok Garbas [:garbas]]

Attachment: Bug 1413196 - Almost permafailing gecko decision tasks on Try: TypeError: argument of type 'NoneType' is not iterable during Generating tasks for release-bouncer-sub fennec

Review commit: https://reviewboard.mozilla.org/r/194974/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/194974/

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Confirmed this patch fixes the issue I was seeing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=864bfe5f73f048da587f96f487d989ed037b1b71

Comment 5

[Rok Garbas [:garbas]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e5f5e91b78fcf916d7567fa32be6289e6a892d5f

Comment 6

[Rail Aliiev [:rail]]

Comment on attachment 8923822 [details]
Bug 1413196 - Almost permafailing gecko decision tasks on Try: TypeError: argument of type 'NoneType' is not iterable during Generating tasks for release-bouncer-sub fennec

https://reviewboard.mozilla.org/r/194974/#review200008

::: taskcluster/taskgraph/transforms/task.py:1346
(Diff revision 1)
>              for k, v in notifications.items():
>                  if isinstance(v, dict) and len(v) == 1 and v.keys()[0].startswith('by-'):
>                      v = {'tmp': v}
>                      resolve_keyed_by(v, 'tmp', 'notifications', **config.params)
>                      v = v['tmp']
> -                if isinstance(v, list):
> +                if v is None:

Can you move this check and make it the first after `for k, v in notifications.items():` please

Comment 7

[Rail Aliiev [:rail]]

(In reply to Rail Aliiev [:rail] ⌚️ET from comment #6)
> Comment on attachment 8923822 [details]
> Bug 1413196 - Almost permafailing gecko decision tasks on Try: TypeError:
> argument of type 'NoneType' is not iterable during Generating tasks for
> release-bouncer-sub fennec
> 
> https://reviewboard.mozilla.org/r/194974/#review200008
> 
> ::: taskcluster/taskgraph/transforms/task.py:1346
> (Diff revision 1)
> >              for k, v in notifications.items():
> >                  if isinstance(v, dict) and len(v) == 1 and v.keys()[0].startswith('by-'):
> >                      v = {'tmp': v}
> >                      resolve_keyed_by(v, 'tmp', 'notifications', **config.params)
> >                      v = v['tmp']
> > -                if isinstance(v, list):
> > +                if v is None:
> 
> Can you move this check and make it the first after `for k, v in
> notifications.items():` please

nm, we need it in the current position because we mutate `v` in the first condition

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8b8e9e2e6243
Almost permafailing gecko decision tasks on Try: TypeError: argument of type 'NoneType' is not iterable during Generating tasks for release-bouncer-sub fennec r=rail

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/0a7ff6e19bcc
Almost permafailing gecko decision tasks on Try: TypeError: argument of type 'NoneType' is not iterable during Generating tasks for release-bouncer-sub fennec r=rail a=bustage

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Reopened Try.

Comment 11

[Aki Sasaki (not active)]

I would love for us to add tests :)

Comment 12

[Dustin J. Mitchell [:dustin] (he/him)]

It's going to be difficult to add tests for this -- it's designed to be flexible in two ways: it runs with lots of different inputs and resulting outputs; and it is designed to be modified frequently.  So end-to-end tests (parameters in, taskgraph out) would be outdated by every change, and we'd quickly fall back to some shell script that generates results, copies them to the expected results in the tests, and commits the result, eliminating the benefit of the tests.  Also, we would need hundred thousand parameters to get a representative sample, so the test run would take tens of minutes, at least.

I think the best approach for testing is to test transforms individually.  The schemas provide some assurance that the test inputs are at least well-formed, and it's much easier to invent a set of "interesting" inputs to a single transform.  Even so, those tests would need to be written carefully to highlight important differences without being too sensitive to things others might want to change.  It would be easy for the tests to embody the same complexity as the transform, in which case anyone modifying the transform just has to write everything twice.

In this particular case, the issues that caused problems stood out pretty clearly in review, but in general that has not been the case.  Carefully applied testing could certainly improve the situation.

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/8b8e9e2e6243