525464 - Show tree status in tinderboxpushlog (open, closed, ...)

Status: RESOLVED FIXED

(Tree Management Graveyard :: TBPL, defect)

Description

[Marco Bonardo [:mak]]

Tinderboxpushlog is a really good place to quickly check status of a tree, but it does not report the OPEN/CLOSED/METERED status of the tree.
To see that you have to open the complete tinderboxes page that can be quite slow and looks like a too "heavy" task to just check a string value.

Would it be possible to add such information on tinderboxpushlog pages?

Comment 2

[Justin Wood (:Callek)]

mak, any chance we have a mockup of how this should look?

I might have time/be willing to do it, if I had some UX guidance.

Comment 3

[Marco Bonardo [:mak]]

I'm not the right man to get a mockup from :)
I'd guess an indicator in the top part would be fine, something simple like a semaphoric indicator (green = open, red = closed, orange any other status) with the complete status message in a tooltip.
Even better would be a panel with complete status message, today's sheriff, buildduty, sum up of boxes load... so like a panel with advanced stats.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Mockup

This displays the status message which we currently display on tinderbox, including the "openness status" but also the short status message that goes along with it and which can contain very important information, such as "closed for bug 123456", "Closed for compartments landing" or "Beta 7 blockers only".

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Would love to see this fixed. The lack of this information is the only reason I still have bookmarks to the old tinderbox.

An important part of the mockup is the fact that the status, and matching message, is *always* displayed. I believe this will reduce the amount of people checking in when they shouldn't.

Comment 6

[Phil Ringnalda (:philor)]

The sticking point is not actually "where could we stick it in tbpl," it's "where can we get it?"

The tree status is, from the outside world's perspective, just a bit of HTML in the midst of 3.2MB of HTML. I very much do not want tbpl downloading that 3.2MB of HTML on every refresh, to parse out one or two words.

From inside tinderbox's guts, it's just a matter of adding a new output format that only does the http://mxr.mozilla.org/mozilla/source/webtools/tinderbox/showbuilds.pl?mark=186-189#186 chunk, so we could shove that in an iframe, but inside tinderbox's guts is as foul and putrid a place as you could hope to not find yourself, so I'm not holding my breath for that hero to show up.

Or, someone could write a dead-simple web app that just has a three-value select for status and a textarea for a reason, that will cough it up as json for us and as an html fragment for an iframe on tinderbox, but that's still not the sort of thing that an outsider can knock together and have deployed in a couple of evening hours, the way we operate.

Comment 7

[Arpad Borsos [:Swatinem]]

(In reply to comment #6)
> Or, someone could write a dead-simple web app that just has a three-value
> select for status and a textarea for a reason, that will cough it up as json
> for us and as an html fragment for an iframe on tinderbox, but that's still not
> the sort of thing that an outsider can knock together and have deployed in a
> couple of evening hours, the way we operate.

Lets just do that on tbpl.m.o and somehow write a tinderbox script that just fetches the same json that tbpl is.
One thing that comes to my mind though is how to easily do ldap auth?

Comment 8

[Axel Hecht]

FTR, I don't expect our reporting story to end with tbpl. It's a better story than tinderbox, for sure, but it's still far away from showing what the builds actually do. Which, if it wasn't so confusing, would be great.

Long story short, not sure how much we should invest to put a crotch from the left onto the right.

Lastly, tree status doesn't require ldap, but the sheriff password.

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

You could download this url:

http://tinderbox.mozilla.org/admintree.cgi?tree=Firefox

It'll require an unusually ugly screen scrape since you have to first scrape for the second <textarea> on that page, and then scape the contents of *that* for the relevant message.

But it doesn't require any passwords, only once you submit its forms which we obviously aren't interested in here.

However that page is still 200kB and takes *forever* to download, so it might overload the server to hit it too much.

Comment 10

[Robert Helmer [:rhelmer]]

(In reply to comment #6)
> From inside tinderbox's guts, it's just a matter of adding a new output format
> that only does the
> http://mxr.mozilla.org/mozilla/source/webtools/tinderbox/showbuilds.pl?mark=186-189#186
> chunk, so we could shove that in an iframe, but inside tinderbox's guts is as
> foul and putrid a place as you could hope to not find yourself, so I'm not
> holding my breath for that hero to show up.

I don't think it's quite as melodramatic as that... file a bug on Tinderbox and specify the output format you want, we can find someone to do it.

This still leaves Tinderbox in charge of tree status, making some other source authoritative (perhaps bug 553549) still needs to happen at some point.

Comment 11

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Filed bug 611373

Comment 12

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Show tree status in tinderboxpushlog

Comment 13

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Will this just poll the status once? Or once every few minutes?

It'd be great if it could be updated every few minutes just like the status for the various builds.

Comment 14

[Arpad Borsos [:Swatinem]]

Comment on attachment 493057 [details] [diff] [review]
Show tree status in tinderboxpushlog

Just as a driveby comment: please use jquery functions as is the preferred tbpl style instead of standard dom. Also Jonas is right, this probably needs to be updated in some interval (as does the sheriff info btw)

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Though the sheriff info only really needs to be updated once a day (I think it switches at PST midnight). Or once every few hours if you want to skip the timezone calculations.

Comment 16

[Arpad Borsos [:Swatinem]]

I just noticed when looking at http://tinderbox-stage.mozilla.org/Firefox/status.html that it does not include a trailing </p>.
I think someone with the sheriff password should make sure the status contains valid html instead of tbpl trying to work around it.

Comment 17

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to comment #14)
> (as does the sheriff info btw)

File a bug, please.

(In reply to comment #16)
> I just noticed when looking at
> http://tinderbox-stage.mozilla.org/Firefox/status.html that it does not include
> a trailing </p>.
> I think someone with the sheriff password should make sure the status contains
> valid html instead of tbpl trying to work around it.

The HTML is valid. It isn't XML, nor should it be.

I'll probably be looking at the other comments later this week.

Comment 18

[Arpad Borsos [:Swatinem]]

(In reply to comment #17) 
> The HTML is valid. It isn't XML, nor should it be.

I just want to make sure tbpl generates html that is also xhtml compatible.

Comment 19

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to comment #18)
> (In reply to comment #17) 
> > The HTML is valid. It isn't XML, nor should it be.
> 
> I just want to make sure tbpl generates html that is also xhtml compatible.

I'm not sure what you're talking about. tbpl doesn't generate markup.

Comment 20

[Markus Stange [:mstange]]

Comment on attachment 493057 [details] [diff] [review]
Show tree status in tinderboxpushlog

Yeah, I don't know either what Arpad is looking for. Thanks for your work, this is great!
I'll convert it to jQuery before checking in later today if you don't mind.

Comment 21

[(no longer active)]

(In reply to comment #16)
> I just noticed when looking at
> http://tinderbox-stage.mozilla.org/Firefox/status.html that it does not include
> a trailing </p>.
> I think someone with the sheriff password should make sure the status contains
> valid html instead of tbpl trying to work around it.

I _could_ do that for you, but I won't!

We have to accept the fact that the markup there won't be valid XHTML, because it's going to be entered by humans (who usually have more pressing matters to worry about, such as a burning tree ;)

Comment 22

[Markus Stange [:mstange]]

Yeah, the only thing that needs to parse that HTML is the browser's innerHTML setter, and we can rely on that being fairly robust. :)

http://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/rev/533ef55dbe4f

Comment 23

[Reed Loden [:reed]]

Uh, you should not be polling tinderbox-stage for this stuff. tinderbox and tinderbox-stage aren't using the same data source for tree status, so the status is likely to be wrong.

Comment 24

[(no longer active)]

I backed out the patch.  It scared the crap out of me when I saw this message while sitting at a pub having a drink:

The tree is FUZZY PINK MONKEYS - see your fate. 

Which is the contents of <http://tinderbox-stage.mozilla.org/Firefox/status.html>.

This should not land until the correct page has been set up on tinderbox.m.o, and the patch has been modified to point to the real server.

Comment 25

[(no longer active)]

(In reply to comment #22)
> Yeah, the only thing that needs to parse that HTML is the browser's innerHTML
> setter, and we can rely on that being fairly robust. :)

Indeed, and if not, you can always blame me or bz ;-)

Comment 26

[Markus Stange [:mstange]]

(In reply to comment #24)
> This should not land until the correct page has been set up on tinderbox.m.o,
> and the patch has been modified to point to the real server.

Oh, okay.

I've updated the server. (Ehsan, you do have access to it, too, don't you?)

Comment 27

[(no longer active)]

(In reply to comment #26)
> I've updated the server. (Ehsan, you do have access to it, too, don't you?)

Yes, and I just verified that my access really works, so in the future I can update the code myself.  Thanks for taking care of it this time...

Speaking of which, why are we not running a cron job every 5 minutes to update the code?

Comment 28

[Reed Loden [:reed]]

http://tinderbox.mozilla.org/Firefox/status.html is live now. The patch can be relanded with the URL changed from tinderbox-stage to tinderbox.

Comment 29

[Markus Stange [:mstange]]

http://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/rev/bc58c4a26094