1419957 - Remove things that are unused now pulse_actions is no more

Status: RESOLVED FIXED

(Tree Management :: Treeherder, enhancement, P2)

Description

[Ed Morley [:emorley]]

Pulse actions was decommissioned in bug 1379172.

I've lost track of exactly what Treeherder pieces were used to support it, but I think there might have been Pulse messages sent out that are no longer required etc.

Comment 1

[Armen [:armenzg]]

There was a message sent everytime an action was requested within the UI.
Maybe even one after a developer would select jobs and then hit submit.

Comment 2

[Ed Morley [:emorley]]

So I have a WIP locally to remove things that seem unused, however I'm getting a little confused.

It was my understanding that pulse_actions was the only thing listening to the {job,push,push-runnable-job}-action-messages, and as such with that switched off I can remove anything relating to it.

However after removing those pieces, I can't seem to find anywhere in the Treeherder frontend that handles Taskcluster _retriggers_ (the buildbot job retriggers are handled by self-serve, and those parts look fine, plus I can see Taskcluster parts for backfilling/custom actions etc).

Are taskcluster retriggers only performed via "Edit and Retrigger" and not via the normal retrigger button?

Brian, I don't suppose you know more? :-)

My WIP is here:
https://github.com/mozilla/treeherder/compare/rm-pulse-actions-support

Comment 3

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/2982

Comment 4

[Brian Stack [:bstack]]

We held off on porting retrigger and cancel to action tasks (they are currently handled by mozilla-taskcluster via those messages). I believe the reason we've not landed them yet is that we're waiting for treeherder to use the new auth0 auth stuff so that people don't have to log back in every 3 days. We felt that retrigger and cancel were more frequently used and so making more people do the log-out/log-in dance was not desirable. I have a branch sitting around somewhere that ports them to action tasks in treeherder if we want to land it sooner rather than later!

Comment 5

[Greg Arndt [:garndt]]

We also held off temporarily to identify a better flow for people trying to cancel or retrigger a task for which they do not have scopes for.

Currently action tasks, including the new retrigger action task, require the user signed in to possess the scopes for the decision task that would create it.  In other words, if you try retriggering  task scheduled on a level repo, you must have level 3 scopes.  For other actions, such as backfilling and adding new jobs, we have maintained that one must possess those scopes to schedule those tasks.  However, this might be more disruptive for retriggers/cancels.

There are a couple of options:
1. Maintain the current model that the user must posses the scopes for tasks they are scheduling/cancelling
2. Have a deputy schedule these tasks for users that do not have such scopes but are some how recognized by treeherder.  This is essentially what mozilla-taskcluster does now.  Users are not allowed to alter the original task, only schedule a new run of it.  3. Use parameterized hooks (work in progress) to wrap action tasks.  Users will need to the scope to trigger a hook but do not need the scope to edit a hook task definition (meaning, they can't edit the definition of the original task).

#2 is what we do now with mozilla-taskcluster, so I don't think we want to continue with this model because it requires one service to have a broad range of scopes.  #3 requires the work from our Outreachy program that will go on for the next few months.

Comment 6

[Ed Morley [:emorley]]

Ah thank you for the clarification - I think the piece I was missing was that the pulse messages emitted from Treeherder were/are being listened to not just by pulse_actions, but mozilla-taskcluster too. There's no immediate rush to move off of mozilla-taskcluster, it just reduces the scope of the cleanup I can perform in this bug for now.

I really should try and find the time at some point to learn more about how the taskcluster pieces fit together, so I wouldn't need to keep on asking daft questions hehe.

Brian, just to avoid any breakage - I don't suppose you could confirm I'm about to remove the correct things.

Looking here:
https://github.com/taskcluster/mozilla-taskcluster/blob/e0c9771a31bf2b271a8879048c33417132432c8e/src/treeherder/action_handler.js#L91-L95

It seems like these message types are completely unused (since they were presumably for pulse_actions only):
https://github.com/mozilla/treeherder/blob/5dd4e3b01f4e9d6f4ef8058386bbc09e3f416625/schemas/push-action-message.json
https://github.com/mozilla/treeherder/blob/5dd4e3b01f4e9d6f4ef8058386bbc09e3f416625/schemas/push-runnable-job-action-message.json

And for this one, I believe I can remove the backend parts that generate the `backfill` event, since only `retrigger` and `cancel` are used (presumably I'll have to leave the schema as is to avoid bumping the version):
https://github.com/mozilla/treeherder/blob/5dd4e3b01f4e9d6f4ef8058386bbc09e3f416625/schemas/job-action-message.json#L30

Comment 7

[Brian Stack [:bstack]]

I believe you are correct!

Comment 8

[Brian Stack [:bstack]]

Happy to talk about how the TC bits fit together anytime. Perhaps we can take the results of us talking and turn it into more docs if that makes sense.

Comment 9

[Ed Morley [:emorley]]

Comment on attachment 8931440 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/2982

I've updated the PR to only remove the Pulse messages/action types that I believe are unused. I don't believe the frontend parts have any test coverage, so some closer eyes on those (and thoughts on the conditional marked TODO) would be helpful :-)

Comment 10

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/958cc079a77db8f7162b126e86df45ac29a7b20f
Bug 1419957 - Remove old style backfill/add new job features (#2982)

Since everything but standard retrigger/cancellation is now handled
client-side by tcactions, making the backend parts that provided pulse
messages to pulse_actions redundant (since it was decommissioned in
bug 1379172).