Bug 251157 - [REGRESSION] Unauthenticated request to list PR files triggering rate-limits
Summary: [REGRESSION] Unauthenticated request to list PR files triggering rate-limits
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-01-25 09:44 PST by Aakash Jain
Modified: 2023-01-25 14:19 PST (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Aakash Jain 2023-01-25 09:50:12 PST
Example of recent build in which the logic worked fine: https://ews-build.webkit.org/#/builders/1/builds/94523 
In this "check-change-relevance" step says: "Pull request doesn't have relevant changes" and the 'stdio' log say: "This pull request does not have relevant changes."
Comment 2 Aakash Jain 2023-01-25 09:53:45 PST
In the non-working build https://ews-build.webkit.org/#/builders/1/builds/94597, check-change-relevance step says: "Pull request contains relevant changes", and no 'stdio' log.

IN this build from Jan 13, when "check-change-relevance" said "Pull request contains relevant changes", there was also 'stdio' log saying: "This pull request contains relevant changes.": https://ews-build.webkit.org/#/builders/1/builds/93414


Missing 'stdio' log indicate that in CheckChangeRelevance.start() self._get_patch() returned empty patch triggering this code path for early exit (without any 'stdio' log): https://github.com/WebKit/WebKit/blob/main/Tools/CISupport/ews-build/steps.py#L1211


_get_patch() method was recently changed on Jan 19 in https://commits.webkit.org/259088@main
Comment 3 Aakash Jain 2023-01-25 11:13:28 PST
For non-working build https://ews-build.webkit.org/#/builders/1/builds/94597, https://ews-build.webkit.org/api/v2/sourcestamps/740172/changes shows that the files in sourcestamp was empty [].

However, for old working build https://ews-build.webkit.org/#/builders/1/builds/93414,  https://ews-build.webkit.org/api/v2/sourcestamps/736612/changes shows that the files in sourcestamp was non-empty.


Also, the logs on server have this:
twistd.log.1:2023-01-25 09:39:38-0800 [HTTP11ClientProtocol (TLSMemoryBIOProtocol),client] Failed fetching files for PR #9083: response code 400
Comment 4 Aakash Jain 2023-01-25 11:17:48 PST
Seems like a regression from https://commits.webkit.org/259282@main (Bug 250714)
Comment 5 Jonathan Bedard 2023-01-25 11:58:37 PST
We aren't fetching files with auth, that's the problem, silly mistake, easy fix.
Comment 6 Aakash Jain 2023-01-25 12:33:57 PST
This is happening on multiple queues including WebkitPy (e.g.: https://ews-build.webkit.org/#/builders/5/builds/108210), Services-EWS (e.g.: https://ews-build.webkit.org/#/builders/20/builds/103549) etc.
Comment 7 Radar WebKit Bug Importer 2023-01-25 12:40:26 PST
<rdar://problem/104659979>
Comment 8 Jonathan Bedard 2023-01-25 12:43:20 PST
Pull request: https://github.com/WebKit/WebKit/pull/9141
Comment 9 EWS 2023-01-25 12:54:53 PST
Committed 259382@main (42ad013f55b2): <https://commits.webkit.org/259382@main>

Reviewed commits have been landed. Closing PR #9141 and removing active labels.
Comment 10 Jonathan Bedard 2023-01-25 14:12:35 PST
Re-opening for pull request https://github.com/WebKit/WebKit/pull/9149
Comment 11 EWS 2023-01-25 14:19:54 PST
Committed 259386@main (c54b1f4645fb): <https://commits.webkit.org/259386@main>

Reviewed commits have been landed. Closing PR #9149 and removing active labels.