Bug 83294 – perf-o-matic should include unit in runs JSON responses

• Component: New Bugs
• Status: RESOLVED FIXED

[Ryosuke Niwa]

perf-o-matic should include unit in runs JSON responses

[Ryosuke Niwa]

Created attachment 135856 [details]
Patch

[Ryosuke Niwa]

Once we land this & push it to the production, we can show units on perf-o-matic :)

[Ryosuke Niwa]

Note that I've already made the frontend change upstream, and rhelmer has pulled my changes:
https://github.com/mozilla/graphs/commit/ba7b2b21f5b001f8513aca8e613f6e0920d8fff3
https://github.com/mozilla/graphs/commit/c1fc48d40134b25876a7664eadab342f36a33b3a

[Tony Chang]

Comment on attachment 135856 [details]
Patch

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

> Websites/webkit-perf.appspot.com/models.py:452
> +        return '{"test_runs": [%s], "averages": {%s}, "min": %s, "max": %s, "unit": %s, "date_range": null, "stat": "ok"}' % (self.json_runs,
> +            self.json_averages, str(self.json_min) if self.json_min else 'null', str(self.json_max) if self.json_max else 'null',

Nit: You might want to start naming the strings. E.g., %(test_runs)s .. %(averages)s .. etc.  That might make it a bit easier to match up the string substitutions.

[Ryosuke Niwa]

Comment on attachment 135856 [details]
Patch

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

>> Websites/webkit-perf.appspot.com/models.py:452
>> +            self.json_averages, str(self.json_min) if self.json_min else 'null', str(self.json_max) if self.json_max else 'null',
> 
> Nit: You might want to start naming the strings. E.g., %(test_runs)s .. %(averages)s .. etc.  That might make it a bit easier to match up the string substitutions.

Yeah I guess. But then I'll have to repeat that in the string and in the dictionary and will make the code much more verbose.
I could make '"min": %s, "max": %s, "unit": %s, "date_range": null, "stat": "ok"' a dictionary and just call json.dumps instead.
(Can't call dumps on test_runs and json_averages since they aren't real json.
On the other hand, that ends up requiring some string manipulating of { and }. Not sure if that's desirable :\

[Ryosuke Niwa]

Committed r113356: <http://trac.webkit.org/changeset/113356>