)]}'
{"oslo_concurrency/processutils.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"ad35ddcaf032b3586d137c7c7c9ce5ed9b9059f3","unresolved":false,"context_lines":[{"line_number":42,"context_line":"# time module as the check because that\u0027s a monkey patched module we use"},{"line_number":43,"context_line":"# in combination with subprocess below, so they need to match."},{"line_number":44,"context_line":"eventlet \u003d importutils.try_import(\u0027eventlet\u0027)"},{"line_number":45,"context_line":"eventlet_patched \u003d eventlet and eventlet.patcher.is_monkey_patched(time)"},{"line_number":46,"context_line":"if eventlet_patched:"},{"line_number":47,"context_line":"    if os.name \u003d\u003d \u0027nt\u0027:"},{"line_number":48,"context_line":"        # subprocess.Popen.communicate will spawn two threads consuming"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f436f4f_10e55e09","line":45,"range":{"start_line":45,"start_character":49,"end_line":45,"end_character":72},"updated":"2017-08-14 15:02:23.000000000","message":"question: This checks if \u0027time\u0027 is monkey-patched. is it sufficient?\n\nL.53-54 indicates eventlet to use the original version of subprocess and thread. I wonder what happens if eventlet.monkey_patch(thread\u003dTrue) for example.\n\n[1] http://eventlet.net/doc/patching.html#monkeypatching-the-standard-library","commit_id":"b6b6673a7d2a96c47891799e66e18d8010d7a64f"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"472eee69ad0e4fde92668e88007229bbea4601e9","unresolved":false,"context_lines":[{"line_number":42,"context_line":"# time module as the check because that\u0027s a monkey patched module we use"},{"line_number":43,"context_line":"# in combination with subprocess below, so they need to match."},{"line_number":44,"context_line":"eventlet \u003d importutils.try_import(\u0027eventlet\u0027)"},{"line_number":45,"context_line":"eventlet_patched \u003d eventlet and eventlet.patcher.is_monkey_patched(time)"},{"line_number":46,"context_line":"if eventlet_patched:"},{"line_number":47,"context_line":"    if os.name \u003d\u003d \u0027nt\u0027:"},{"line_number":48,"context_line":"        # subprocess.Popen.communicate will spawn two threads consuming"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f436f4f_64686f1c","line":45,"range":{"start_line":45,"start_character":49,"end_line":45,"end_character":72},"in_reply_to":"9f436f4f_10e55e09","updated":"2017-08-14 15:42:41.000000000","message":"I\u0027d say that\u0027s reasonable. I mean, I don\u0027t see why someone would use eventlet without patching the \u0027thread\u0027 and \u0027time\u0027 modules. That would cause other issues.","commit_id":"b6b6673a7d2a96c47891799e66e18d8010d7a64f"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"bc22d6a03318d8bdf3b6aa0eda126d01df1be280","unresolved":false,"context_lines":[{"line_number":42,"context_line":"# time module as the check because that\u0027s a monkey patched module we use"},{"line_number":43,"context_line":"# in combination with subprocess below, so they need to match."},{"line_number":44,"context_line":"eventlet \u003d importutils.try_import(\u0027eventlet\u0027)"},{"line_number":45,"context_line":"eventlet_patched \u003d eventlet and eventlet.patcher.is_monkey_patched(time)"},{"line_number":46,"context_line":"if eventlet_patched:"},{"line_number":47,"context_line":"    if os.name \u003d\u003d \u0027nt\u0027:"},{"line_number":48,"context_line":"        # subprocess.Popen.communicate will spawn two threads consuming"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f436f4f_8a80e8c0","line":45,"range":{"start_line":45,"start_character":49,"end_line":45,"end_character":72},"in_reply_to":"9f436f4f_64686f1c","updated":"2017-08-15 08:37:35.000000000","message":"Although monkey_patch is usually called without arguments (which means \u0027all\u0027), according to the code search result [1] your assumption is necessarily correct. My suggestion is to specify a corresponding module name which you exactly want. For example, manila code monkey_patch-es subprocess but it does not patched time. In this case, monkey-patched \u0027subprocess\u0027 will be used. I see similar cases in nova code too.\n\n[1] http://codesearch.openstack.org/?q\u003dmonkey_patch\u0026i\u003dnope\u0026files\u003d\u0026repos\u003d","commit_id":"b6b6673a7d2a96c47891799e66e18d8010d7a64f"},{"author":{"_account_id":9796,"name":"ChangBo Guo","email":"glongwave@gmail.com","username":"gcb"},"change_message_id":"652906342445ee9b15985882468e6d31a367d4a6","unresolved":false,"context_lines":[{"line_number":42,"context_line":"# time module as the check because that\u0027s a monkey patched module we use"},{"line_number":43,"context_line":"# in combination with subprocess below, so they need to match."},{"line_number":44,"context_line":"eventlet \u003d importutils.try_import(\u0027eventlet\u0027)"},{"line_number":45,"context_line":"eventlet_patched \u003d eventlet and eventlet.patcher.is_monkey_patched(time)"},{"line_number":46,"context_line":"if eventlet_patched:"},{"line_number":47,"context_line":"    if os.name \u003d\u003d \u0027nt\u0027:"},{"line_number":48,"context_line":"        # subprocess.Popen.communicate will spawn two threads consuming"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f436f4f_fa6e1dc7","line":45,"range":{"start_line":45,"start_character":49,"end_line":45,"end_character":72},"in_reply_to":"9f436f4f_8a80e8c0","updated":"2017-08-15 11:41:34.000000000","message":"See the comments from bnemec, I\u0027m not sure which way is better, this commit doesn\u0027t change the logic here, Another patch to fix the issue is appreciated if we need.","commit_id":"b6b6673a7d2a96c47891799e66e18d8010d7a64f"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"6e7ad5e0f084bf8e468d617a3c333a18d799a479","unresolved":false,"context_lines":[{"line_number":42,"context_line":"# time module as the check because that\u0027s a monkey patched module we use"},{"line_number":43,"context_line":"# in combination with subprocess below, so they need to match."},{"line_number":44,"context_line":"eventlet \u003d importutils.try_import(\u0027eventlet\u0027)"},{"line_number":45,"context_line":"eventlet_patched \u003d eventlet and eventlet.patcher.is_monkey_patched(time)"},{"line_number":46,"context_line":"if eventlet_patched:"},{"line_number":47,"context_line":"    if os.name \u003d\u003d \u0027nt\u0027:"},{"line_number":48,"context_line":"        # subprocess.Popen.communicate will spawn two threads consuming"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f436f4f_062e1591","line":45,"range":{"start_line":45,"start_character":49,"end_line":45,"end_character":72},"in_reply_to":"9f436f4f_8a80e8c0","updated":"2017-08-16 08:32:13.000000000","message":"So regardless of what modules are patched, we cannot use the eventlet subprocess module on Windows. We have to use the original one, just wrapping the communicate call so that it\u0027s greenthread friendly (if needed).\n\nNow, the only thing we need in this case is to see whether exec calls are expected to be \u0027green\u0027 or not. As ChangBo mentioned, this was previously decided based on the \u0027time\u0027 module. This patch doesn\u0027t change this check (can be done in a separate patch as it\u0027s unrelated to this bug fix).","commit_id":"b6b6673a7d2a96c47891799e66e18d8010d7a64f"},{"author":{"_account_id":9796,"name":"ChangBo Guo","email":"glongwave@gmail.com","username":"gcb"},"change_message_id":"652906342445ee9b15985882468e6d31a367d4a6","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        # stdout/stderr when passing data through stdin. We need to make"},{"line_number":50,"context_line":"        # sure that *native* threads will be used as pipes are blocking"},{"line_number":51,"context_line":"        # on Windows."},{"line_number":52,"context_line":"        # Recent eventlet versions actually do patch subprocess."},{"line_number":53,"context_line":"        subprocess \u003d eventlet.patcher.original(\u0027subprocess\u0027)"},{"line_number":54,"context_line":"        subprocess.threading \u003d eventlet.patcher.original(\u0027threading\u0027)"},{"line_number":55,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f436f4f_5a2f6987","line":52,"range":{"start_line":52,"start_character":8,"end_line":52,"end_character":64},"updated":"2017-08-15 11:41:34.000000000","message":"can you add more information about eventlet change to indicate if we can remove the workaround in the future.","commit_id":"b6b6673a7d2a96c47891799e66e18d8010d7a64f"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"6e7ad5e0f084bf8e468d617a3c333a18d799a479","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        # stdout/stderr when passing data through stdin. We need to make"},{"line_number":50,"context_line":"        # sure that *native* threads will be used as pipes are blocking"},{"line_number":51,"context_line":"        # on Windows."},{"line_number":52,"context_line":"        # Recent eventlet versions actually do patch subprocess."},{"line_number":53,"context_line":"        subprocess \u003d eventlet.patcher.original(\u0027subprocess\u0027)"},{"line_number":54,"context_line":"        subprocess.threading \u003d eventlet.patcher.original(\u0027threading\u0027)"},{"line_number":55,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f436f4f_abd39203","line":52,"range":{"start_line":52,"start_character":8,"end_line":52,"end_character":64},"in_reply_to":"9f436f4f_5a2f6987","updated":"2017-08-16 08:32:13.000000000","message":"I\u0027ve just checked. So as of eventlet 0.20, the subprocess module does get patched by default. Still, the global requirements file requests eventlet\u003e\u003d0.18.2 (https://github.com/openstack/requirements/blob/1cd2c8da1a6534c23959cd0a4035e7f530471386/global-requirements.txt#L56)\n\nThat being considered, I think we should leave that check in place for now.","commit_id":"b6b6673a7d2a96c47891799e66e18d8010d7a64f"},{"author":{"_account_id":9796,"name":"ChangBo Guo","email":"glongwave@gmail.com","username":"gcb"},"change_message_id":"652906342445ee9b15985882468e6d31a367d4a6","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    else:"},{"line_number":56,"context_line":"        from eventlet.green import subprocess"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    from eventlet import tpool"},{"line_number":59,"context_line":"else:"},{"line_number":60,"context_line":"    import subprocess"},{"line_number":61,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9f436f4f_3a794593","line":58,"range":{"start_line":58,"start_character":0,"end_line":58,"end_character":30},"updated":"2017-08-15 11:41:34.000000000","message":"this is used only when eventlet_patched and os.name \u003d\u003d \u0027nt\u0027:\n\nmove this into line 53 or line 396 ?","commit_id":"b6b6673a7d2a96c47891799e66e18d8010d7a64f"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"09dd56916967228030dc9287d95f208d9a8b72e3","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    else:"},{"line_number":56,"context_line":"        from eventlet.green import subprocess"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    from eventlet import tpool"},{"line_number":59,"context_line":"else:"},{"line_number":60,"context_line":"    import subprocess"},{"line_number":61,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9f436f4f_b3042416","line":58,"range":{"start_line":58,"start_character":0,"end_line":58,"end_character":30},"in_reply_to":"9f436f4f_3a794593","updated":"2017-08-16 10:58:55.000000000","message":"At a second thought, I\u0027d leave it here to simplify unit tests.","commit_id":"b6b6673a7d2a96c47891799e66e18d8010d7a64f"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"6e7ad5e0f084bf8e468d617a3c333a18d799a479","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    else:"},{"line_number":56,"context_line":"        from eventlet.green import subprocess"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    from eventlet import tpool"},{"line_number":59,"context_line":"else:"},{"line_number":60,"context_line":"    import subprocess"},{"line_number":61,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9f436f4f_e6f1d102","line":58,"range":{"start_line":58,"start_character":0,"end_line":58,"end_character":30},"in_reply_to":"9f436f4f_3a794593","updated":"2017-08-16 08:32:13.000000000","message":"Done","commit_id":"b6b6673a7d2a96c47891799e66e18d8010d7a64f"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"789b2576292652c38c2a05dbdefca54f6bd049fe","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        # on Windows."},{"line_number":52,"context_line":"        # Recent eventlet versions actually do patch subprocess."},{"line_number":53,"context_line":"        subprocess \u003d eventlet.patcher.original(\u0027subprocess\u0027)"},{"line_number":54,"context_line":"        subprocess.threading \u003d eventlet.patcher.original(\u0027threading\u0027)"},{"line_number":55,"context_line":"    else:"},{"line_number":56,"context_line":"        from eventlet.green import subprocess"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f436f4f_b9660829","line":54,"updated":"2017-08-18 09:58:13.000000000","message":"This means patching subprocess module back, it affects not only this library, but whole program. Can we avoid doing that and rely on user not patching subprocess at all?","commit_id":"3ac3c169ada2e1402ef9e71f0653af696a3d28af"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"71492e0eb0dcb2ae289494a3f5d2705496cb55db","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        # on Windows."},{"line_number":52,"context_line":"        # Recent eventlet versions actually do patch subprocess."},{"line_number":53,"context_line":"        subprocess \u003d eventlet.patcher.original(\u0027subprocess\u0027)"},{"line_number":54,"context_line":"        subprocess.threading \u003d eventlet.patcher.original(\u0027threading\u0027)"},{"line_number":55,"context_line":"    else:"},{"line_number":56,"context_line":"        from eventlet.green import subprocess"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f436f4f_b4984f1b","line":54,"in_reply_to":"9f436f4f_14471bbf","updated":"2017-08-18 10:12:22.000000000","message":"I meant to actually replace this \"repatching\" with assertion that subprocess is not patched. That would mean that we signal caller that patching subprocess on Windows yields problems.\n\nBut I see your point, we hardly affect caller here, thanks for explanation.","commit_id":"3ac3c169ada2e1402ef9e71f0653af696a3d28af"},{"author":{"_account_id":8543,"name":"Lucian Petrut","email":"lpetrut@cloudbasesolutions.com","username":"plucian"},"change_message_id":"be150192ac4923482cf9b232654f1d4c2438252a","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        # on Windows."},{"line_number":52,"context_line":"        # Recent eventlet versions actually do patch subprocess."},{"line_number":53,"context_line":"        subprocess \u003d eventlet.patcher.original(\u0027subprocess\u0027)"},{"line_number":54,"context_line":"        subprocess.threading \u003d eventlet.patcher.original(\u0027threading\u0027)"},{"line_number":55,"context_line":"    else:"},{"line_number":56,"context_line":"        from eventlet.green import subprocess"},{"line_number":57,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f436f4f_14471bbf","line":54,"in_reply_to":"9f436f4f_b9660829","updated":"2017-08-18 10:05:28.000000000","message":"It\u0027s just this module that\u0027s using the unpatched subprocess module, other subprocess imports throughout the caller code are unaffected.\n\nFor example, if subprocess was already patched, processutils will use the unpatched version while sys.modules will still contain the patched subprocess module.\n\nConsidering that this is a simple, unintrusive change, I\u0027d rather have this check here than all the caller modules.","commit_id":"3ac3c169ada2e1402ef9e71f0653af696a3d28af"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"0f1dbfc22a7d845cec12b29194466ed5b200d294","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    else:"},{"line_number":56,"context_line":"        from eventlet.green import subprocess"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    from eventlet import tpool"},{"line_number":59,"context_line":"else:"},{"line_number":60,"context_line":"    import subprocess"},{"line_number":61,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f436f4f_f3ececbb","line":58,"updated":"2017-08-16 11:32:13.000000000","message":"Honestly I don\u0027t like to do this only for testing, but I don\u0027t see any good way....","commit_id":"3ac3c169ada2e1402ef9e71f0653af696a3d28af"}],"oslo_concurrency/tests/unit/test_processutils.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"979d7ff0566bd1acb4c39309414d2dec5f9fd0a5","unresolved":false,"context_lines":[{"line_number":127,"context_line":"    @mock.patch.object(os, \u0027name\u0027, \u0027nt\u0027)"},{"line_number":128,"context_line":"    @mock.patch.object(processutils.subprocess, \"Popen\")"},{"line_number":129,"context_line":"    @mock.patch.object(processutils, \u0027tpool\u0027, create\u003dTrue)"},{"line_number":130,"context_line":"    def _test_windows_execute(self, mock_tpool, mock_popen,"},{"line_number":131,"context_line":"                              use_eventlet\u003dFalse):"},{"line_number":132,"context_line":"        # We want to ensure that if eventlet is used on Windows,"},{"line_number":133,"context_line":"        # \u0027communicate\u0027 calls are wrapped with eventlet.tpool.execute."}],"source_content_type":"text/x-python","patch_set":6,"id":"9f436f4f_d3ba08bf","line":130,"range":{"start_line":130,"start_character":36,"end_line":130,"end_character":59},"updated":"2017-08-16 11:28:41.000000000","message":"There are three mock.patch.object above. Don\u0027t you need to have an argument corresponding to os.name?","commit_id":"3ac3c169ada2e1402ef9e71f0653af696a3d28af"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"0f1dbfc22a7d845cec12b29194466ed5b200d294","unresolved":false,"context_lines":[{"line_number":127,"context_line":"    @mock.patch.object(os, \u0027name\u0027, \u0027nt\u0027)"},{"line_number":128,"context_line":"    @mock.patch.object(processutils.subprocess, \"Popen\")"},{"line_number":129,"context_line":"    @mock.patch.object(processutils, \u0027tpool\u0027, create\u003dTrue)"},{"line_number":130,"context_line":"    def _test_windows_execute(self, mock_tpool, mock_popen,"},{"line_number":131,"context_line":"                              use_eventlet\u003dFalse):"},{"line_number":132,"context_line":"        # We want to ensure that if eventlet is used on Windows,"},{"line_number":133,"context_line":"        # \u0027communicate\u0027 calls are wrapped with eventlet.tpool.execute."}],"source_content_type":"text/x-python","patch_set":6,"id":"9f436f4f_73011c6a","line":130,"range":{"start_line":130,"start_character":36,"end_line":130,"end_character":59},"in_reply_to":"9f436f4f_d3ba08bf","updated":"2017-08-16 11:32:13.000000000","message":"Sorry, I was wrong. mock.patch.object at L.127 specifies the new value, so there is no need for an argument.","commit_id":"3ac3c169ada2e1402ef9e71f0653af696a3d28af"}]}