Bug 107659 – QueueStatusServer needs pages to display historical queue data

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Alan Cutter]

QueueStatusServer is only recording queue data at the moment, a page handler needs to be written to present the information in a user friendly way.

[Alan Cutter]

Created attachment 184402 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 184402 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184402&action=review

> Tools/ChangeLog:8
> +        Created a /queue-charts/<queue-name> handler to present queue and patch data using Google Chart Tools.

I might have just called it charts/ :)

> Tools/QueueStatusServer/config/charts.py:31
> +# How far back to view the history, specified in seconds.
> +default_view_range = 60 * 60 * 24
> +patch_log_limit = 500

Eventually we might want these to be get-parameters to the chart, but OK.

[WebKit Review Bot]

Attachment 184402 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/QueueStatusServer/app.yaml', u'Tools/QueueStatusServer/config/charts.py', u'Tools/QueueStatusServer/filters/webkit_extras.py', u'Tools/QueueStatusServer/handlers/queuecharts.py', u'Tools/QueueStatusServer/handlers/queuestatus.py', u'Tools/QueueStatusServer/index.yaml', u'Tools/QueueStatusServer/main.py', u'Tools/QueueStatusServer/model/queuelog.py', u'Tools/QueueStatusServer/stylesheets/charts.css', u'Tools/QueueStatusServer/templates/queuecharts.html', u'Tools/QueueStatusServer/templates/queuestatus.html']" exit_code: 1
Tools/QueueStatusServer/model/queuelog.py:69:  [QueueLog._get_or_create_txn] Class 'QueueLog' has no 'get_by_key_name' member  [pylint/E1101] [5]
Tools/QueueStatusServer/handlers/queuecharts.py:34:  No name 'appengine' in module 'google'  [pylint/E0611] [5]
Tools/QueueStatusServer/handlers/queuecharts.py:35:  No name 'appengine' in module 'google'  [pylint/E0611] [5]
Tools/QueueStatusServer/handlers/queuecharts.py:50:  [QueueCharts.get] Instance of 'QueueCharts' has no 'error' member  [pylint/E1101] [5]
Tools/QueueStatusServer/handlers/queuecharts.py:64:  [QueueCharts.get] Instance of 'QueueCharts' has no 'response' member  [pylint/E1101] [5]
Tools/QueueStatusServer/handlers/queuecharts.py:130:  [QueueCharts._get_timestamp] Instance of 'QueueCharts' has no 'request' member  [pylint/E1101] [5]
Tools/QueueStatusServer/handlers/queuecharts.py:137:  [QueueCharts._get_view_range] Instance of 'QueueCharts' has no 'request' member  [pylint/E1101] [5]
Total errors found: 7 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alan Cutter]

(In reply to comment #2)
> (From update of attachment 184402 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=184402&action=review
> 
> > Tools/ChangeLog:8
> > +        Created a /queue-charts/<queue-name> handler to present queue and patch data using Google Chart Tools.
> 
> I might have just called it charts/ :)
> 

I was leaving room to add bot-charts. (:

> > Tools/QueueStatusServer/config/charts.py:31
> > +# How far back to view the history, specified in seconds.
> > +default_view_range = 60 * 60 * 24
> > +patch_log_limit = 500
> 
> Eventually we might want these to be get-parameters to the chart, but OK.

view_range is a get-parameter already. It is currently a future task to make a decent UI for selecting the range and the time to view from on the page.

[Alan Cutter]

Comment on attachment 184402 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184402&action=review

> Tools/QueueStatusServer/handlers/queuecharts.py:86
> +                    "process_duration": patch_log.process_duration,
> +                    "retry_count": patch_log.retry_count,
> +                    "status_update_count": patch_log.status_update_count,
> +                    "wait_duration": patch_log.wait_duration,

These two durations should be converted to minutes.

[Alan Cutter]

Created attachment 184426 [details]
Patch

[Alan Cutter]

A "pre-release" of this patch can be viewed at: http://107659.webkit-commit-queue.appspot.com/queue-charts/chromium-ews
Feel free to suggest improvements to the interface.

[WebKit Review Bot]

Attachment 184426 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/QueueStatusServer/app.yaml', u'Tools/QueueStatusServer/config/charts.py', u'Tools/QueueStatusServer/filters/webkit_extras.py', u'Tools/QueueStatusServer/handlers/queuecharts.py', u'Tools/QueueStatusServer/handlers/queuestatus.py', u'Tools/QueueStatusServer/index.yaml', u'Tools/QueueStatusServer/main.py', u'Tools/QueueStatusServer/model/queuelog.py', u'Tools/QueueStatusServer/stylesheets/charts.css', u'Tools/QueueStatusServer/templates/queuecharts.html', u'Tools/QueueStatusServer/templates/queuestatus.html']" exit_code: 1
Tools/QueueStatusServer/model/queuelog.py:69:  [QueueLog._get_or_create_txn] Class 'QueueLog' has no 'get_by_key_name' member  [pylint/E1101] [5]
Tools/QueueStatusServer/handlers/queuecharts.py:34:  No name 'appengine' in module 'google'  [pylint/E0611] [5]
Tools/QueueStatusServer/handlers/queuecharts.py:35:  No name 'appengine' in module 'google'  [pylint/E0611] [5]
Tools/QueueStatusServer/handlers/queuecharts.py:50:  [QueueCharts.get] Instance of 'QueueCharts' has no 'error' member  [pylint/E1101] [5]
Tools/QueueStatusServer/handlers/queuecharts.py:64:  [QueueCharts.get] Instance of 'QueueCharts' has no 'response' member  [pylint/E1101] [5]
Tools/QueueStatusServer/handlers/queuecharts.py:130:  [QueueCharts._get_timestamp] Instance of 'QueueCharts' has no 'request' member  [pylint/E1101] [5]
Tools/QueueStatusServer/handlers/queuecharts.py:137:  [QueueCharts._get_view_range] Instance of 'QueueCharts' has no 'request' member  [pylint/E1101] [5]
Total errors found: 7 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Seidel (no email)]

(In reply to comment #7)
> A "pre-release" of this patch can be viewed at: http://107659.webkit-commit-queue.appspot.com/queue-charts/chromium-ews
> Feel free to suggest improvements to the interface.

Feedback:
- use "Hours Ago" instead of a big time blob to mark the x-asis  You can add a page timestamp somewhere else if you like instead.
- have the hover/click show "20 hours ago" instead of "-20.35"
- In general, all hover/clicks should be rounded to at most 2 decimals.
- I read left-to-right.  It might make sense for "now" (0) to be on the left. :)  Not sure.
- Patches Waiting and Patch Retries lines don't seem to have any data.
- I'm not sure we're showing the most interesting graph at the top.  But I"m not sure which is most interesting.

It also might make sense to add a table of "interesting stats" at the top, since those may be easier to read than the graphs.

[Alan Cutter]

Created attachment 185210 [details]
Patch

[WebKit Review Bot]

Attachment 185210 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/QueueStatusServer/app.yaml', u'Tools/QueueStatusServer/config/charts.py', u'Tools/QueueStatusServer/filters/webkit_extras.py', u'Tools/QueueStatusServer/handlers/queuecharts.py', u'Tools/QueueStatusServer/handlers/queuestatus.py', u'Tools/QueueStatusServer/index.yaml', u'Tools/QueueStatusServer/main.py', u'Tools/QueueStatusServer/model/queuelog.py', u'Tools/QueueStatusServer/stylesheets/charts.css', u'Tools/QueueStatusServer/templates/queuecharts.html', u'Tools/QueueStatusServer/templates/queuestatus.html']" exit_code: 1
Tools/QueueStatusServer/model/queuelog.py:69:  [QueueLog._get_or_create_txn] Class 'QueueLog' has no 'get_by_key_name' member  [pylint/E1101] [5]
Tools/QueueStatusServer/handlers/queuecharts.py:137:  trailing whitespace  [pep8/W291] [5]
Tools/QueueStatusServer/handlers/queuecharts.py:34:  No name 'appengine' in module 'google'  [pylint/E0611] [5]
Tools/QueueStatusServer/handlers/queuecharts.py:35:  No name 'appengine' in module 'google'  [pylint/E0611] [5]
Tools/QueueStatusServer/handlers/queuecharts.py:47:  [QueueCharts.get] Instance of 'QueueCharts' has no 'error' member  [pylint/E1101] [5]
Tools/QueueStatusServer/handlers/queuecharts.py:69:  [QueueCharts.get] Instance of 'QueueCharts' has no 'response' member  [pylint/E1101] [5]
Tools/QueueStatusServer/handlers/queuecharts.py:140:  [QueueCharts._get_timestamp] Instance of 'QueueCharts' has no 'request' member  [pylint/E1101] [5]
Tools/QueueStatusServer/handlers/queuecharts.py:147:  [QueueCharts._get_view_range] Instance of 'QueueCharts' has no 'request' member  [pylint/E1101] [5]
Total errors found: 8 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alan Cutter]

(In reply to comment #9)
> (In reply to comment #7)
> > A "pre-release" of this patch can be viewed at: http://107659.webkit-commit-queue.appspot.com/queue-charts/chromium-ews
> > Feel free to suggest improvements to the interface.
> 
> Feedback:
> - use "Hours Ago" instead of a big time blob to mark the x-asis  You can add a page timestamp somewhere else if you like instead.

Agreed, changed to use a simpler X axis title.

> - have the hover/click show "20 hours ago" instead of "-20.35"

Tooltip now shows human friendly duration string.

> - In general, all hover/clicks should be rounded to at most 2 decimals.

No more decimals in tooltips.

> - I read left-to-right.  It might make sense for "now" (0) to be on the left. :)  Not sure.

Spoke with others about this, more people seemed to be in favor of a left-to-right timeline.

> - Patches Waiting and Patch Retries lines don't seem to have any data.

The "patches waiting" seems to work for the commit-queue but not others.
There are some issues with the existing data logging that I'd like to change in a separate bug.

> It also might make sense to add a table of "interesting stats" at the top, since those may be easier to read than the graphs.

Thought about this but couldn't decide what additional information would be good to show that's not easily ascertained from the graphs themselves.


Other additions:
 - Ability to choose view range.
 - Automatic time unit selection.
 - More appropriate gridlines.
 - Changed some colours.
 - Added links to other queue charts.

Latest changes can still be seen at: http://107659.webkit-commit-queue.appspot.com/queue-charts/chromium-ews

[Eric Seidel (no email)]

Comment on attachment 185210 [details]
Patch

I think this is a great start and we should ship and iterate.

[WebKit Review Bot]

Comment on attachment 185210 [details]
Patch

Clearing flags on attachment: 185210

Committed r141129: <http://trac.webkit.org/changeset/141129>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.