Bug 160698

Summary:

EWS logging should ensure the logging to file is stopped on queue termination

Product:

WebKit

Reporter:

Aakash Jain <aakash_jain>

Component:

Tools / Tests

Assignee:

Aakash Jain <aakash_jain>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aakash_jain, ap, commit-queue, dbates, glenn, lforschler

Priority:

Version:

WebKit Local Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

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

Attachments:

Description	Flags
Proposed patch	dbates: review+, dbates: commit-queue-
Updated patch	aakash_jain: commit-queue+
Updated patch	none

Aakash Jain

Reported 2016-08-09 10:40:17 PDT

EWS starts logging to file when the queue begins, but does not stops the logging to file when queue is terminated. It might results in unexpected and unwanted log messages to go to log file, making the log file less readable and confusing. We should make sure to stop the logging to file on queue termination.

Attachments
Proposed patch (17.29 KB, patch) 2016-08-09 11:09 PDT, Aakash Jain	dbates: review+ dbates: commit-queue-	Details Formatted Diff Diff
Updated patch (17.28 KB, patch) 2016-08-09 12:00 PDT, Aakash Jain	aakash_jain: commit-queue+	Details Formatted Diff Diff
Updated patch (17.28 KB, patch) 2016-08-09 12:01 PDT, Aakash Jain	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Aakash Jain

Comment 1 2016-08-09 11:09:03 PDT

Created attachment 285651 [details] Proposed patch

Daniel Bates

Comment 2 2016-08-09 11:35:10 PDT

Comment on attachment 285651 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=285651&action=review > Tools/Scripts/webkitpy/tool/bot/queueengine.py:129 > + _log.info("%s" % message) It is unnecessary to create a new string and use string interpolation when the result string is identical to the string we are interpolating. We should pass message directly: _log.info(message) > Tools/Scripts/webkitpy/tool/bot/queueengine_unittest.py:149 > + expected_logs = "%s\n" % termination_message I'm unclear on our plan for Python 3. We may want to consider updating this code to use String.format() instead of % to be compatible with Python 3. > Tools/Scripts/webkitpy/tool/commands/queues.py:112 > + return os.path.join("..", "%s-logs" % self.name) Ditto. > Tools/Scripts/webkitpy/tool/commands/queues.py:117 > + return os.path.join(self._log_directory(), "%s.log" % self.name) Ditto.

Aakash Jain

Comment 3 2016-08-09 11:59:39 PDT

> I'm unclear on our plan for Python 3. We may want to consider updating this code to use String.format() instead of % to be compatible with Python 3. I think we should do it in a separate patch if we decide to change. There are a lot of places in this file and similar files using %. Currently none of the file in the webkitpy/tool directory use String.format()

Aakash Jain

Comment 4 2016-08-09 12:00:11 PDT

Created attachment 285655 [details] Updated patch

Aakash Jain

Comment 5 2016-08-09 12:01:48 PDT

Created attachment 285656 [details] Updated patch Added the "Reviewed by" in ChangeLog.

WebKit Commit Bot

Comment 6 2016-08-09 12:31:44 PDT

Comment on attachment 285656 [details] Updated patch Clearing flags on attachment: 285656 Committed r204289: <http://trac.webkit.org/changeset/204289>

WebKit Commit Bot

Comment 7 2016-08-09 12:31:48 PDT

All reviewed patches have been landed. Closing bug.

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