)]}'
{"cinder/privsep/__init__.py":[{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"1a463d6f83692c51f8f21f0c76597266f35d708a","unresolved":false,"context_lines":[{"line_number":26,"context_line":"# utilize \u0027oslo_concurrency.processutils\u0027 to execute commands, we need to"},{"line_number":27,"context_line":"# monkey patch the \u0027time\u0027 module here again in order to make"},{"line_number":28,"context_line":"# processutils eventlet friendly."},{"line_number":29,"context_line":"eventlet.monkey_patch(time\u003dTrue)"},{"line_number":30,"context_line":"sys_admin_pctxt \u003d priv_context.PrivContext("},{"line_number":31,"context_line":"    \u0027cinder\u0027,"},{"line_number":32,"context_line":"    cfg_section\u003d\u0027cinder_sys_admin\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_06472679","line":29,"updated":"2018-08-24 17:36:20.000000000","message":"This raises some red flags for me.  Per http://specs.openstack.org/openstack/openstack-specs/specs/eventlet-best-practices.html#monkey-patching it is best to do monkey patching as early as possible in the application, and doing it in a submodule after other things are imported can be problematic.  It\u0027s also tricky to monkey patch selectively like this as some modules don\u0027t like it when you mix green and non-green versions (hence the logic in subprocess).\n\nI guess I don\u0027t understand why this is needed either.  If Cinder isn\u0027t monkey patching, then it shouldn\u0027t matter if subprocess uses the green module, and if Cinder is monkey patching, then why isn\u0027t time patched with everything else?","commit_id":"f151d2a4d4a1b8bdad6389c0730da45e169e77ae"},{"author":{"_account_id":23083,"name":"TommyLike","email":"tommylikehu@gmail.com","username":"TommyLike"},"change_message_id":"6fd316c5018818279d10f60fa0605cce1922a511","unresolved":false,"context_lines":[{"line_number":26,"context_line":"# utilize \u0027oslo_concurrency.processutils\u0027 to execute commands, we need to"},{"line_number":27,"context_line":"# monkey patch the \u0027time\u0027 module here again in order to make"},{"line_number":28,"context_line":"# processutils eventlet friendly."},{"line_number":29,"context_line":"eventlet.monkey_patch(time\u003dTrue)"},{"line_number":30,"context_line":"sys_admin_pctxt \u003d priv_context.PrivContext("},{"line_number":31,"context_line":"    \u0027cinder\u0027,"},{"line_number":32,"context_line":"    cfg_section\u003d\u0027cinder_sys_admin\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_3e1a5fc5","line":29,"in_reply_to":"3f79a3b5_06472679","updated":"2018-08-25 02:10:35.000000000","message":"I re-monkey patch the time module here is just because when privsep execute command, it will only import this module and then execute, so, no matter whether it\u0027s patched in cinder. we have to re-monkey patch these modules. Correct me if I am wrong:)\n\n[1]: https://github.com/openstack/oslo.privsep/blob/b1f50df3b1cde1b8c770d810e201077aa1bec921/oslo_privsep/daemon.py#L422 \n[2]: https://github.com/openstack/oslo.concurrency/blob/master/oslo_concurrency/processutils.py#L45","commit_id":"f151d2a4d4a1b8bdad6389c0730da45e169e77ae"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"ecc0e5c32d7b4fca8851232ca251c1c742224f9e","unresolved":false,"context_lines":[{"line_number":26,"context_line":"# utilize \u0027oslo_concurrency.processutils\u0027 to execute commands, we need to"},{"line_number":27,"context_line":"# monkey patch the \u0027time\u0027 module here again in order to make"},{"line_number":28,"context_line":"# processutils eventlet friendly."},{"line_number":29,"context_line":"eventlet.monkey_patch(time\u003dTrue)"},{"line_number":30,"context_line":"sys_admin_pctxt \u003d priv_context.PrivContext("},{"line_number":31,"context_line":"    \u0027cinder\u0027,"},{"line_number":32,"context_line":"    cfg_section\u003d\u0027cinder_sys_admin\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_df03d075","line":29,"in_reply_to":"3f79a3b5_3e1a5fc5","updated":"2018-08-29 22:16:27.000000000","message":"Oh, I may have to take back my comment on the other review about green threads not being a problem.  I forgot that project code gets imported into the daemon process, and if that\u0027s assuming one type of threading and we give it another bad things will happen. :-/","commit_id":"f151d2a4d4a1b8bdad6389c0730da45e169e77ae"}]}