EWS should provide better information to Dashboard via JSON

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Alexey Proskuryakov]

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.

[Alexey Proskuryakov]

Created attachment 221617 [details]
proposed patch

[WebKit Commit Bot]

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.

[Ryosuke Niwa]

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?

[Alexey Proskuryakov]

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.

[Alexey Proskuryakov]

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.