Only fetch target task for actions that require it

RESOLVED FIXED in Firefox 67

Status

task
P5
normal
RESOLVED FIXED
8 months ago
3 months ago

People

(Reporter: dustin, Assigned: ialokkumarsingh0, Mentored)

Tracking

Trunk
mozilla67

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(1 attachment)

taskcluster/taskgraph/actions/registry.py

    # fetch the target task, if taskId was given
    # FIXME: many actions don't need this, so move this fetch into the callbacks
    # that do need it
    if task_id:
        task = taskcluster.get_task_definition(task_id)
    else:                                                                                                                                                                                                                                                                                                                                                                    
        task = None

Basically, this means looking for the task implementations that look at their `task` argument and add this logic in those implementations, then remove it from registry.py, and removing the `task` argument to all action callbacks.
Severity: normal → enhancement
Priority: -- → P5
Assignee

Comment 1

4 months ago

Dustin, can you assign me?

Done!

Assignee: nobody → ialokkumarsingh0
Assignee

Comment 3

3 months ago

Dustin, I didn't find any taskgraph file in taskcluster how I can get started with this issue?

This bug is in the Firefox source code (https://hg.mozilla.org/mozilla-central/). You can find more information at https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Source_Code.

I'd suggest getting a checkout of that repository set up, but you don't need to go so far as building Firefox (you can if you want, though!). In the checkout, run ./mach taskgraph tasks. That runs the task-generation system that is how we determine what tasks to run for each push. https://firefox-source-docs.mozilla.org/taskcluster/taskcluster/taskgraph.html has more information. Once that's working, you've got everything set up to get started.

Next, have a look at the code in taskcluster/taskgraph/actions. It sets up a kind of registry, and registers a bunch of actions. All of them get a task argument, but not all of them use it. The snippet of code in the first comment of this bug takes the time to fetch the task for each one, though. So, please refactor so that only the actions that need that data fetch it.

Finally, to submit the patch, you'll need to set up a Phabricator account -- docs are at https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html#user-guide

Most of this set-up is one-time work that will be useful to you in working on other Firefox bugs -- and there are plenty of them to work on! So consider it a worthwhile investment of time.

Assignee

Comment 5

3 months ago

Dustin, I successfully clone the mozilla/central repo from mercurial and run the command ./mach taskgraph tasks all things work as expected. I saw the taskcluster/taskgraph/actions/ folder there is a bunch of files which contains callback and this callback trigger from register.py since we are fetching task-definition from taskid for every task but every task does not need this. I understand the problem(if not please suggest me if I am missing something) but one simple question How do I know which callback need this or which not? And How I can test the change I made(is it working right or not)?

You can tell the callbacks that don't need this information because they never reference the task variable. Testing will be a bit tricky, especially without the permissions to run some of these actions, but I can take care of that.

Comment 9

3 months ago
Pushed by dmitchell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bb243f4edd88
Only fetch target task for actions that require it r=dustin

Comment 10

3 months ago
bugherder
Status: NEW → RESOLVED
Last Resolved: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.