Bug 208920

Summary:

[ews] Resubmit patches to commit-queue which were cq- by commit-queue and later cq+

Product:

WebKit

Reporter:

Aakash Jain <aakash_jain>

Component:

Tools / Tests

Assignee:

Aakash Jain <aakash_jain>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aakash_jain, ap, jbedard, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

Other

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=196628

Bug Depends on:

Bug Blocks:

201934

Attachments:

Description	Flags
WIP	none
Patch	none
Patch	none

Aakash Jain

Reported 2020-03-11 08:56:38 PDT

EWS django app should resubmit patches to commit-queue which were cq- by commit-queue (e.g.: due to flaky test failure) and later were cq+. Currently ews django app does not re-submit any patch, it sends the patch to buildbot only once. Because of this is a patch which was rejected by commit-queue, and later cq+ again by patch author, would not be picked up again.

Attachments
WIP (6.71 KB, patch) 2020-03-11 09:08 PDT, Aakash Jain	no flags	Details Formatted Diff Diff
Patch (9.70 KB, patch) 2020-03-17 12:47 PDT, Aakash Jain	no flags	Details Formatted Diff Diff
Patch (9.51 KB, patch) 2020-03-17 14:32 PDT, Aakash Jain	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Aakash Jain

Comment 1 2020-03-11 09:08:42 PDT

Created attachment 393247 [details] WIP

Aakash Jain

Comment 2 2020-03-17 12:47:50 PDT

Created attachment 393778 [details] Patch

Jonathan Bedard

Comment 3 2020-03-17 13:44:39 PDT

Comment on attachment 393778 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393778&action=review > Tools/BuildSlaveSupport/ews-app/ews/fetcher.py:106 > def patches_to_send_to_buildbot(self, patch_ids, commit_queue=False): If we are adding a new 'patches_to_send_to_commit_queue' function, what's the point of this flag? > Tools/BuildSlaveSupport/ews-app/ews/common/bugzilla.py:67 > + def convert_iso_timestamp(cls, iso_timestamp): Nit: Not convinced that this should be a separate function, I would have put the logic in the above function. > Tools/BuildSlaveSupport/ews-app/ews/common/buildbot.py:-102 > - return Buildbot.icons_for_queues_mapping I think this deletion was unintentional. > Tools/BuildSlaveSupport/ews-app/ews/common/buildbot.py:104 > + def fetch_builders(cls): Do we ever intend to call this outside of update_builder_name_to_id_mapping? Seems like we should consider putting the contents inside that function

Aakash Jain

Comment 4 2020-03-17 14:32:02 PDT

Created attachment 393788 [details] Patch

Aakash Jain

Comment 5 2020-03-17 14:35:36 PDT

(In reply to Jonathan Bedard from comment #3) > If we are adding a new 'patches_to_send_to_commit_queue' function, what's the point of this flag? Removed in updated patch > Nit: Not convinced that this should be a separate function, I would have put the logic in the above function. Moved inside same function. > > - return Buildbot.icons_for_queues_mapping > I think this deletion was unintentional. It was actually intentional. The return value is never used and the function name also doesn't indicate that the function will return anything. > Do we ever intend to call this outside of update_builder_name_to_id_mapping? Seems like we should consider putting the contents inside that function Done in updated patch.

EWS

Comment 6 2020-03-17 17:01:48 PDT

Committed r258611: <https://trac.webkit.org/changeset/258611> All reviewed patches have been landed. Closing bug and clearing flags on attachment 393788 [details].

Radar WebKit Bug Importer

Comment 7 2020-03-17 17:02:16 PDT

<rdar://problem/60561892>

Note You need to log in before you can comment on or make changes to this bug.