Bug 183222

Summary: [webkitpy] Bugzilla class should use NetworkTransaction for network GET requests
Product: WebKit Reporter: Aakash Jain <aakash_jain>
Component: Tools / TestsAssignee: Aakash Jain <aakash_jain>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, ap, commit-queue, dean_johnson, ews-watchlist, glenn, jbedard, lforschler, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=183156
https://bugs.webkit.org/show_bug.cgi?id=182420
https://bugs.webkit.org/show_bug.cgi?id=183463
Attachments:
Description Flags
Proposed patch
none
Updated patch without additional logging none

Aakash Jain
Reported 2018-02-28 13:26:51 PST
commit-queue failed to land https://webkit-queues.webkit.org/patch/334757/commit-queue Logs indicate network issue. We should add retry on network error on all the Bugzilla operations now. 2018-02-28 12:52:12,529 - Patch does not pass tests 2018-02-28 12:52:15,976 - Fetching: https://bugs.webkit.org/attachment.cgi?id=334757&action=edit 2018-02-28 12:52:16,255 - Fetching: https://bugs.webkit.org/show_bug.cgi?id=183171&ctype=xml&excludefield=attachmentdata 2018-02-28 12:52:16,522 - Running: webkit-patch --status-host=webkit-queues.webkit.org --bot-id=webkit-cq-01 build-and-test --no-clean --no-update --test --non-interactive --build-style=release --port=mac 2018-02-28 13:03:27,741 - Passed tests 2018-02-28 13:04:04,687 - Bug does not already exist for media/video-poster.html, creating. 2018-02-28 13:04:04,687 - Creating bug with title "Flaky Test: media/video-poster.html" Traceback (most recent call last): File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/bot/queueengine.py", line 103, in run if not self._delegate.process_work_item(work_item): File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/commands/queues.py", line 340, in process_work_item if task.run(): File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py", line 88, in run if not self._did_pass_tests_recently(): File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py", line 72, in _did_pass_tests_recently return self._test_patch() File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/bot/patchanalysistask.py", line 374, in _test_patch return self._retry_layout_tests() File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/bot/patchanalysistask.py", line 305, in _retry_layout_tests self._report_flaky_tests(first_results.failing_test_results(), first_results_archive) File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/bot/patchanalysistask.py", line 217, in _report_flaky_tests self._delegate.report_flaky_tests(self._patch, flaky_test_results, results_archive) File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/commands/queues.py", line 403, in report_flaky_tests reporter.report_flaky_tests(patch, flaky_test_results, results_archive) File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/bot/flakytestreporter.py", line 184, in report_flaky_tests flake_bug_id = self._create_bug_for_flaky_test(flaky_test, author_emails, latest_flake_message) File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/tool/bot/flakytestreporter.py", line 116, in _create_bug_for_flaky_test blocked="50856") File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py", line 711, in create_bug self.browser.open(config_urls.bug_server_url + "enter_bug.cgi?product=WebKit") File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_mechanize.py", line 203, in open return self._mech_open(url, data, timeout=timeout) File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_mechanize.py", line 230, in _mech_open response = UserAgentBase.open(self, request, data) File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_opener.py", line 193, in open response = urlopen(self, req, data) File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 344, in _open '_open', req) File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 332, in _call_chain result = func(*args) File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1170, in https_open return self.do_open(conn_factory, req) File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1118, in do_open raise URLError(err) URLError: <urlopen error [Errno 60] Operation timed out> 2018-02-28 13:04:12,557 - Exception while preparing queue Sleeping until 2018-02-28 13:06:12 (120 seconds).
Attachments
Proposed patch (11.23 KB, patch)
2018-03-02 13:23 PST, Aakash Jain 		no flags
DetailsFormatted DiffDiff
Updated patch without additional logging (10.29 KB, patch)
2018-03-02 14:54 PST, Aakash Jain 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Aakash Jain
Comment 1 2018-02-28 13:27:58 PST
https://bugs.webkit.org/show_bug.cgi?id=183156 added retry on some of the network operations while talking to bugzilla, and that helped. We should add retry in all the network operations in Bugzilla class.
Aakash Jain
Comment 2 2018-02-28 15:45:05 PST
One more examples: https://webkit-queues.webkit.org/results/6711146 File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py", line 493, in fetch_attachment_contents
Aakash Jain
Comment 3 2018-03-02 12:49:12 PST
Updated title. NetworkTransaction class has the retry logic. Bugzilla class should use NetworkTransaction for the network operations.
Aakash Jain
Comment 4 2018-03-02 13:23:07 PST
Created attachment 334920 [details] Proposed patch Using method open_url() which uses NetworkTransaction for all self.browser.open() Also reverting changes done in https://trac.webkit.org/changeset/229064/webkit and using open_url() instead as this is cleaner approach.
Dean Johnson
Comment 5 2018-03-02 13:38:15 PST
Comment on attachment 334920 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=334920&action=review I really like the philosophy of the patch (retry instead of failing completely If necessary), just have a small issue with the implementation of NetworkTransaction. Let's chat in-person Monday and see if we can find a better way to do this. If not, the benefit of the patch probably outweighs the new logging in NetworkTransaction. > Tools/Scripts/webkitpy/common/net/networktransaction.py:58 > + _log.warn('Received HTTP status {} while loading {}. Retrying in {} seconds...'.format(e.code, url, self._backoff_seconds)) Why make this change? Is there a way for us to bubble up e.<some_var> for the URL instead of requiring it to be passed into NetworkTransaction? I don't like that passing url is now required for useful logging. > Tools/Scripts/webkitpy/common/net/networktransaction.py:62 > + _log.warn('Received URLError: "{}" while loading {}. Retrying in {} seconds...'.format(e.reason, url, self._backoff_seconds)) Ditto.
Alexey Proskuryakov
Comment 6 2018-03-02 14:08:02 PST
> Using method open_url() which uses NetworkTransaction for all self.browser.open() Does this only apply retries to GET, never POST? Given that we can't retry POST, the only real fix is to fix the network.
Aakash Jain
Comment 7 2018-03-02 14:54:39 PST
Created attachment 334933 [details] Updated patch without additional logging Patch without additional logging. Will add logs in a separate patch. Let's fix the urgent issue causing visible EWS problems first.
Aakash Jain
Comment 8 2018-03-02 14:58:30 PST
> I really like the philosophy of the patch (retry instead of failing > completely If necessary), just have a small issue with the implementation of > NetworkTransaction. Let's chat in-person Monday and see if we can find a > better way to do this. If not, the benefit of the patch probably outweighs > the new logging in NetworkTransaction. Sure. For now, I have removed the additional logging. Let's discuss on Monday and add that in a separate patch. > Does this only apply retries to GET, never POST? Yes, this apply retries only to POST (only to browser.open(), not to browser.submit()) Reference for open(): http://mechanize.readthedocs.io/en/latest/browser_api.html#mechanize.Browser.open
Aakash Jain
Comment 9 2018-03-02 15:01:14 PST
> Yes, this apply retries only to POST (only to browser.open(), not to I meant only to GET. (wish bugzilla has option to edit comments).
WebKit Commit Bot
Comment 10 2018-03-05 17:31:00 PST
Comment on attachment 334933 [details] Updated patch without additional logging Clearing flags on attachment: 334933 Committed r229296: <https://trac.webkit.org/changeset/229296>
WebKit Commit Bot
Comment 11 2018-03-05 17:31:02 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 12 2018-03-05 17:33:09 PST
<rdar://problem/38161814>
Note You need to log in before you can comment on or make changes to this bug.