<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:rniwa@webkit.org" title="Ryosuke Niwa <rniwa@webkit.org>"> <span class="fn">Ryosuke Niwa</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - use multiprocessing in run-benchmark to launch web server"
href="https://bugs.webkit.org/show_bug.cgi?id=145584">bug 145584</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #254129 Flags</td>
<td>review?
</td>
<td>review+
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - use multiprocessing in run-benchmark to launch web server"
href="https://bugs.webkit.org/show_bug.cgi?id=145584#c2">Comment # 2</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - use multiprocessing in run-benchmark to launch web server"
href="https://bugs.webkit.org/show_bug.cgi?id=145584">bug 145584</a>
from <span class="vcard"><a class="email" href="mailto:rniwa@webkit.org" title="Ryosuke Niwa <rniwa@webkit.org>"> <span class="fn">Ryosuke Niwa</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=254129&action=diff" name="attach_254129" title="patch">attachment 254129</a> <a href="attachment.cgi?id=254129&action=edit" title="patch">[details]</a></span>
patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=254129&action=review">https://bugs.webkit.org/attachment.cgi?id=254129&action=review</a>
<span class="quote">> Tools/ChangeLog:4
> +</span >
Missing bug URL.
<span class="quote">> Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/simple_http_server_driver.py:35
> + except Exception as e:</span >
Spell out exception or error.
<span class="quote">> Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/simple_http_server_driver.py:65
> + command = ['/usr/sbin/lsof', '-a', '-iTCP', '-sTCP:LISTEN', '-p', str(self.server_process.pid)]
> + output = subprocess.check_output(command)</span >
Perhaps we don't really need a local variable `command` here?
<span class="quote">> Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/simple_http_server_driver.py:66
> + self.server_port = int(re.findall('TCP \*:(\d+) \(LISTEN\)', output)[0])</span >
Why don't we just use re.search if we're grabbing the first result?
<span class="quote">> Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/simple_http_server_driver.py:71
> + except Exception as e:
> + _log.info('Error: %s' % e)</span >
nit: spell out e.
<span class="quote">> Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/simple_http_server_driver.py:76
> + except Exception as e:
> + sys.exit("Cannot listen to server, max tries exceeded: %s" % e)</span >
Ditto. Also, why don't we use _log.error and sys.exit separately as done elsewhere for consistency?
<span class="quote">> Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/simple_http_server_driver.py:87
> + except Exception as e:
> + _log.info('Server not running, max tries exceeded: %s' % e)</span >
This code never runs unless _log.info or time.sleep blows up.
I think what you wanna do is to return immediately after subprocess.check_call.
And just error and sys.exit when we got out of the for loop without hitting that return.
<span class="quote">> Tools/Scripts/webkitpy/benchmark_runner/http_server_driver/simple_http_server_driver.py:121
> + def postResult(self, result):</span >
Can we rename this to post_result for consistency if we're using PEP style?</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>