Closed Bug 366784 Opened 18 years ago Closed 6 years ago

Make showbuilds.cgi with no parameters more informative

Categories

(Webtools Graveyard :: Tinderbox, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: crowderbt, Assigned: cls)

References

Details

Attachments

(4 files, 10 obsolete files)

The main tinderbox showbuilds.cgi page could be a large table showing the color-coded status of each subtree:
,______________________________________________________
|__Tree______|_Status_|______ Subtrees________________|
|_Aviary-1.0_|__Open__|_Red_|_Orange_|_Green__|_Green_|
|__Browser___|_Closed_|_Red_|_Green__|_Orange_|_Green_|
|__Firefox___|__Open__|_Red_|_Green__|_Green__|_Green_|
-------------------------------------------------------

... and so on, with each subtree datum being a clickable link, as well.  If this is not clear, I can do a better mock-up.  It would be great to get a 50,000-ft view on this page.

Also:  It would be cool to be able to disable viewing of certain tinderboxen in this table, using some sort of search or filter criteria in the URL (built from check-boxes on the table?).

,________________________________________________________
|_|__Tree______|_Status_|______ Subtrees________________|
|X|_Aviary-1.0_|__Open__|_Red_|_Orange_|_Green__|_Green_|
|_|__Browser___|_Closed_|_Red_|_Green__|_Orange_|_Green_|
|_|__Firefox___|__Open__|_Red_|_Green__|_Green__|_Green_|
---------------------------------------------------------
[=Disable Viewing=]
Attached file simple mockup (obsolete) —
crappy HTML mockup based on crowder's ASCII. If we want each column to be sortable, I suggest making separate columns for build status (green, red, orange).

To preserve user's selected trees and sort order, I think that the selection action should build a URL that the user can bookmark (as opposed to using cookies, etc).
Here is some rationale for this one:

* probably don't need sorting; just bring the most important to the top
* limit the trees shown by default, have something that fits on a normal screen by default, and let users turn more on or off as desired
Attachment #251312 - Attachment is obsolete: true
Attachment #251339 - Attachment is patch: false
Attachment #251339 - Attachment mime type: text/plain → text/html
we have about 50 trees. you don't want to see most of them.
some trees have 10 or more boxes.

there was a version of shwobuilds.cgi which had some reasonable panel support for multiple trees (check out panel and some of the related alternate views).
(In reply to comment #3)
> we have about 50 trees. you don't want to see most of them.
> some trees have 10 or more boxes.

Right. I think showing a screenfull is a reasonable default, and more can be turned on if desired.

> there was a version of shwobuilds.cgi which had some reasonable panel support
> for multiple trees (check out panel and some of the related alternate views).

I'll take a look, thanks! 

I'd like to first get to a mockup that we'd be happy to have on tinderbox.mozilla.org, then figure out exactly the right way to implement that in tinderbox (e.g. should use cached data, allow turning on/off trees, etc).

I've got some visual touches to add to attachment 251339 [details], but I think this is the reasonable default we should go for. Thoughts?
If you're going to arbitrarily limit the data shown, then I fail to see how this is much better than the current main tinderbox page that shows multiple panels.  There's also the issue of the performance impact for a large number of trees (t.m.o) if these pages are dynamically generated (which they would almost have to be if you want the layout to be user control).  My primary concern is about the (unnecessary?) complexity that this will add to the tbox1 code that we've been working to clean up.
(In reply to comment #5)
> If you're going to arbitrarily limit the data shown, then I fail to see how
> this is much better than the current main tinderbox page that shows multiple
> panels.  There's also the issue of the performance impact for a large number of
> trees (t.m.o) if these pages are dynamically generated (which they would almost
> have to be if you want the layout to be user control).  My primary concern is
> about the (unnecessary?) complexity that this will add to the tbox1 code that
> we've been working to clean up.

The main goal is to provide a more useful overview of the overall project status. I would certainly want to use cached data for this (e.g. quickparse.txt) and not dynamically generate it each time.

The dynamically generated page (with user-selected tinderboxes) should have a similar performance impact to what we have now, where people create their own custom pages by pulling panel.html from various trees into iframes.

I definitely want to keep things simple on the implementation side, I haven't looked at showbuilds.cgi at all yet so maybe I am making unreasonable assumptions about what we have cached. I was planning on depending on any cached data we have available about available trees and tree status, rather than rebuilding that info on each hit.

What I really want to get out of all this is a more useful page when users go to http://tinderbox.mozilla.org, what is there now is more of an administrative interface, and not useful to developers who just want to see if they can check in or not. The current showbuilds.cgi should definitely be available still, but the default page should give relevant info to the most users. Whatever the correct path to implement that is, is fine with me. For example this could be a separate page on a separate server for all I care; I am assuming that we can make it more performant if it's on the tinderbox server and has local access to all cached data though.
What I'd really, really like is to be able to also serve this data up as an RSS feed, so users can poll and be notified when a tree they care about changes state. That doesn't necessarily need to be tied up in this bug, but I want to keep it in mind implementation-wise.
File a separate bug on the RSS feed.  It should be relatively easy to generate the RSS file when the rest of the static pages are generated.
(In reply to comment #5)
> If you're going to arbitrarily limit the data shown, then I fail to see how
> this is much better than the current main tinderbox page that shows multiple
> panels.  There's also the issue of the performance impact for a large number of
> trees (t.m.o) if these pages are dynamically generated (which they would almost
> have to be if you want the layout to be user control).  My primary concern is
> about the (unnecessary?) complexity that this will add to the tbox1 code that
> we've been working to clean up.

How about this - we could write a static page out to index.html (this is just a redirect to showbuilds.cgi), and do all the "hiding" stuff in Javascript.

I actually don't care about the hiding/sorting/etc. so much, as making the front page of tinderbox more useful. Let's leave that out of the initial version, and leave it up to someone to implement in JS.

Does a static index.html (generated periodically by tinderbox when trees are removed/added), that lists the tree name, tree state, and individual build status (as in the mockups) make sense?
(In reply to comment #9)
> (In reply to comment #5)
> Does a static index.html (generated periodically by tinderbox when trees are
> removed/added), that lists the tree name, tree state, and individual build
> status (as in the mockups) make sense?

This is assuming we implement it by e.g. pulling in panels in iframes, or something along those lines; the index.html would not need to be updated on each build status change, only if a tree is added or removed.
This is a screen shot of a working index.html page I made for Tinderbox, feedback is welcome
(In reply to comment #11)
> This is a screen shot of a working index.html page I made for Tinderbox,
> feedback is welcome

You're always free to attach a patch to make it this pretty! :)
These are the files used to create the provided screen shot. (Note that to load the tinderbox panels locally for debug you will need to accept the request for "UniversalBrowserRead" - this is expected behavior)
(In reply to comment #13)
> These are the files used to create the provided screen shot.

<td class="bgcolor"><a name="Sunbird"</a></td>

<a> is broken there.
fixed a bug.. (thanks to Reed Loden for pointing it out)
Attachment #285414 - Attachment is obsolete: true
(In reply to comment #13)
> Created an attachment (id=285414) [details]
> Working mockup of the index page - files
> 
> These are the files used to create the provided screen shot. (Note that to load
> the tinderbox panels locally for debug you will need to accept the request for
> "UniversalBrowserRead" - this is expected behavior)

This looks great! The UniversalBrowserRead is just to do cross-site XmlHttpRequest, so not needed if this was on tinderbox.mozilla.org right?

Only feedback from me at this point is to drop the following obsolete projects:

Aviary
BlueBird
Browser
Phoenix
Testing

Anyway I think this is a very good start, we should make this index.html on Tinderbox ASAP and further improve from there (adding more JSON output modes to Tinderbox that this could take advantage of, for instance).
(In reply to comment #16)
> (In reply to comment #13)
> > Created an attachment (id=285414) [details] [details]
> > Working mockup of the index page - files
> > 
> > These are the files used to create the provided screen shot. (Note that to load
> > the tinderbox panels locally for debug you will need to accept the request for
> > "UniversalBrowserRead" - this is expected behavior)
> 
> This looks great! The UniversalBrowserRead is just to do cross-site
> XmlHttpRequest, so not needed if this was on tinderbox.mozilla.org right?
> 

Exactly

> Only feedback from me at this point is to drop the following obsolete projects:
> 
> Aviary
> BlueBird
> Browser
> Phoenix
> Testing
> 

done, uploading new files

> Anyway I think this is a very good start, we should make this index.html on
> Tinderbox ASAP and further improve from there (adding more JSON output modes to
> Tinderbox that this could take advantage of, for instance).
> 

That sounds like a plan :)
Fixed up files as per Rob Helmers comments. Also removed any debugging code so this will no longer run locally using UniversalBrowserRead. (See obsolete attachments for that)

This is only an index.html page that loads the panels generated by Tinderbox
Attachment #285416 - Attachment is obsolete: true
Attachment #285426 - Flags: review?(cls)
Comment on attachment 285426 [details]
Working mockup of the index page - files

I like the mockups but we need to figure out how to dynamically generate that page.  The hardcoded values are not acceptable for the general case and requires that justdave or someone with server access modify the page for the mozilla.org case.

A side point: Until this point, tbox1 has not used any "advanced" html features (iframes, css, etc).  I'm not sure if that's a big deal for anyone anymore but I thought that I'd point that out.
Attachment #285426 - Flags: review?(cls) → review-
(In reply to comment #19)
> (From update of attachment 285426 [details])
> I like the mockups but we need to figure out how to dynamically generate that
> page.  The hardcoded values are not acceptable for the general case and
> requires that justdave or someone with server access modify the page for the
> mozilla.org case.


What if we published the list of trees as JSON from showbuilds.cgi? As far as I can tell there's no way to get the contents of make_tree_list() 

That list could be available dynamically from showbuilds.cgi, as well as dumped to a static file whenever the list is changed.


> A side point: Until this point, tbox1 has not used any "advanced" html features
> (iframes, css, etc).  I'm not sure if that's a big deal for anyone anymore but
> I thought that I'd point that out.
> 

You mean, there's no way to externally deliver the list of trees from an existing .cgi?  No, there's not.  Why would that be necessary?  index.html (tinderbox.html in the attachment) will have to be generated and whatever generates the file would call make_tree_list().  It would be just like the existing static pages.
Attached patch dump tree_list as json (obsolete) — Splinter Review
(In reply to comment #21)
> You mean, there's no way to externally deliver the list of trees from an
> existing .cgi?  No, there's not.  Why would that be necessary?  index.html
> (tinderbox.html in the attachment) will have to be generated and whatever
> generates the file would call make_tree_list().  It would be just like the
> existing static pages.

Here's a quick example of what I mean; if we add something like this, then we do not need to special-case generating an index.html, index.html can simply be a static page that pulls this info from tinderbox instead of being a generated file.

The practical advantage is that other people can write applications that pull data from tinderbox programmatically. Someone with no Perl or Tinderbox-server-specific skills can easily make use of the data this way, and it requires that the client pull less data from the server and do a larger share of the work.

This patch also refactors the JSON dumping to it's own function, and makes it real JSON (as per bug 399190), which is not directly related to this patch but makes it easier to read I think.
Attachment #285543 - Flags: review?
Comment on attachment 285543 [details] [diff] [review]
dump tree_list as json

I see how that patch exposes the tree list to external json users but you're missing the glue that actually improves the default index.html output.  Just having the tree output in json format is not enough.

Having the json output is really a tangential improvement not a core requirement for this feature.  I'm not seeing how using json requires the client to pull less data for index.html than having the page generated.  To the contrary, using json looks like it would require extra trips to the server to get all of the data that would otherwise be available from a single static pull.  What am I missing here?

Nitpicks about the patch:
You should follow the existing style in showbuilds.cgi for the different modes.
show_tree_list_json() should be another do_<mode> function in showbuilds.pl since it isn't used globally.  Otherwise, I think the refactoring is good.
Attachment #285543 - Flags: review? → review-
(In reply to comment #23)
> (From update of attachment 285543 [details] [diff] [review])
> I see how that patch exposes the tree list to external json users but you're
> missing the glue that actually improves the default index.html output.  Just
> having the tree output in json format is not enough.
> 
> Having the json output is really a tangential improvement not a core
> requirement for this feature.  I'm not seeing how using json requires the
> client to pull less data for index.html than having the page generated.  To the
> contrary, using json looks like it would require extra trips to the server to
> get all of the data that would otherwise be available from a single static
> pull.  What am I missing here?


Yes it would require an extra round-trip compared to generating the HTML. For this patch I was optimizing more for making life easier for someone who wanted to grab Tinderbox data from Javascript (or another external application for that matter), and trying not to introduce a ton of change on the Tinderbox server side.

If we wanted to optimize for roundtrips, I would say to put all of the data that makes up the panel.html files into one set of JSON data, then there'd only need to be one trip to get the index and script, and one for each time the script wants to check for data. 

Currently we do a roundtrip for each panel.html so I think that'd be a bigger win overall anyway.
cls and I discussed on irc; there are two obvious ways to go from here:

1) generate index.html from Tinderbox as-needed (technically I guess we'd generate panelLoad.js unless the script is put into the index.html), and populate the tree names there

2) generate a JSON file from Tinderbox as-needed which contains each tree name and the equivalent data of each $tree/panel.html file

I'm going to start a separate bug for #2, as I think that'd be useful in any case. Dominic what do you think?
(In reply to comment #25)
> cls and I discussed on irc; there are two obvious ways to go from here:
> 
> 1) generate index.html from Tinderbox as-needed (technically I guess we'd
> generate panelLoad.js unless the script is put into the index.html), and
> populate the tree names there
> 
> 2) generate a JSON file from Tinderbox as-needed which contains each tree name
> and the equivalent data of each $tree/panel.html file
> 
> I'm going to start a separate bug for #2, as I think that'd be useful in any
> case. Dominic what do you think?
> 

I'd say providing JSON output first will simplify the process greatly.  I've fixed up your patch to make it use quickparse for leaner JSON output.  It's filed under bug 400708
The back-end perl code for outputting better information to the Tinderbox index page.
Attachment #285426 - Attachment is obsolete: true
Attachment #297395 - Flags: review?(cls)
Attachment #297395 - Flags: review?(cls)
brain child of rhelmer.
Attachment #297395 - Attachment is obsolete: true
Attachment #300959 - Flags: review?(cls)
Comment on attachment 300959 [details] [diff] [review]
Perl code to allow json output for entire tinderbox. Not just one tree.

There are a couple of style nitpicks.  We're moving away from using & on perl functions as they stop perl from doing any type-checking on functions.  And since the presentation of the json tree is only used by showbuilds.cgi, show_tree_list_json() should be moved to showbuilds.pl.

I'm not certain why you need the 'next unless grep' bit in show_tree_list_json().  It looks like it was copied from do_quickparse().  It isn't needed here because make_tree_list() returns all of the available trees rather than a list given by the user so it doesn't need to be checked. Also, you're storing the tree_state & bonsai_tree name (what about viewvc?) but you never return that information.

Actually, after loading all of the individual build data for each tree, you're just returning the summary data from quickparse.txt ?  I don't like the idea of reading quickparse.txt in general, but especially not after loading the full set of data via tb_load_treedata().
Attachment #300959 - Flags: review?(cls) → review-
Assignee: morgamic → dominic.baranski
Attached patch v1.0 (obsolete) — Splinter Review
Getting away from the json tangent, here's a patch that provides the tree summary feature using dynamic data and the mockup as a guideline.
Assignee: dominic.baranski → cls
Attachment #285543 - Attachment is obsolete: true
Attachment #300959 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Depends on: 415502
Attached patch V2.0 (obsolete) — Splinter Review
A nicer version of cls's demo.  I'm thinking that this new front page should not overwrite the current front page (at least until it's more accepted by the community). I'll probably incorporate a link to this new front page from the current showbuilds.cgi page to do that.
Attached patch v2.1 (obsolete) — Splinter Review
Merge local changes with Dominic's patch:
* Refresh the summary page every 5 mins
* Change page header to 'Tinderbox Tree Summary'
* Set status to 'Not Available' instead of 'Unknown' when open/closed status cannot be determined
* Remove unnecessary local $color variable
* Add link to summary page in the waterfall footer
Attachment #301209 - Attachment is obsolete: true
Attachment #306651 - Attachment is obsolete: true
Attached patch v2.2Splinter Review
* Adds admin links to summary page.
* Adds footer links to admin page to help page navigation

I'm open to suggestion for the 'admin page' links on the summary page.  I'm not sold on the current implementation.  I thought about adding the admin links to the bottom of the page like the showbuilds links at the top but I didn't want to require people to scroll to the bottom of a potentially long page to search for a string in a long list of trees.
Attachment #307570 - Attachment is obsolete: true
Attachment #307824 - Flags: review?(justdave)
Attachment #307824 - Flags: review?(justdave) → review?(reed)
v2.2 has been checked into tbox1_20080527_cls_branch
Comment on attachment 307824 [details] [diff] [review]
v2.2

This looks ok (save the flagrant use of HTML4 that makes me want to cry), but where's defaultStyle.css?
In patch v2.0.
Attachment #307824 - Flags: review?(reed) → review+
Attached patch v3.0Splinter Review
In a couple of other bugs (like bug 451297), we want to use CSS so we need to clean up defaultStyle.css so that it can be used on all pages.  I know just enough CSS to get myself in trouble so let me know if there's a better way to do this and keep at least the colors customizable in defaultStyle.css .
v3.0 has been checked in on tbox1_20080527_cls_branch
Product: Webtools → Webtools Graveyard
Tinderbox isn't maintained anymore. Closing.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: