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

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Aakash Jain]

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.

[Aakash Jain]

Created attachment 285651 [details]
Proposed patch

[Daniel Bates]

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]

> 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]

Created attachment 285655 [details]
Updated patch

[Aakash Jain]

Created attachment 285656 [details]
Updated patch

Added the "Reviewed by" in ChangeLog.

[WebKit Commit Bot]

Comment on attachment 285656 [details]
Updated patch

Clearing flags on attachment: 285656

Committed r204289: <http://trac.webkit.org/changeset/204289>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.