[webkit-reviews] review denied: [Bug 49187] [NRWT] Make http locking similar to perl implementation : [Attachment 73378] proposed patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Gabor Rapcsanyi
<rgabor at inf.u-szeged.hu>'s request for review:
Bug 49187: [NRWT] Make http locking similar to perl implementation
https://bugs.webkit.org/show_bug.cgi?id=49187

Attachment 73378: proposed patch
https://bugs.webkit.org/attachment.cgi?id=73378&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=73378&action=review

Just some small nits in the unittest.  The approach seems fine overall.

> WebKitTools/Scripts/webkitpy/common/system/file_lock.py:27
> +"""This class helps to lock files exclusively."""

Nit: maybe mention that this is across processes.

> WebKitTools/Scripts/webkitpy/common/system/file_lock_unittest.py:38
> +	   self._lock_path = os.path.join(tempfile.gettempdir(),
"TestWebKit.lock")
> +	   self._file_lock = FileLock(self._lock_path)

Can you run this in setUp() instead?

> WebKitTools/Scripts/webkitpy/common/system/file_lock_unittest.py:48
> +	   # Create the lock.
> +	   self._file_lock.acquire_lock()
> +	   self.assertTrue(os.path.exists(self._lock_path))

What happens if two bots that run on the same machine try to run python-tests
at the same time?  Maybe we can randomize the lock name.