)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"9524f8e38b1cee4deb2244e92eff37d562ef3639","unresolved":false,"context_lines":[{"line_number":12,"context_line":"using a semaphore."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: Id9d38832c67f2d81d382cda797a48fee943a27f1"},{"line_number":15,"context_line":"Closes-bug: #1654287"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"1f485f77_6560cfcf","line":15,"range":{"start_line":15,"start_character":0,"end_line":15,"end_character":20},"updated":"2017-11-13 09:55:38.000000000","message":"Does it? 1654287 is when caller of daemon doesn\u0027t read the result back and hence the output data from executed command are left in the socket. Does this patch solve also that issue?","commit_id":"7711a6ce31c3ffa8249bfeac53a4d9e306a7299f"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"f462a6baf53689dac6d52d11a742ff20632aa53e","unresolved":false,"context_lines":[{"line_number":12,"context_line":"using a semaphore."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: Id9d38832c67f2d81d382cda797a48fee943a27f1"},{"line_number":15,"context_line":"Closes-bug: #1654287"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"1f485f77_bd35a5dc","line":15,"range":{"start_line":15,"start_character":0,"end_line":15,"end_character":20},"in_reply_to":"1f485f77_6560cfcf","updated":"2017-11-14 01:23:56.000000000","message":"The Client._need_restart flag handles this case.\nAnd _thread_worker_timeout threads test this.","commit_id":"7711a6ce31c3ffa8249bfeac53a4d9e306a7299f"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"65abbed603349e035580fbca6919b33b39007acb","unresolved":false,"context_lines":[{"line_number":12,"context_line":"using a semaphore."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: Id9d38832c67f2d81d382cda797a48fee943a27f1"},{"line_number":15,"context_line":"Closes-bug: #1654287"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"1f485f77_98c66548","line":15,"range":{"start_line":15,"start_character":0,"end_line":15,"end_character":20},"in_reply_to":"1f485f77_bd35a5dc","updated":"2017-11-14 16:39:32.000000000","message":"Cool, thanks for explanation.","commit_id":"7711a6ce31c3ffa8249bfeac53a4d9e306a7299f"}],"oslo_rootwrap/client.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"340b2d306ba804df7b871d4b87e9445cae74a4f4","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"try:"},{"line_number":52,"context_line":"    import eventlet.semaphore"},{"line_number":53,"context_line":"    _exec_sem \u003d eventlet.semaphore.Semaphore()"},{"line_number":54,"context_line":"except:"},{"line_number":55,"context_line":"    _exec_sem \u003d DummySemaphore()"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4b6375_1eed1d8b","line":52,"updated":"2017-10-27 20:04:26.000000000","message":"I don\u0027t like that the behavior here depends on whether eventlet is installed in the venv. What if it\u0027s installed, but the process is still not monkey patched?\n\nShould we use some standard semaphore primitive and allow eventlet to patch it for us if needed? threading.Semaphore?","commit_id":"4445ffb4749a721da2ca2cdf358dfcbc0e8c19dc"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"9eeec017241328eaca47d03f7e195a4b90a72396","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"try:"},{"line_number":52,"context_line":"    import eventlet.semaphore"},{"line_number":53,"context_line":"    _exec_sem \u003d eventlet.semaphore.Semaphore()"},{"line_number":54,"context_line":"except:"},{"line_number":55,"context_line":"    _exec_sem \u003d DummySemaphore()"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4b6375_62ae9d99","line":52,"in_reply_to":"3f4b6375_1eed1d8b","updated":"2017-10-30 07:09:55.000000000","message":"Going to follow the code block 20 lines above. ;)\n\nthreading.Semaphore will not work as eventlet is a cooperative thread implementation. threading.Semaphore lacks eventlet.switch thing.\n\nI suspect _patched_socket can give false result depending on import order, though.","commit_id":"4445ffb4749a721da2ca2cdf358dfcbc0e8c19dc"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"297b3e8fcda8027381469a44f868abad740b9ce5","unresolved":false,"context_lines":[{"line_number":51,"context_line":"try:"},{"line_number":52,"context_line":"    import eventlet.semaphore"},{"line_number":53,"context_line":"    _exec_sem \u003d eventlet.semaphore.Semaphore()"},{"line_number":54,"context_line":"except:"},{"line_number":55,"context_line":"    _exec_sem \u003d DummySemaphore()"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"ClientManager \u003d daemon.get_manager_class()"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4b6375_7e2f71a7","line":54,"updated":"2017-10-27 20:06:11.000000000","message":"should be except ImportError I guess?.. Also, shouldn\u0027t we isolate the scope of the exception handler to line 52? Or do you expect errors from line 53 too?","commit_id":"4445ffb4749a721da2ca2cdf358dfcbc0e8c19dc"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"340b2d306ba804df7b871d4b87e9445cae74a4f4","unresolved":false,"context_lines":[{"line_number":140,"context_line":"        self._ensure_initialized()"},{"line_number":141,"context_line":"        locked \u003d False"},{"line_number":142,"context_line":"        retry \u003d False"},{"line_number":143,"context_line":"        res \u003d None"},{"line_number":144,"context_line":"        try:"},{"line_number":145,"context_line":"            locked \u003d _exec_sem.acquire()"},{"line_number":146,"context_line":"            proxy \u003d self._proxy"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4b6375_1ea2bd4c","line":143,"updated":"2017-10-27 20:04:26.000000000","message":"nit: this change could belong to a different patch since the UnboundNameError could happen regardless of the change. (in line 135).","commit_id":"4445ffb4749a721da2ca2cdf358dfcbc0e8c19dc"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"9eeec017241328eaca47d03f7e195a4b90a72396","unresolved":false,"context_lines":[{"line_number":140,"context_line":"        self._ensure_initialized()"},{"line_number":141,"context_line":"        locked \u003d False"},{"line_number":142,"context_line":"        retry \u003d False"},{"line_number":143,"context_line":"        res \u003d None"},{"line_number":144,"context_line":"        try:"},{"line_number":145,"context_line":"            locked \u003d _exec_sem.acquire()"},{"line_number":146,"context_line":"            proxy \u003d self._proxy"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4b6375_4293a15d","line":143,"in_reply_to":"3f4b6375_1ea2bd4c","updated":"2017-10-30 07:09:55.000000000","message":"I added this in order to handle WaitTimeout in the finally clause, IIRC. Also restarting proxy is kinda gross hack, too.","commit_id":"4445ffb4749a721da2ca2cdf358dfcbc0e8c19dc"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"20f480ebaa8c5671051469f21c326740c03ab2a8","unresolved":false,"context_lines":[{"line_number":140,"context_line":"        self._ensure_initialized()"},{"line_number":141,"context_line":"        locked \u003d False"},{"line_number":142,"context_line":"        retry \u003d False"},{"line_number":143,"context_line":"        res \u003d None"},{"line_number":144,"context_line":"        try:"},{"line_number":145,"context_line":"            locked \u003d _exec_sem.acquire()"},{"line_number":146,"context_line":"            proxy \u003d self._proxy"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4b6375_c4c40c43","line":143,"in_reply_to":"3f4b6375_4293a15d","updated":"2017-10-31 17:32:14.000000000","message":"Actually you are right, there is no code path in the old patch that would trigger the error that I mentioned.","commit_id":"4445ffb4749a721da2ca2cdf358dfcbc0e8c19dc"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"340b2d306ba804df7b871d4b87e9445cae74a4f4","unresolved":false,"context_lines":[{"line_number":143,"context_line":"        res \u003d None"},{"line_number":144,"context_line":"        try:"},{"line_number":145,"context_line":"            locked \u003d _exec_sem.acquire()"},{"line_number":146,"context_line":"            proxy \u003d self._proxy"},{"line_number":147,"context_line":"            res \u003d proxy.run_one_command(cmd, stdin)"},{"line_number":148,"context_line":"        except (EOFError, IOError):"},{"line_number":149,"context_line":"            retry \u003d True"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4b6375_befba945","line":146,"updated":"2017-10-27 20:04:26.000000000","message":"nit: can\u0027t you calculate \u0027proxy\u0027 before acquiring the lock?","commit_id":"4445ffb4749a721da2ca2cdf358dfcbc0e8c19dc"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"9eeec017241328eaca47d03f7e195a4b90a72396","unresolved":false,"context_lines":[{"line_number":143,"context_line":"        res \u003d None"},{"line_number":144,"context_line":"        try:"},{"line_number":145,"context_line":"            locked \u003d _exec_sem.acquire()"},{"line_number":146,"context_line":"            proxy \u003d self._proxy"},{"line_number":147,"context_line":"            res \u003d proxy.run_one_command(cmd, stdin)"},{"line_number":148,"context_line":"        except (EOFError, IOError):"},{"line_number":149,"context_line":"            retry \u003d True"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4b6375_68931fa5","line":146,"in_reply_to":"3f4b6375_befba945","updated":"2017-10-30 07:09:55.000000000","message":"A lock holder may restart the proxy, so self._proxy can change while waiting for a lock.","commit_id":"4445ffb4749a721da2ca2cdf358dfcbc0e8c19dc"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"340b2d306ba804df7b871d4b87e9445cae74a4f4","unresolved":false,"context_lines":[{"line_number":154,"context_line":"                        proxy \u003d self._restart(proxy)"},{"line_number":155,"context_line":"                finally:"},{"line_number":156,"context_line":"                    _exec_sem.release()"},{"line_number":157,"context_line":"        # res can be None if we received final None sent by dying server thread"},{"line_number":158,"context_line":"        # instead of response to our request. Process is most likely to be dead"},{"line_number":159,"context_line":"        # at this point."},{"line_number":160,"context_line":"        if retry or res is None:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4b6375_5ec155e4","line":157,"updated":"2017-10-27 20:04:26.000000000","message":"I believe this comment belongs to line 154.","commit_id":"4445ffb4749a721da2ca2cdf358dfcbc0e8c19dc"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"340b2d306ba804df7b871d4b87e9445cae74a4f4","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        if retry or res is None:"},{"line_number":161,"context_line":"            try:"},{"line_number":162,"context_line":"                _exec_sem.acquire()"},{"line_number":163,"context_line":"                res \u003d proxy.run_one_command(cmd, stdin)"},{"line_number":164,"context_line":"            finally:"},{"line_number":165,"context_line":"                _exec_sem.release()"},{"line_number":166,"context_line":"        return res"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4b6375_9e8d8dd5","line":163,"range":{"start_line":163,"start_character":16,"end_line":163,"end_character":55},"updated":"2017-10-27 20:04:26.000000000","message":"wouldn\u0027t it make sense to move this line to after line 154, and then remove the whole section (lines 160-165)?","commit_id":"4445ffb4749a721da2ca2cdf358dfcbc0e8c19dc"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"64b45a2b5f9da1b2a60c483dc4e1576508469dc8","unresolved":false,"context_lines":[{"line_number":52,"context_line":"    import eventlet.semaphore"},{"line_number":53,"context_line":"    _exec_sem \u003d eventlet.semaphore.Semaphore()"},{"line_number":54,"context_line":"else:"},{"line_number":55,"context_line":"    _exec_sem \u003d DummySemaphore()"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"ClientManager \u003d daemon.get_manager_class()"},{"line_number":58,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f4b6375_2e8cabd5","line":55,"updated":"2017-11-02 08:24:35.000000000","message":"can you add a comment to explain why this complexity is necessary?\nisn\u0027t simpler to just use threading.Lock()?","commit_id":"92bd12bde363a01fdb1b30935080e1f4f673b060"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"ed7995ba4f75bc9d40d9006038a3a3303aa232e8","unresolved":false,"context_lines":[{"line_number":52,"context_line":"    import eventlet.semaphore"},{"line_number":53,"context_line":"    _exec_sem \u003d eventlet.semaphore.Semaphore()"},{"line_number":54,"context_line":"else:"},{"line_number":55,"context_line":"    _exec_sem \u003d DummySemaphore()"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"ClientManager \u003d daemon.get_manager_class()"},{"line_number":58,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f4b6375_09320911","line":55,"in_reply_to":"3f4b6375_2e8cabd5","updated":"2017-11-02 08:51:33.000000000","message":"Done","commit_id":"92bd12bde363a01fdb1b30935080e1f4f673b060"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"ae6846d3c27b9b43a2b8bf28f3737bbd3620b2fb","unresolved":false,"context_lines":[{"line_number":146,"context_line":"            if self._need_restart:"},{"line_number":147,"context_line":"                proxy \u003d self._restart(proxy)"},{"line_number":148,"context_line":"            try:"},{"line_number":149,"context_line":"                # Usually draining stale data on socket should be enough,"},{"line_number":150,"context_line":"                # but we cannot do that easily."},{"line_number":151,"context_line":"                self._need_restart \u003d True"},{"line_number":152,"context_line":"                res \u003d proxy.run_one_command(cmd, stdin)"},{"line_number":153,"context_line":"                self._need_restart \u003d False"},{"line_number":154,"context_line":"            except (EOFError, IOError):"},{"line_number":155,"context_line":"                retry \u003d True"},{"line_number":156,"context_line":"            # res can be None if we received final None sent by dying"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f4b6375_e65c93ee","line":153,"range":{"start_line":149,"start_character":16,"end_line":153,"end_character":42},"updated":"2017-11-01 19:49:17.000000000","message":"It could be a single helper function reused here and in line 162","commit_id":"92bd12bde363a01fdb1b30935080e1f4f673b060"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"ed7995ba4f75bc9d40d9006038a3a3303aa232e8","unresolved":false,"context_lines":[{"line_number":146,"context_line":"            if self._need_restart:"},{"line_number":147,"context_line":"                proxy \u003d self._restart(proxy)"},{"line_number":148,"context_line":"            try:"},{"line_number":149,"context_line":"                # Usually draining stale data on socket should be enough,"},{"line_number":150,"context_line":"                # but we cannot do that easily."},{"line_number":151,"context_line":"                self._need_restart \u003d True"},{"line_number":152,"context_line":"                res \u003d proxy.run_one_command(cmd, stdin)"},{"line_number":153,"context_line":"                self._need_restart \u003d False"},{"line_number":154,"context_line":"            except (EOFError, IOError):"},{"line_number":155,"context_line":"                retry \u003d True"},{"line_number":156,"context_line":"            # res can be None if we received final None sent by dying"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f4b6375_c942b168","line":153,"range":{"start_line":149,"start_character":16,"end_line":153,"end_character":42},"in_reply_to":"3f4b6375_e65c93ee","updated":"2017-11-02 08:51:33.000000000","message":"Done","commit_id":"92bd12bde363a01fdb1b30935080e1f4f673b060"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"6a79a889500252ab722741a8ab05ce71168668b2","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"if oslo_rootwrap._patched_socket:"},{"line_number":52,"context_line":"    import eventlet.semaphore"},{"line_number":53,"context_line":"    # threading.Lock is not compatible with eventlet."},{"line_number":54,"context_line":"    _exec_sem \u003d eventlet.semaphore.Semaphore()"},{"line_number":55,"context_line":"else:"},{"line_number":56,"context_line":"    # daemon connection is per thread, so no need to lock."}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4b6375_7b3ee0b8","line":53,"range":{"start_line":53,"start_character":0,"end_line":53,"end_character":53},"updated":"2017-11-02 08:58:14.000000000","message":"isn\u0027t it usually patched?\n\nPython 2.7.10 (default, Jul 14 2015, 19:46:27) \n[GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.39)] on darwin\nType \"help\", \"copyright\", \"credits\" or \"license\" for more information.\n\u003e\u003e\u003e import eventlet\n\u003e\u003e\u003e eventlet.monkey_patch()\n\u003e\u003e\u003e import threading\n\u003e\u003e\u003e threading.Lock().__module__\n\u0027eventlet.semaphore\u0027\n\u003e\u003e\u003e","commit_id":"045681f93589c1615de9e527e09c80750c60df69"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"98277085d0e4c58bd241e07421b08c1f3149b6f0","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"if oslo_rootwrap._patched_socket:"},{"line_number":52,"context_line":"    import eventlet.semaphore"},{"line_number":53,"context_line":"    # threading.Lock is not compatible with eventlet."},{"line_number":54,"context_line":"    _exec_sem \u003d eventlet.semaphore.Semaphore()"},{"line_number":55,"context_line":"else:"},{"line_number":56,"context_line":"    # daemon connection is per thread, so no need to lock."}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4b6375_cd45a8b5","line":53,"range":{"start_line":53,"start_character":0,"end_line":53,"end_character":53},"in_reply_to":"3f4b6375_3b57dd5a","updated":"2017-11-08 04:16:48.000000000","message":"Not quite agreed.\n\nMy understanding is that we don\u0027t need this lock in the threading thread impl, but here we go.","commit_id":"045681f93589c1615de9e527e09c80750c60df69"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"032510b23588dc642714cfc416fdf6bac4dd37a4","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"if oslo_rootwrap._patched_socket:"},{"line_number":52,"context_line":"    import eventlet.semaphore"},{"line_number":53,"context_line":"    # threading.Lock is not compatible with eventlet."},{"line_number":54,"context_line":"    _exec_sem \u003d eventlet.semaphore.Semaphore()"},{"line_number":55,"context_line":"else:"},{"line_number":56,"context_line":"    # daemon connection is per thread, so no need to lock."}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4b6375_db58348b","line":53,"range":{"start_line":53,"start_character":0,"end_line":53,"end_character":53},"in_reply_to":"3f4b6375_3bb98814","updated":"2017-11-02 09:18:35.000000000","message":"shouldn\u0027t it be fixed instead? ie. monkey patch earlier.","commit_id":"045681f93589c1615de9e527e09c80750c60df69"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"33ce35a490f63900da037e831d097a04d21ddf6a","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"if oslo_rootwrap._patched_socket:"},{"line_number":52,"context_line":"    import eventlet.semaphore"},{"line_number":53,"context_line":"    # threading.Lock is not compatible with eventlet."},{"line_number":54,"context_line":"    _exec_sem \u003d eventlet.semaphore.Semaphore()"},{"line_number":55,"context_line":"else:"},{"line_number":56,"context_line":"    # daemon connection is per thread, so no need to lock."}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4b6375_3bb98814","line":53,"range":{"start_line":53,"start_character":0,"end_line":53,"end_character":53},"in_reply_to":"3f4b6375_7b3ee0b8","updated":"2017-11-02 09:04:00.000000000","message":"AFAIK monkey patching happens later and cannot help.","commit_id":"045681f93589c1615de9e527e09c80750c60df69"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"d7a5b32d2aaa5c76a39a65b46cda3b20a724aa22","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"if oslo_rootwrap._patched_socket:"},{"line_number":52,"context_line":"    import eventlet.semaphore"},{"line_number":53,"context_line":"    # threading.Lock is not compatible with eventlet."},{"line_number":54,"context_line":"    _exec_sem \u003d eventlet.semaphore.Semaphore()"},{"line_number":55,"context_line":"else:"},{"line_number":56,"context_line":"    # daemon connection is per thread, so no need to lock."}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4b6375_d738c3d3","line":53,"range":{"start_line":53,"start_character":0,"end_line":53,"end_character":53},"in_reply_to":"3f4b6375_c6c25b85","updated":"2017-11-06 04:47:14.000000000","message":"why not?\nisn\u0027t the lock necessary for non-eventlet threading as well?\nanyway it doesn\u0027t seem to worth such micro optimizations here.","commit_id":"045681f93589c1615de9e527e09c80750c60df69"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"62f1179cab4931b411911378baa1b64d8f920559","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"if oslo_rootwrap._patched_socket:"},{"line_number":52,"context_line":"    import eventlet.semaphore"},{"line_number":53,"context_line":"    # threading.Lock is not compatible with eventlet."},{"line_number":54,"context_line":"    _exec_sem \u003d eventlet.semaphore.Semaphore()"},{"line_number":55,"context_line":"else:"},{"line_number":56,"context_line":"    # daemon connection is per thread, so no need to lock."}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4b6375_3b57dd5a","line":53,"range":{"start_line":53,"start_character":0,"end_line":53,"end_character":53},"in_reply_to":"3f4b6375_d738c3d3","updated":"2017-11-07 17:44:36.000000000","message":"Yes, agreed, just apply lock consistently.","commit_id":"045681f93589c1615de9e527e09c80750c60df69"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"b5a1057c05364977aa247db51df06ccb81a801af","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"if oslo_rootwrap._patched_socket:"},{"line_number":52,"context_line":"    import eventlet.semaphore"},{"line_number":53,"context_line":"    # threading.Lock is not compatible with eventlet."},{"line_number":54,"context_line":"    _exec_sem \u003d eventlet.semaphore.Semaphore()"},{"line_number":55,"context_line":"else:"},{"line_number":56,"context_line":"    # daemon connection is per thread, so no need to lock."}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4b6375_c6c25b85","line":53,"range":{"start_line":53,"start_character":0,"end_line":53,"end_character":53},"in_reply_to":"3f4b6375_db58348b","updated":"2017-11-06 01:38:44.000000000","message":"ok _patched_socket\u003d\u003dTrue usually means threading.Lock is also patched, but I still don\u0027t want to apply lock if eventlet isn\u0027t used.","commit_id":"045681f93589c1615de9e527e09c80750c60df69"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"6f2467af48512b668238997683576a8450dfd4be","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        self._process \u003d None"},{"line_number":55,"context_line":"        self._finalize \u003d None"},{"line_number":56,"context_line":"        # This is for eventlet compatibility. multiprocessing stores"},{"line_number":57,"context_line":"        # daemon connection in ProcessLocalSet, so this won\u0027t be"},{"line_number":58,"context_line":"        # needed with the threding module."},{"line_number":59,"context_line":"        self._exec_sem \u003d threading.Lock()"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1f485f77_3b3e9d29","line":57,"range":{"start_line":57,"start_character":31,"end_line":57,"end_character":46},"updated":"2017-11-10 04:39:07.000000000","message":"do you mean ForkAwareLocal?","commit_id":"3fc5e6e9ef293037d76662ded0dd957888b06e9e"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"4c62349fae5bfee1875d0a77d9d33adbea0931ac","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        self._process \u003d None"},{"line_number":55,"context_line":"        self._finalize \u003d None"},{"line_number":56,"context_line":"        # This is for eventlet compatibility. multiprocessing stores"},{"line_number":57,"context_line":"        # daemon connection in ProcessLocalSet, so this won\u0027t be"},{"line_number":58,"context_line":"        # needed with the threding module."},{"line_number":59,"context_line":"        self._exec_sem \u003d threading.Lock()"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1f485f77_1e0f4f2f","line":57,"range":{"start_line":57,"start_character":31,"end_line":57,"end_character":46},"in_reply_to":"1f485f77_1b008134","updated":"2017-11-10 05:22:43.000000000","message":"so, this shouldn\u0027t be necessary for eventlet either, if we can ensure that monkey patch is done before the first import of multiprocessing.util?","commit_id":"3fc5e6e9ef293037d76662ded0dd957888b06e9e"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"33c6571817041d6deb7cdf45c0ce469d71831fde","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        self._process \u003d None"},{"line_number":55,"context_line":"        self._finalize \u003d None"},{"line_number":56,"context_line":"        # This is for eventlet compatibility. multiprocessing stores"},{"line_number":57,"context_line":"        # daemon connection in ProcessLocalSet, so this won\u0027t be"},{"line_number":58,"context_line":"        # needed with the threding module."},{"line_number":59,"context_line":"        self._exec_sem \u003d threading.Lock()"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1f485f77_be9a7bc2","line":57,"range":{"start_line":57,"start_character":31,"end_line":57,"end_character":46},"in_reply_to":"1f485f77_1e0f4f2f","updated":"2017-11-10 05:31:30.000000000","message":"test_functional_eventlet monkey patches pretty early and still need this kind of locks.\n\nyou can track down the root cause if you want. ;-)","commit_id":"3fc5e6e9ef293037d76662ded0dd957888b06e9e"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"b0fdae6cf78cd708a400044d67bcc500b94c0bde","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        self._process \u003d None"},{"line_number":55,"context_line":"        self._finalize \u003d None"},{"line_number":56,"context_line":"        # This is for eventlet compatibility. multiprocessing stores"},{"line_number":57,"context_line":"        # daemon connection in ProcessLocalSet, so this won\u0027t be"},{"line_number":58,"context_line":"        # needed with the threding module."},{"line_number":59,"context_line":"        self._exec_sem \u003d threading.Lock()"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1f485f77_1b008134","line":57,"range":{"start_line":57,"start_character":31,"end_line":57,"end_character":46},"in_reply_to":"1f485f77_3b3e9d29","updated":"2017-11-10 04:57:22.000000000","message":"Yeah, maybe you are right.\nand it\u0027s monkey patched to eventlet.corolocal.local","commit_id":"3fc5e6e9ef293037d76662ded0dd957888b06e9e"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"76727892117d7f010fbd1599a0a18abbb3f3293e","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        self._finalize \u003d None"},{"line_number":56,"context_line":"        # This is for eventlet compatibility. multiprocessing stores"},{"line_number":57,"context_line":"        # daemon connection in ProcessLocalSet, so this won\u0027t be"},{"line_number":58,"context_line":"        # needed with the threding module."},{"line_number":59,"context_line":"        self._exec_sem \u003d threading.Lock()"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def _initialize(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f4b6375_c4f0d394","line":58,"range":{"start_line":58,"start_character":26,"end_line":58,"end_character":34},"updated":"2017-11-08 18:31:21.000000000","message":"threading","commit_id":"3fc5e6e9ef293037d76662ded0dd957888b06e9e"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"b0fdae6cf78cd708a400044d67bcc500b94c0bde","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        self._finalize \u003d None"},{"line_number":56,"context_line":"        # This is for eventlet compatibility. multiprocessing stores"},{"line_number":57,"context_line":"        # daemon connection in ProcessLocalSet, so this won\u0027t be"},{"line_number":58,"context_line":"        # needed with the threding module."},{"line_number":59,"context_line":"        self._exec_sem \u003d threading.Lock()"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def _initialize(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"1f485f77_7bd095ca","line":58,"range":{"start_line":58,"start_character":26,"end_line":58,"end_character":34},"in_reply_to":"3f4b6375_c4f0d394","updated":"2017-11-10 04:57:22.000000000","message":"Done","commit_id":"3fc5e6e9ef293037d76662ded0dd957888b06e9e"}],"oslo_rootwrap/tests/test_functional_eventlet.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"f5a59207570fecc822f16314985efe7e25dac8d1","unresolved":false,"context_lines":[{"line_number":27,"context_line":"            pass"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"        def _thread_worker(self, seconds, msg):"},{"line_number":30,"context_line":"            code, out, err \u003d self.execute("},{"line_number":31,"context_line":"                [\u0027sh\u0027, \u0027-c\u0027, \u0027sleep %d; echo %s\u0027 % (seconds, msg)])"},{"line_number":32,"context_line":"            # Ignore trailing newline"},{"line_number":33,"context_line":"            self.assertEqual(msg, out.rstrip())"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4b6375_e62470b1","line":30,"updated":"2017-10-24 08:44:53.000000000","message":"You need to raise an exception before the output is read from the rwd socket.","commit_id":"b798d80b1c36931d5fc4f81f84e3278ec875807b"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"970fe7263e50560e51533c441a726856176f54f1","unresolved":false,"context_lines":[{"line_number":27,"context_line":"            pass"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"        def _thread_worker(self, seconds, msg):"},{"line_number":30,"context_line":"            code, out, err \u003d self.execute("},{"line_number":31,"context_line":"                [\u0027sh\u0027, \u0027-c\u0027, \u0027sleep %d; echo %s\u0027 % (seconds, msg)])"},{"line_number":32,"context_line":"            # Ignore trailing newline"},{"line_number":33,"context_line":"            self.assertEqual(msg, out.rstrip())"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f4b6375_46ba24c1","line":30,"in_reply_to":"3f4b6375_e62470b1","updated":"2017-10-24 09:00:10.000000000","message":"Yup, we might need to raise an timeoutexception to break the waiting thread to reproduce the same failure mode.\n\nHowever, it seems the root cause is sharing the connection between eventlet threads and AFAIK oslo.rootwrap isn\u0027t very eventlet friendly.","commit_id":"b798d80b1c36931d5fc4f81f84e3278ec875807b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"340b2d306ba804df7b871d4b87e9445cae74a4f4","unresolved":false,"context_lines":[{"line_number":39,"context_line":"                except eventlet.Timeout:"},{"line_number":40,"context_line":"                    pass"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        def test_eventlet_threads(self):"},{"line_number":43,"context_line":"            th \u003d []"},{"line_number":44,"context_line":"            # 10 was not enough for some reason."},{"line_number":45,"context_line":"            for i in range(15):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4b6375_fe7481d2","line":42,"updated":"2017-10-27 20:04:26.000000000","message":"the test case is not self-describing. Consider renaming the method to reflect the intent better and/or add docstring or a comment explaining the scenario.","commit_id":"4445ffb4749a721da2ca2cdf358dfcbc0e8c19dc"},{"author":{"_account_id":9200,"name":"IWAMOTO Toshihiro","email":"iwamoto@valinux.co.jp","username":"toshii"},"change_message_id":"b137b1acded80d6a4c1d5ba8c353c5f82bf94eb8","unresolved":false,"context_lines":[{"line_number":39,"context_line":"                except eventlet.Timeout:"},{"line_number":40,"context_line":"                    pass"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        def test_eventlet_threads(self):"},{"line_number":43,"context_line":"            th \u003d []"},{"line_number":44,"context_line":"            # 10 was not enough for some reason."},{"line_number":45,"context_line":"            for i in range(15):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4b6375_2a09572b","line":42,"in_reply_to":"3f4b6375_fe7481d2","updated":"2017-11-01 09:17:49.000000000","message":"Done","commit_id":"4445ffb4749a721da2ca2cdf358dfcbc0e8c19dc"}]}