Bug 127265 - EWS should provide better information to Dashboard via JSON
Summary: EWS should provide better information to Dashboard via JSON
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks: 127006
  Show dependency treegraph
 
Reported: 2014-01-19 22:54 PST by Alexey Proskuryakov
Modified: 2014-01-20 10:34 PST (History)
4 users (show)

See Also:


Attachments
proposed patch (8.71 KB, patch)
2014-01-19 23:01 PST, Alexey Proskuryakov
rniwa: review+
ap: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2014-01-19 22:54:22 PST
Right now, the dashboard only uses queue length, but I have a patch almost ready that adds a popover with latest status.

Also, we don't need to perform costly operations until asked to, so this patch splits JSON handler in two.
Comment 1 Alexey Proskuryakov 2014-01-19 23:01:09 PST
Created attachment 221617 [details]
proposed patch
Comment 2 WebKit Commit Bot 2014-01-19 23:03:11 PST
Attachment 221617 [details] did not pass style-queue:


ERROR: Tools/QueueStatusServer/handlers/queuestatusjson.py:59:  trailing whitespace  [pep8/W291] [5]
ERROR: Tools/QueueStatusServer/handlers/queuestatusjson.py:58:  [QueueStatusJSON._rows_for_work_items] Instance of 'QueueStatusJSON' has no 'request' member  [pylint/E1101] [5]
ERROR: Tools/QueueStatusServer/handlers/queuestatusjson.py:109:  [QueueStatusJSON.get] Instance of 'QueueStatusJSON' has no 'request' member  [pylint/E1101] [5]
ERROR: Tools/QueueStatusServer/handlers/queuelengthjson.py:47:  whitespace before '}'  [pep8/E202] [5]
ERROR: Tools/QueueStatusServer/handlers/queuelengthjson.py:36:  [QueueLengthJSON.get] Instance of 'QueueLengthJSON' has no 'response' member  [pylint/E1101] [5]
ERROR: Tools/QueueStatusServer/handlers/queuelengthjson.py:41:  [QueueLengthJSON.get] Instance of 'QueueLengthJSON' has no 'error' member  [pylint/E1101] [5]
ERROR: Tools/QueueStatusServer/handlers/queuelengthjson.py:44:  [QueueLengthJSON.get] Instance of 'QueueLengthJSON' has no 'response' member  [pylint/E1101] [5]
ERROR: Tools/QueueStatusServer/handlers/queuelengthjson.py:49:  [QueueLengthJSON.get] Instance of 'QueueLengthJSON' has no 'response' member  [pylint/E1101] [5]
Total errors found: 8 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Ryosuke Niwa 2014-01-19 23:15:15 PST
Comment on attachment 221617 [details]
proposed patch

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

> Tools/QueueStatusServer/app.yaml:2
> -version: 162287 # Bugzilla bug ID of last major change
> +version: ap # Bugzilla bug ID of last major change

This doesn't look right.

> Tools/QueueStatusServer/handlers/queuestatusjson.py:59
> +            

Nit: Whitespaces.

> Tools/QueueStatusServer/handlers/queuestatusjson.py:74
>      def _bots(self, queue):
> +        # Collect all bots that ever served this queue.

Why don't we rename the method instead of adding a comment like this?
Comment 4 Alexey Proskuryakov 2014-01-19 23:30:56 PST
Comment on attachment 221617 [details]
proposed patch

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

>> Tools/QueueStatusServer/app.yaml:2
>> +version: ap # Bugzilla bug ID of last major change
> 
> This doesn't look right.

Yeah, this is mentioned in ChangeLog. There really isn't anything that's right to put in diff here, I think.

>> Tools/QueueStatusServer/handlers/queuestatusjson.py:59
>> +            
> 
> Nit: Whitespaces.

Could you please elaborate? Is it bad to have this empty line?

>> Tools/QueueStatusServer/handlers/queuestatusjson.py:74
>> +        # Collect all bots that ever served this queue.
> 
> Why don't we rename the method instead of adding a comment like this?

This comment is about the line below, not about the method.
Comment 5 Alexey Proskuryakov 2014-01-20 10:34:28 PST
Committed <http://trac.webkit.org/r162358>.

> Could you please elaborate? Is it bad to have this empty line?

I see, this was about trailing spaces in the empty line, not about the empty line itself.

> This comment is about the line below, not about the method.

Slightly reworded the comment to make it clearer.