Bug 203270 - Refactor "builds" table "build_number" row to "build_tag" to fit more generic use cases.
Summary: Refactor "builds" table "build_number" row to "build_tag" to fit more generic...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: dewei_zhu
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-22 14:03 PDT by dewei_zhu
Modified: 2019-10-25 15:54 PDT (History)
2 users (show)

See Also:


Attachments
Patch (95.87 KB, patch)
2019-10-22 14:10 PDT, dewei_zhu
no flags Details | Formatted Diff | Diff
Patch (95.18 KB, patch)
2019-10-22 17:05 PDT, dewei_zhu
rniwa: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description dewei_zhu 2019-10-22 14:03:44 PDT
Refactor "builds" table "builder_number" row to "build_serial" to fit more generic use cases.
Comment 1 dewei_zhu 2019-10-22 14:10:08 PDT
Created attachment 381599 [details]
Patch
Comment 2 Ryosuke Niwa 2019-10-22 14:27:13 PDT
Comment on attachment 381599 [details]
Patch

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

> Websites/perf.webkit.org/init-database.sql:79
> +    build_serial varchar(64) NOT NULL,

Not sure serial is a good name for this since non-numeric identifier isn’t in “series”.
Maybe we can keep “number” and just change the type.
Or perhaps “revision” since we use that for commits as well.
Comment 3 dewei_zhu 2019-10-22 17:05:30 PDT
Created attachment 381633 [details]
Patch
Comment 4 Ryosuke Niwa 2019-10-23 14:33:01 PDT
Comment on attachment 381633 [details]
Patch

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

> Websites/perf.webkit.org/public/api/upload-root.php:14
> +        exit_with_error('ShouldNotHaveBothBuildNumberAndBuildTag', array('buildNumber' => $_POST['buildNumber'], 'buildTag' => $_POST['buildTag']));

This is a lengthly error message. Also, it seems fine if we specified both & both are identical during the transition period.
Why not check the values? and return BuilderNumberTagMismatch in the case they're not equal.

> Websites/perf.webkit.org/public/include/report-processor.php:110
> +        if (array_key_exists('buildNumber', $report)) {
> +            if (array_key_exists('buildTag', $report))

Ditto about allowing both if they're identical.
Comment 5 Ryosuke Niwa 2019-10-23 14:34:08 PDT
Note: we may want to add a guide on how to upgrade perf dashboard to a new version to ReadMe.md saying we want to run the migration sql file and then also clearing the cache directory.
Comment 6 dewei_zhu 2019-10-25 15:49:19 PDT
Landed in r251564.
Comment 7 dewei_zhu 2019-10-25 15:54:53 PDT
<rdar://problem/56635615>