Bug 193735 - [ews-app] Add method to save Build data to database
Summary: [ews-app] Add method to save Build data to database
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-01-23 12:54 PST by Aakash Jain
Modified: 2019-01-28 15:09 PST (History)
5 users (show)

See Also:

Attachments
Proposed path (4.40 KB, patch)
2019-01-23 12:58 PST, Aakash Jain 		lforschler: review+
Details | Formatted Diff | Diff
Updated patch (6.36 KB, patch)
2019-01-28 04:51 PST, Aakash Jain 		lforschler: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Aakash Jain 2019-01-23 12:54:21 PST 
We should add method to save Build data to database for ews-app.
Comment 1 Aakash Jain 2019-01-23 12:58:35 PST 
Created attachment 359931 [details]
Proposed path
Comment 2 Lucas Forschler 2019-01-23 13:15:42 PST 
Comment on attachment 359931 [details]
Proposed path

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

logic looks good to me, with possible name/refactoring.

> Tools/BuildSlaveSupport/ews-app/ews/common/util.py:46
> +def is_valid_int_id(id):

would this be ok as 'is_valid_id' ?

> Tools/BuildSlaveSupport/ews-app/ews/models/build.py:51
> +    def save_build(cls, patch_id, buildid, builderid, number, result, state_string, started_at, complete_at=None):

since we are using underscores for patch_id, should we continue that with build_id, builder_id?

> Tools/BuildSlaveSupport/ews-app/ews/models/build.py:72
> +        if not (util.is_valid_int_id(patch_id) and util.is_valid_int_id(buildid) and util.is_valid_int_id(builderid)  and util.is_valid_int_id(number)):

nit: extra space before the last 'and'
Comment 3 Aakash Jain 2019-01-28 04:51:15 PST 
Created attachment 360334 [details]
Updated patch

> would this be ok as 'is_valid_id' ?
renamed. Also updated the method to make it more generic.

> since we are using underscores for patch_id, should we continue that with build_id, builder_id?
Uploaded Patch for this change in https://bugs.webkit.org/show_bug.cgi?id=193883 Also updated this patch to use build_id, builder_id.

> nit: extra space before the last 'and'
Fixed.

Also added some more functionality (e.g.: update_build() method). Can you please review again?
Comment 4 Aakash Jain 2019-01-28 11:28:51 PST 
This patch should apply to ToT after https://bugs.webkit.org/show_bug.cgi?id=193883
Comment 5 WebKit Commit Bot 2019-01-28 12:12:54 PST 
Comment on attachment 360334 [details]
Updated patch

Rejecting attachment 360334 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 360334, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Tools/ChangeLog is not at the top of the file.

Full output: https://webkit-queues.webkit.org/results/10926382
Comment 6 Aakash Jain 2019-01-28 15:08:50 PST 
Committed r240603: <https://trac.webkit.org/changeset/240603>
Comment 7 Radar WebKit Bug Importer 2019-01-28 15:09:27 PST 
<rdar://problem/47612792>