Closed
Bug 1382911
Opened 7 years ago
Closed 7 years ago
Custom action callback to trigger a bunch of jobs
Categories
(Firefox Build System :: Task Configuration, task)
Firefox Build System
Task Configuration
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla56
People
(Reporter: hassan, Assigned: hassan, Mentored)
Details
Attachments
(1 file, 3 obsolete files)
No description provided.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•7 years ago
|
||
Invoking create_task() complained that I was missing scopes: [task 2017-07-21T02:18:18.715592Z] In other words you are missing scopes from one of the options: [task 2017-07-21T02:18:18.715623Z] * Option 0: [task 2017-07-21T02:18:18.715678Z] - "queue:create-task:highest:aws-provisioner-v1/gecko-1-b-win2012", and [task 2017-07-21T02:18:18.715718Z] - "queue:scheduler-id:-" [task 2017-07-21T02:18:18.715754Z] * Option 1: [task 2017-07-21T02:18:18.715805Z] - "queue:create-task:very-high:aws-provisioner-v1/gecko-1-b-win2012", and [task 2017-07-21T02:18:18.715847Z] - "queue:scheduler-id:-" [task 2017-07-21T02:18:18.715876Z] * Option 2: [task 2017-07-21T02:18:18.715927Z] - "queue:create-task:high:aws-provisioner-v1/gecko-1-b-win2012", and [task 2017-07-21T02:18:18.715964Z] - "queue:scheduler-id:-" [task 2017-07-21T02:18:18.715997Z] * Option 3: [task 2017-07-21T02:18:18.716047Z] - "queue:create-task:medium:aws-provisioner-v1/gecko-1-b-win2012", and [task 2017-07-21T02:18:18.716103Z] - "queue:scheduler-id:-" [task 2017-07-21T02:18:18.716154Z] * Option 4: [task 2017-07-21T02:18:18.716204Z] - "queue:create-task:low:aws-provisioner-v1/gecko-1-b-win2012", and [task 2017-07-21T02:18:18.716241Z] - "queue:scheduler-id:-" [task 2017-07-21T02:18:18.716270Z] * Option 5: [task 2017-07-21T02:18:18.716302Z] - "queue:scheduler-id:-"
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8888602 [details] Bug 1382911 - Custom action callback to trigger a bunch of jobs https://reviewboard.mozilla.org/r/159604/#review164976 ::: taskcluster/actions/add-new-jobs.py:30 (Diff revision 1) > + 'type': 'object', > + 'properties': { > + 'tasks': { > + 'type': 'array', > + 'default': [ > + 'build-win64/opt' watch out for trailing whitespace..
Updated•7 years ago
|
Assignee: nobody → helfi92
Comment 4•7 years ago
|
||
Hassan, can you link to the action task where you saw those errors?
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to Dustin J. Mitchell [:dustin] from comment #4) > Hassan, can you link to the action task where you saw those errors? https://treeherder.mozilla.org/#/jobs?repo=try&revision=61fb5544aac6a119f90891d43390c74ba86f3303&selectedJob=116227379
Comment 6•7 years ago
|
||
Ah, I think the issue is that the task you're creating doesn't have schedulerId set, so it's defaulting to "-", which is why that scope is required ("queue:scheduler-id:-"). Hopefully that points you in the right direction?
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 9•7 years ago
|
||
(In reply to Dustin J. Mitchell [:dustin] from comment #6) > Ah, I think the issue is that the task you're creating doesn't have > schedulerId set, so it's defaulting to "-", which is why that scope is > required ("queue:scheduler-id:-"). Hopefully that points you in the right > direction? You were right, I added the schedulerId and then the task retriggered successfully [1]. But there is something strange however. The schedulerId property in task details [2] still shows "-" as the schedulerId. [1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=72022a3bf620ab7d211645aa9722959f000ae115&selectedJob=116368885 [2] https://tools.taskcluster.net/groups/XOBHDZpvTsmTVbG4JDHqwg/tasks/XOBHDZpvTsmTVbG4JDHqwg/details
Comment 10•7 years ago
|
||
[2] is the action task itself. The task it retriggers is https://tools.taskcluster.net/groups/a0d5r1wbTyuo73h0w3wKvg/tasks/a0d5r1wbTyuo73h0w3wKvg/details which does have the correct schedulerId. However, we should be setting the correct schedulerId on action tasks, too -- I'll open a separate bug for that.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8888762 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8888765 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8888816 -
Attachment is obsolete: true
Comment 16•7 years ago
|
||
mozreview-review |
Comment on attachment 8888602 [details] Bug 1382911 - Custom action callback to trigger a bunch of jobs https://reviewboard.mozilla.org/r/159604/#review165246 Looking good! Just some small changes. ::: taskcluster/actions/add-new-jobs.py:18 (Diff revision 3) > +TASKCLUSTER_INDEX_URL = 'https://index.taskcluster.net/v1/task' > + > +logger = logging.getLogger(__name__) > + > + > +@register_callback_action( Rather than calling this retrigger tasks, let's call it "add new jobs". That should be in name, title, and description. ::: taskcluster/actions/add-new-jobs.py:21 (Diff revision 3) > + > + > +@register_callback_action( > + name='Retrigger tasks', > + title='Schedule Tasks Retrigger', > + symbol='rj-custom', I'm not sure this is a good symbol. Perhaps `a-n` or `add-new`? Our rules for what is a good symbol are undefined afaict. ::: taskcluster/actions/add-new-jobs.py:30 (Diff revision 3) > + schema={ > + 'type': 'object', > + 'properties': { > + 'tasks': { > + 'type': 'array', > + 'default': [ We should not have a default task to add. We should specify that the elements of the array are strings though. http://json-schema.org/latest/json-schema-validation.html#rfc.section.6.9 is how to specify elements of an array in jsonschema ::: taskcluster/actions/add-new-jobs.py:43 (Diff revision 3) > + s = requests.Session() > + task_graph = s.get(url='{}/gecko.v2.{}.pushlog-id.{}.decision/artifacts/{}'.format( > + TASKCLUSTER_INDEX_URL, > + parameters['project'], > + parameters['pushlog_id'], > + 'public/full-task-graph.json')).json() This is _probably_ ok, but just a heads up that calling `.json()` directly on the `s.get(...)` creates a confusing error if you get a 404 or something. Check out `status_code` and `raise_for_status` in https://reviewboard.mozilla.org/r/158948/diff/1#index_header I don't promise that I'm doing it correctly either, but maybe you can figure out a nice way to do it. ::: taskcluster/actions/add-new-jobs.py:46 (Diff revision 3) > + parameters['project'], > + parameters['pushlog_id'], > + 'public/full-task-graph.json')).json() > + > + for elem in input['tasks']: > + if elem in task_graph: Let's also have an `else` that logs that the task wasn't in the task graph to begin with. ::: taskcluster/actions/add-new-jobs.py:52 (Diff revision 3) > + new_task_definition = copy.copy(task_graph[elem]['task']) > + > + # set new created, deadline, and expiry fields > + new_task_definition['created'] = current_json_time() > + new_task_definition['deadline'] = json_time_from_now('1d') > + new_task_definition['expires'] = json_time_from_now('30d') Unfortunately, we also need to go through and update the timestamps for `expires` in artifacts. I have some code for it in the PR I linked above as well that you can find. If you basically copy-paste that for now, I can move it out into a nice shared function when I land my changes.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 20•7 years ago
|
||
mozreview-review |
Comment on attachment 8888602 [details] Bug 1382911 - Custom action callback to trigger a bunch of jobs https://reviewboard.mozilla.org/r/159604/#review165884 ::: taskcluster/actions/add-new-jobs.py:19 (Diff revision 6) > + > +logger = logging.getLogger(__name__) > + > + > +@register_callback_action( > + name='add new jobs', name should not have spaces. perhaps use underscore case.
Comment hidden (mozreview-request) |
Comment 22•7 years ago
|
||
mozreview-review |
Comment on attachment 8888602 [details] Bug 1382911 - Custom action callback to trigger a bunch of jobs https://reviewboard.mozilla.org/r/159604/#review165898 Awesome!
Attachment #8888602 -
Flags: review?(bstack) → review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 23•7 years ago
|
||
Autoland can't push this because it doesn't meet the necessary review requirements in MozReview. http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/autoland.html#landing-commits
Keywords: checkin-needed
Comment 24•7 years ago
|
||
mozreview-review |
Comment on attachment 8888602 [details] Bug 1382911 - Custom action callback to trigger a bunch of jobs https://reviewboard.mozilla.org/r/159604/#review165972 ::: taskcluster/actions/add-new-jobs.py:32 (Diff revision 7) > + 'properties': { > + 'tasks': { > + 'type': 'array', > + 'items': { > + 'type': 'string' > + } This should have some descriptions and/or titles to indicate what it's an array of -- I *think* it's an array of task labels? ::: taskcluster/actions/add-new-jobs.py:47 (Diff revision 7) > + parameters['pushlog_id'], > + 'public/full-task-graph.json')) > + > + if r.status_code != requests.codes.ok: > + if r.status_code == 404: > + logger.error('Could not find task graph on push {}. Reason: {}'.format( `logger.error` doesn't automaticlally raise an exception or terminate the process. These (both usees of `logger.error` in this file) would probably be better handled with `raise Exception(..)` ::: taskcluster/actions/add-new-jobs.py:61 (Diff revision 7) > + new_task_definition = copy.copy(task_graph[elem]['task']) > + > + # set new created, deadline, and expiry fields > + new_task_definition['created'] = current_json_time() > + new_task_definition['deadline'] = json_time_from_now('1d') > + new_task_definition['expires'] = json_time_from_now('30d') I don't think these, or the artifact expiration updates below, are necessary -- `taskgraph.create.create_task` replaces `{relative-datestamp: ..}` with actual datestamps before creating the task, and the tasks in `full-task-graph.json` contain datestamps in that format. ::: taskcluster/actions/add-new-jobs.py:71 (Diff revision 7) > + if isinstance(artifacts, dict): > + artifacts = artifacts.values() > + for artifact in artifacts: > + artifact['expires'] = new_task_definition['expires'] > + > + logger.info("New full task definition: %s", new_task_definition) It's OK for testing in try, but we shouldn't land code that has such big logging output. ::: taskcluster/actions/add-new-jobs.py:76 (Diff revision 7) > + logger.info("New full task definition: %s", new_task_definition) > + > + # actually create the new task > + new_task_id = slugid() > + > + create_task(s, new_task_id, 'add-new', new_task_definition) Aside from `relative-datestamp`, the tasks in `full-task-graph.json` also contain `{task-reference: "..<..>.."}` which is a way to refer to the taskId of a dependent task. I strongly suspect that this action won't work, since it doesn't do anything with those task references. You can see how I handled that in https://reviewboard.mozilla.org/r/159452/diff/4#index_header
Attachment #8888602 -
Flags: review-
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 28•7 years ago
|
||
Comment on attachment 8888602 [details] Bug 1382911 - Custom action callback to trigger a bunch of jobs I rewrote the implementation based on [1]. One thing I noticed is that label-to-taskid.json doesn't have a full mapping of full-task-graph.json labels. For example, android-api-15-gradle-dependencies is shown in the latter but not on the former. Is that normal? Any clarifications would be appreciated. [1] https://reviewboard.mozilla.org/r/159452/diff/4#index_header
Attachment #8888602 -
Flags: review+ → feedback?(dustin)
Comment 29•7 years ago
|
||
Yes, that's normal -- label-to-taskid is only for jobs that were actually created by the original decision task, so anything that it optimized out will not be present. If you find yourself creating a task which has a dependency that is not in label-to-taskid, then you'll also need to create that dependency task (first). I was hoping that wouldn't happen for this first round of actions, and that once we have some common utility functions we could just implement that create-dependencies-as-needed behavior once.
Assignee | ||
Comment 30•7 years ago
|
||
(In reply to Dustin J. Mitchell [:dustin] from comment #29) > Yes, that's normal -- label-to-taskid is only for jobs that were actually > created by the original decision task, so anything that it optimized out > will not be present. If you find yourself creating a task which has a > dependency that is not in label-to-taskid, then you'll also need to create > that dependency task (first). I was hoping that wouldn't happen for this > first round of actions, and that once we have some common utility functions > we could just implement that create-dependencies-as-needed behavior once. Makes a lot of sense!
Assignee | ||
Updated•7 years ago
|
Attachment #8888602 -
Flags: feedback?(dustin) → review?(dustin)
Comment 31•7 years ago
|
||
mozreview-review |
Comment on attachment 8888602 [details] Bug 1382911 - Custom action callback to trigger a bunch of jobs https://reviewboard.mozilla.org/r/159604/#review166772
Attachment #8888602 -
Flags: review?(dustin) → review+
Comment 32•7 years ago
|
||
Pushed by dmitchell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/36df21c0cba7 Custom action callback to trigger a bunch of jobs r=bstack,dustin
Comment 33•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/36df21c0cba7
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Updated•6 years ago
|
Product: TaskCluster → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•