1057260 - Retrigger success notification should display what type of build was retriggered

Status: VERIFIED FIXED

(Tree Management :: Treeherder, defect, P4)

Description

[:Gijs (he/him)]

STR:

1. click a build
2. Click the retrigger button ( |> ) on the bottom left

ER:
Notification like "Retriggering win32 mochitest-plain-5... SUCCESS"

AR:
"Retrigger SUCCESS"



Basically, if I retrigger lots of things, I want to be able to see what things I've recently retriggered, and how many times (I've not tried to experiment with doing multiples, so it could be that more work is needed to make that workable if notifications don't stack properly).

Comment 1

[Ed Morley [:emorley]]

Agreed, I find these messages essential in TBPL.

Comment 2

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I don't even see any notification at all when I click the retrigger button...

Comment 3

[Ed Morley [:emorley]]

(In reply to Wes Kocher (:KWierso) from comment #2)
> I don't even see any notification at all when I click the retrigger button...

I think that was bug 1071319.

Comment 4

[Ed Morley [:emorley]]

https://github.com/mozilla/treeherder-ui/blob/master/webapp/app/plugins/controller.js#L165
        $scope.retriggerJob = function() {
            thBuildApi.retriggerJob($scope.repoName, getRequestId());
        };

https://github.com/mozilla/treeherder-ui/blob/master/webapp/app/js/services/buildapi.js#L29
        retriggerJob: function(repoName, requestId) {
            $http({
...
            }).
            success(function(data, status) {
                notify(status, "retrigger");
            }).
            error(function(data, status) {
                notify(status, "retrigger");
            });
        },

Comment 5

[Ed Morley [:emorley]]

One problem is that for non-buildbot jobs we don't currently have a friendly job name, so just passing in the buildername in the controller.js call isn't a complete fix. (Though might be good enough in the meantime).

Comment 6

[Ed Morley [:emorley]]

Though for classifications we do:

    var saveClassification = function(job) {
...
                    thNotify.send("classification saved for " + job.platform + ": " + job.job_type_name, "success");

Comment 7

[Jonathan French (:jfrench)]

I hoping to implement more minimal version (eg. 'B', 'Cpp') identifiers into retrigger notifications as part of improvements in bug 1148664 - which will expire non-sticky notifications as new ones appear. So that may end up sufficiently addressing the UX issue here.

Comment 8

[Jonathan French (:jfrench)]

I've integrated the type into https://github.com/mozilla/treeherder-ui/pull/483 for bug 1148664, pending review.

Comment 9

[Jonathan French (:jfrench)]

My thinking is also that minimal types ('B', 'Cpp') are always readable within the current notification persistence of 4000ms, whereas really long, detailed job names are perhaps not. So hopefully it will serve as a reasonable solution.

Comment 10

[Jonathan French (:jfrench)]

Merged PR 483, I anticipate it will be present in production sometime next week.

Comment 11

[Jonathan French (:jfrench)]

Hi Gijs, so we have bug 1148664 live on production, and successfully Retriggered jobs will now show up similar to:

Retriggered job (Cpp)

Let me know after trying it out if this is sufficient for your needs.

Comment 12

[Phil Ringnalda (:philor)]

Or like "Retriggered job (6)".

Comment 13

[:Gijs (he/him)]

It seems like it would be better if these showed the platform and the full test name (as philor somewhat implied).

Comment 14

[Jonathan French (:jfrench)]

Ok, will have a look.

Comment 15

[Jonathan French (:jfrench)]

Attachment: treeherder-ui-PR#496

Please see above PR for status and review.

Comment 16

[William Lachance (:wlach)]

Comment on attachment 8599514 [details] [review]
treeherder-ui-PR#496

lgtm, I would ditch the parantheses as mentioned in the pr comment

Comment 17

[Jonathan French (:jfrench)]

Attachment: jobSearchStrName_example

Here's an example using a Job: 'name' I managed to dig up. There are other long examples in pushes I have looked at. 

Beyond the peculiarities of this particular example (ie. replicating 'Mochitest' in the jobSearchStr name) I think using the Job: 'name' for notifications as it currently exists will present a problem in readability.

Camd, wlach, what do you think? Is this example just a case of a job that needs better naming upstream? (and out of our control).

This was one of the reasons I wanted to go just with .job_type_symbol in the retrigger message.

Comment 18

[Jonathan French (:jfrench)]

(In reply to Jonathan French (:jfrench) from comment #17) 
> There are other long examples in pushes I have looked at. 

Now entered as bug 1160187.

Comment 19

[William Lachance (:wlach)]

My two cents it that I'd rather err on the side of being overly descriptive, even if the end result is not as "beautiful". ;)

Comment 20

[Jonathan French (:jfrench)]

That's cool with me. We can sort out the stuff for Job: name, separately in bug 1160187.

Comment 21

[Treeherder GitHub Bugbot]

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

https://github.com/mozilla/treeherder-ui/commit/1ff90c0e00c70a5fbb666d6e5d07ce6e60b3dd71
Bug 1057260 - Display full Job name in retrigger notification

Comment 22

[Jonathan French (:jfrench)]

Marking fixed per above merge. I will verify on the next push to stage/prod.

Comment 23

[Jonathan French (:jfrench)]

Verified fixed on production.

Comment 24

[Treeherder GitHub Bugbot]

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

https://github.com/mozilla/treeherder/commit/0790f427591d3af2e027a1b818a42026007071fb
Bug 1057260 - Display full Job name in retrigger notification