)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"b1354e9bd04eede87a13b44b41cd6b14f9df11e2","unresolved":false,"context_lines":[{"line_number":13,"context_line":"module. The args(os\u003dFalse) make nova process has to wait while executing"},{"line_number":14,"context_line":"cmd in subprocess, which seems to be unnecessary."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"I make this patch to see if any check or gate would fail by removing"},{"line_number":17,"context_line":"os\u003dFalse in monkey_patch. If all check get pass, we may consider to"},{"line_number":18,"context_line":"merge this patch."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: I8b456676a04b9066cb2b570060c0d95cd4fe69f8"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5f7c97a3_b7009bad","line":18,"range":{"start_line":16,"start_character":0,"end_line":18,"end_character":17},"updated":"2018-05-17 08:25:42.000000000","message":"not sure it\u0027s a good reason....\n\nI think we need more specific reason in doing something\nlike this param is not used, or this don\u0027t need anymore or we might suffer some edge case ..\n\n\nquick search from http://eventlet.net/doc/basic_usage.html#primary-api\nand seems os\u003dfalse means no patch on os package\nso guess it might lead to some error.","commit_id":"7b1adaa15cbcb8a886e46974c378c2f22bb4b67d"},{"author":{"_account_id":19849,"name":"JiaJunsu","email":"jiajunsu@huawei.com","username":"jiajunsu"},"change_message_id":"1328089f3b131fe784b15e17d054a5d30c9bc9f5","unresolved":false,"context_lines":[{"line_number":13,"context_line":"module. The args(os\u003dFalse) make nova process has to wait while executing"},{"line_number":14,"context_line":"cmd in subprocess, which seems to be unnecessary."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"I make this patch to see if any check or gate would fail by removing"},{"line_number":17,"context_line":"os\u003dFalse in monkey_patch. If all check get pass, we may consider to"},{"line_number":18,"context_line":"merge this patch."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: I8b456676a04b9066cb2b570060c0d95cd4fe69f8"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5f7c97a3_1c35ddf7","line":18,"range":{"start_line":16,"start_character":0,"end_line":18,"end_character":17},"in_reply_to":"5f7c97a3_b7009bad","updated":"2018-05-18 01:59:29.000000000","message":"Some functional-py35 tests failed in check, I\u0027ll identify them.\nThe patched function in os module is [\u0027fdopen\u0027, \u0027read\u0027, \u0027write\u0027, \u0027wait\u0027, \u0027waitpid\u0027, \u0027open\u0027], mainly influence file-related workflows.\n\nI\u0027ll update this info in commit-msg.","commit_id":"7b1adaa15cbcb8a886e46974c378c2f22bb4b67d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"927bf86c9602e1d93102a1beffb5ffb4e58cc5df","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Remove args(os\u003dFalse) in monkey_patch"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The first patch introduced os\u003dFalse is"},{"line_number":10,"context_line":"https://review.openstack.org/#/c/11556/"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"But there is no explaination why nova should not monkey_patch `os`"},{"line_number":13,"context_line":"module. The args(os\u003dFalse) make nova process has to wait while calling"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"ffd0ebdf_74da04ea","line":10,"updated":"2019-01-04 15:17:44.000000000","message":"Looking at this, it\u0027s definitely not clear why os\u003dFalse was chosen, but my guess is it was just copied from one of the existing files and no one noticed because it was a big change.\n\nFor example, the nova-compute script was conditional based on whether or not it was running on windows:\n\nhttps://review.openstack.org/#/c/11556/29/bin/nova-compute\n\nbut the conductor script didn\u0027t patch os:\n\nhttps://review.openstack.org/#/c/11556/29/bin/nova-conductor\n\nPresumably because at the time, people figured conductor would never run on windows (only compute would need to support windows for hyper-v).\n\nThe nova-all script (which probably no one ever used) unconditionally set os\u003dFalse:\n\nhttps://review.openstack.org/#/c/11556/29/bin/nova-all\n\nI suppose because if you were going to use that, it would be on a single host (like devstack) and required to be linux.\n\nSo then the monkey_patch was moved here:\n\nhttps://review.openstack.org/#/c/11556/29/nova/cmd/__init__.py\n\nAnd I assume os\u003dFalse simply as the lowest common denominator to account for the windows case in nova-compute.\n\nIn fact, cinder-volume still has that conditional on Windows:\n\nhttps://github.com/openstack/cinder/blob/84d268d9a0d0393d5ee81063c36e7d19e57fc647/cinder/cmd/volume.py#L27\n\nSo I guess my main concern would be if we need that same conditional in nova for the nova-compute service to run on Windows (for Hyper-v). Having said that, I don\u0027t see anything special about monkey patching the os module in the compute-hyperv repo, so maybe it\u0027s no longer an issue? Or Hyper-v has worked around it some other way? It would be good to get sign off from Claudiu or Lucian to be sure.","commit_id":"76cfab3c34d3286a7df5ba440c5c0acf1b71e971"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"811d6903cc79a91d99dd382fece00cb2a67ee36d","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Remove args(os\u003dFalse) in monkey_patch"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The first patch introduced os\u003dFalse is"},{"line_number":10,"context_line":"https://review.openstack.org/#/c/11556/"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"But there is no explaination why nova should not monkey_patch `os`"},{"line_number":13,"context_line":"module. The args(os\u003dFalse) make nova process has to wait while calling"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"ffd0ebdf_8873c780","line":10,"in_reply_to":"ffd0ebdf_74da04ea","updated":"2019-01-04 17:08:09.000000000","message":"so based on that i guess we would either want to make this condtional on if its windows or abandon the patch and just continue to have os\u003dFalse but add a comment to say it is required for windows","commit_id":"76cfab3c34d3286a7df5ba440c5c0acf1b71e971"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a7fd24056671e8c780b039186cf9073af90bbf2c","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Remove args(os\u003dFalse) in monkey_patch"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The first patch introduced os\u003dFalse is"},{"line_number":10,"context_line":"https://review.openstack.org/#/c/11556/"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"But there is no explaination why nova should not monkey_patch `os`"},{"line_number":13,"context_line":"module. The args(os\u003dFalse) make nova process has to wait while calling"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"ffd0ebdf_ce05547c","line":10,"in_reply_to":"ffd0ebdf_8873c780","updated":"2019-01-04 21:51:52.000000000","message":"I\u0027d just make it conditional on windows.","commit_id":"76cfab3c34d3286a7df5ba440c5c0acf1b71e971"}],"nova/test.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"12485c2e5899c86c6f425241ab51c063d4349b17","unresolved":false,"context_lines":[{"line_number":22,"context_line":"\"\"\""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"import eventlet  # noqa"},{"line_number":25,"context_line":"eventlet.monkey_patch()"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"import abc"},{"line_number":28,"context_line":"import contextlib"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_f98f0fca","line":25,"range":{"start_line":25,"start_character":0,"end_line":25,"end_character":23},"updated":"2018-08-27 13:22:50.000000000","message":"we probably want to have the same\ndebugging behavior in the unit and fuctional test also \n\nif debugger.enabled():\n    # turn off thread patching to enable the remote debugger\n    eventlet.monkey_patch(thread\u003dFalse)\nelse:\n    eventlet.monkey_patch()\n\nso that you can remote debug them with the same behavior as the real code under a debuggger.\n\nbut i guess that could be a seperate patch.","commit_id":"f1438f187e37c09b9f666db3b2e9ab3942ab07ee"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"69cc96c66fcca2ebd60f587e2612296b139b2936","unresolved":false,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":"import eventlet  # noqa"},{"line_number":25,"context_line":"import os"},{"line_number":26,"context_line":"if os.name \u003d\u003d \u0027nt\u0027:"},{"line_number":27,"context_line":"    # for nova-compute running on Windows(Hyper-v)"},{"line_number":28,"context_line":"    # pipes don\u0027t support non-blocking I/O"},{"line_number":29,"context_line":"    eventlet.monkey_patch(os\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":6,"id":"ffd0ebdf_ce2a34e6","line":26,"updated":"2019-01-04 21:55:10.000000000","message":"I don\u0027t think we really need this for test code. Note that you didn\u0027t do it here either:\n\nhttps://review.openstack.org/#/c/568999/6/nova/tests/functional/__init__.py\n\nAnd I don\u0027t think it\u0027s needed (for tests).","commit_id":"a65582ae899b2992ca20954ab60c595ca31bbc19"}],"nova/utils.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"927bf86c9602e1d93102a1beffb5ffb4e58cc5df","unresolved":false,"context_lines":[{"line_number":1295,"context_line":"        # turn off thread patching to enable the remote debugger"},{"line_number":1296,"context_line":"        eventlet.monkey_patch(thread\u003dFalse)"},{"line_number":1297,"context_line":"    else:"},{"line_number":1298,"context_line":"        eventlet.monkey_patch()"},{"line_number":1299,"context_line":""},{"line_number":1300,"context_line":"    # NOTE(rgerganov): oslo.context is storing a global thread-local variable"},{"line_number":1301,"context_line":"    # which keeps the request context for the current thread. If oslo.context"}],"source_content_type":"text/x-python","patch_set":5,"id":"ffd0ebdf_34779c3d","line":1298,"updated":"2019-01-04 15:17:44.000000000","message":"Unless I\u0027m mistaken, this means everything is monkey patched now, right?\n\nhttp://eventlet.net/doc/basic_usage.html#eventlet.monkey_patch\n\nhttps://github.com/eventlet/eventlet/blob/v0.24.1/eventlet/patcher.py#L213","commit_id":"76cfab3c34d3286a7df5ba440c5c0acf1b71e971"}]}