Closed Bug 1341727 Opened 3 years ago Closed 3 years ago

Action callbacks (created from actions.json templates) do not work

Categories

(Firefox Build System :: Task Configuration, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla54

People

(Reporter: wlach, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Ok, so I just did another round of testing of the action callback stuff. We're closer, but it's still not working:

https://treeherder.allizom.org/#/jobs?repo=try&revision=c49517153fd59e9c3e25790e07980882f4e2bb3e&selectedJob=73812537

Looking at the log I see the following errors:

[task 2017-02-22T17:40:54.811445Z] + ./mach --log-no-times taskgraph action-callback --pushlog-id=171879 --pushdate=1487781897 --project=try '--message=try: -b o -p linux64 -u mochitest-1 -t none' --owner=wlachance@mozilla.com --level=1 --base-repository=https://hg.mozilla.org/mozilla-central --head-repository=https://hg.mozilla.org/try/ --head-ref=c49517153fd59e9c3e25790e07980882f4e2bb3e --head-rev=c49517153fd59e9c3e25790e07980882f4e2bb3e --revision-hash=c49517153fd59e9c3e25790e07980882f4e2bb3e
[task 2017-02-22T17:40:55.156882Z] It looks like you passed an unrecognized argument into mach.
[task 2017-02-22T17:40:55.156933Z] 
[task 2017-02-22T17:40:55.157050Z] The taskgraph command does not accept the arguments: --pushlog-id=171879 --pushdate=1487781897 --project=try --message=try: -b o -p linux64 -u mochitest-1 -t none --owner=wlachance@mozilla.com --level=1 --base-repository=https://hg.mozilla.org/mozilla-central --head-repository=https://hg.mozilla.org/try/ --head-ref=c49517153fd59e9c3e25790e07980882f4e2bb3e --head-rev=c49517153fd59e9c3e25790e07980882f4e2bb3e --revision-hash=c49517153fd59e9c3e25790e07980882f4e2bb3e

I can confirm the same error locally. It looks like these options are added by default to the payload in `taskcluster/actions/registry.py`. Was this inadvertent or is something else broken?

(needinfo'ing jonas and bstack, who I think are most familiar with this)
Flags: needinfo?(jopsen)
Flags: needinfo?(bstack)
I'm going to defer to jonas on action-callback I think. It's interesting that the entirety of '--message=try: -b o -p linux64 -u mochitest-1 -t none'  is inside quotes though, right? Maybe that's tripping it up?
Flags: needinfo?(bstack)
(In reply to Brian Stack [:bstack] from comment #1)
> I'm going to defer to jonas on action-callback I think. It's interesting
> that the entirety of '--message=try: -b o -p linux64 -u mochitest-1 -t none'
> is inside quotes though, right? Maybe that's tripping it up?

Yes, that looks wrong, though even just pushlog-id seems to trip mach up:

Hammersmith:mozilla-central wlach$ ./mach --log-no-times taskgraph action-callback --pushlog-id=171879
It looks like you passed an unrecognized argument into mach.

The taskgraph command does not accept the arguments: --pushlog-id=171879
(In reply to William Lachance (:wlach) (use needinfo!) from comment #2)
> (In reply to Brian Stack [:bstack] from comment #1)
> > I'm going to defer to jonas on action-callback I think. It's interesting
> > that the entirety of '--message=try: -b o -p linux64 -u mochitest-1 -t none'
> > is inside quotes though, right? Maybe that's tripping it up?
> 
> Yes, that looks wrong, though even just pushlog-id seems to trip mach up:
> 
> Hammersmith:mozilla-central wlach$ ./mach --log-no-times taskgraph
> action-callback --pushlog-id=171879
> It looks like you passed an unrecognized argument into mach.
> 
> The taskgraph command does not accept the arguments: --pushlog-id=171879

Ok experimentally I managed to get this working via treeherder by just taking out all the command line arguments altogether:

https://hg.mozilla.org/try/rev/ce0d772b7333d748623eb8cd05d4708c7c29c52a
https://treeherder.allizom.org/#/jobs?repo=try&revision=0cedc34f0555f8fc260da8c9b4c84fd98c293c21&selectedJob=73857665

So I guess the question for jonas is what he meant to do with these command line arguments, and whether it's ok to just take them out.
> So I guess the question for jonas is what he meant to do with these command line
> arguments, and whether it's ok to just take them out.

I have no idea what those commandline args are good for :) I probably copy/pasted a bit too much there.
So we just need to remove them.
Flags: needinfo?(jopsen)
Comment on attachment 8840161 [details]
Bug 1341727 - Removed invalid arguments in action callback template.

https://reviewboard.mozilla.org/r/114660/#review116166

::: taskcluster/actions/registry.py:177
(Diff revision 1)
>                      'description': 'Task executing callback for action.\n\n---\n' + description,
>                  },
>                  'workerType': 'gecko-decision',
>                  'provisionerId': 'aws-provisioner-v1',
>                  'scopes': [
> -                    repo_scope,
> +                    'assume:repo:hg.mozilla.org/projects/{}:*'.format(parameters['project']),

This will only work for project branches, not for try or mozilla-central, etc.  I think you want to keep this hunk and the one above.
Attachment #8840161 - Flags: review?(dustin) → review-
Comment on attachment 8840161 [details]
Bug 1341727 - Removed invalid arguments in action callback template.

https://reviewboard.mozilla.org/r/114660/#review116174

::: taskcluster/actions/registry.py:177
(Diff revision 1)
>                      'description': 'Task executing callback for action.\n\n---\n' + description,
>                  },
>                  'workerType': 'gecko-decision',
>                  'provisionerId': 'aws-provisioner-v1',
>                  'scopes': [
> -                    repo_scope,
> +                    'assume:repo:hg.mozilla.org/projects/{}:*'.format(parameters['project']),

Indeed, I'm not sure of the motivation for this change? Just removing the arguments below makes things work for me.

::: taskcluster/mach_commands.py:314
(Diff revision 1)
> -        actions.trigger_action_callback()
> +        try:
> +            self.setup_logging()
> +            return actions.trigger_action_callback()
> +        except Exception:
> +            traceback.print_exc()
> +            sys.exit(1)

Shouldn't this change get its own commit?
Attachment #8840161 - Attachment is obsolete: true
Comment on attachment 8840161 [details]
Bug 1341727 - Removed invalid arguments in action callback template.

https://reviewboard.mozilla.org/r/114660/#review116608

::: taskcluster/actions/registry.py:227
(Diff revision 2)
>                          '/home/worker/bin/run-task', '--vcs-checkout=/home/worker/checkouts/gecko',
>                          '--', 'bash', '-cx',
>                          """\
>  cd /home/worker/checkouts/gecko &&
>  ln -s /home/worker/artifacts artifacts &&
> -./mach --log-no-times taskgraph action-callback """ + ' '.join([
> +./mach --log-no-times taskgraph action-callback """,

Nit: no space between action-callback and """
Attachment #8840161 - Flags: review?(wlachance) → review+
Comment on attachment 8840700 [details]
Bug 1341727 - Setup logging in ./mach taskgraph action-callback.

https://reviewboard.mozilla.org/r/115130/#review116774
Attachment #8840700 - Flags: review?(dustin) → review+
Please hit autoland :)
Keywords: checkin-needed
Fixing a nit
Keywords: checkin-needed
Fixing a nit
Comment on attachment 8840700 [details]
Bug 1341727 - Setup logging in ./mach taskgraph action-callback.

https://reviewboard.mozilla.org/r/115130/#review116858
Attachment #8840700 - Flags: review+
Comment on attachment 8840161 [details]
Bug 1341727 - Removed invalid arguments in action callback template.

https://reviewboard.mozilla.org/r/114660/#review116860
Comment on attachment 8840161 [details]
Bug 1341727 - Removed invalid arguments in action callback template.

https://reviewboard.mozilla.org/r/114660/#review116862
Comment on attachment 8840161 [details]
Bug 1341727 - Removed invalid arguments in action callback template.

https://reviewboard.mozilla.org/r/114660/#review116866
Attachment #8840161 - Flags: review+
Pushed by wlachance@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a6e54feb30bf
Removed invalid arguments in action callback template. r=dustin,wlach
https://hg.mozilla.org/integration/autoland/rev/bafc79da8b43
Setup logging in ./mach taskgraph action-callback. r=dustin,wlach
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a6e54feb30bf
https://hg.mozilla.org/mozilla-central/rev/bafc79da8b43
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.