)]}'
{".pre-commit-config.yaml":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f3e8b311142d91fa7217140d4a4b8a99cb54d7cf","unresolved":true,"context_lines":[{"line_number":39,"context_line":"      - id: autopep8"},{"line_number":40,"context_line":"        files: \u0027^.*\\.py$\u0027"},{"line_number":41,"context_line":"        args: [\u0027--in-place\u0027, \u0027--exclude\u0027, \u0027nova/api/openstack/__init__.py\u0027]"},{"line_number":42,"context_line":"        # E402 is skipped for conditional eventlet monkey patching"},{"line_number":43,"context_line":"      - id: autopep8"},{"line_number":44,"context_line":"        files: \u0027nova/api/openstack/__init__.py\u0027"},{"line_number":45,"context_line":"        args: [\u0027--ignore\u0027, \u0027E402\u0027]"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"91a03be2_669a4365","line":42,"updated":"2024-01-09 13:14:41.000000000","message":"could we do that with #noqa or simialr","commit_id":"9aa65c30610e686d006af14a639d38cdb091300b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"51f4c1109023b7c1e4d3179a360bedae18895b18","unresolved":true,"context_lines":[{"line_number":39,"context_line":"      - id: autopep8"},{"line_number":40,"context_line":"        files: \u0027^.*\\.py$\u0027"},{"line_number":41,"context_line":"        args: [\u0027--in-place\u0027, \u0027--exclude\u0027, \u0027nova/api/openstack/__init__.py\u0027]"},{"line_number":42,"context_line":"        # E402 is skipped for conditional eventlet monkey patching"},{"line_number":43,"context_line":"      - id: autopep8"},{"line_number":44,"context_line":"        files: \u0027nova/api/openstack/__init__.py\u0027"},{"line_number":45,"context_line":"        args: [\u0027--ignore\u0027, \u0027E402\u0027]"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"37321c96_6b9c81ba","line":42,"in_reply_to":"91a03be2_669a4365","updated":"2024-01-09 20:15:25.000000000","message":"We could. I did all this trying to avoid having to add #noqa to 7ish lines in the same file but that\u0027s probably actually cleaner than what I\u0027ve ended up doing here. I\u0027ll change it.","commit_id":"9aa65c30610e686d006af14a639d38cdb091300b"}],"/COMMIT_MSG":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a94fab91e75857fd45d75db0ff76951fbe631488","unresolved":true,"context_lines":[{"line_number":9,"context_line":"This replaces the direct usage of eventlet greenthreads and queue with"},{"line_number":10,"context_line":"the futurist.ThreadPoolExecutor, which uses native threads underneath,"},{"line_number":11,"context_line":"if eventlet patching is disabled."},{"line_number":12,"context_line":"."},{"line_number":13,"context_line":"Futurist has different executors available that use the same interface,"},{"line_number":14,"context_line":"so this can be easily swapped out in the future if we find we need a"},{"line_number":15,"context_line":"different executor. Using a futurist executor is also handy for unit"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":16,"id":"393aae62_0bc54f0f","line":12,"range":{"start_line":12,"start_character":0,"end_line":12,"end_character":1},"updated":"2023-08-16 07:31:49.000000000","message":"sigh..","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"702366b84edfb341720cf3a99c6d131b6fa9da46","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This replaces the direct usage of eventlet greenthreads and queue with"},{"line_number":10,"context_line":"the futurist.ThreadPoolExecutor, which uses native threads underneath,"},{"line_number":11,"context_line":"if eventlet patching is disabled."},{"line_number":12,"context_line":"."},{"line_number":13,"context_line":"Futurist has different executors available that use the same interface,"},{"line_number":14,"context_line":"so this can be easily swapped out in the future if we find we need a"},{"line_number":15,"context_line":"different executor. Using a futurist executor is also handy for unit"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":16,"id":"c485cfc9_64634470","line":12,"range":{"start_line":12,"start_character":0,"end_line":12,"end_character":1},"in_reply_to":"393aae62_0bc54f0f","updated":"2023-08-18 02:10:09.000000000","message":"Done","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"de0e9e79665bf0bf3de42fcbae75f967a0e26c29","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"df2c2241_fac87d45","updated":"2023-08-14 13:46:00.000000000","message":"Can we depends-on this so we can see it actually tested?\n\nhttps://review.opendev.org/c/openstack/devstack/+/891214","commit_id":"2bcfbf111a64bf4ec36de4f9b6f11849ce1e05ad"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"65142751a4c703f43358041269ff5cd01a814f13","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"f4be554d_6de34c4d","updated":"2023-08-16 07:31:01.000000000","message":"I _think_ I incorporated all of the feedback (except removing eventlet entirely) but I may have missed something.","commit_id":"2bcfbf111a64bf4ec36de4f9b6f11849ce1e05ad"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0021714f8f3bb078dcf25dcc8c15f2aa5fe36d01","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":15,"id":"9cbbf3a5_ade4b28a","in_reply_to":"909e253b_509ac621","updated":"2023-08-14 14:05:58.000000000","message":"Oh I missed your comment, since I\u0027ve had this open since last week. I don\u0027t think we should abandon the eventlet option just yet ...","commit_id":"2bcfbf111a64bf4ec36de4f9b6f11849ce1e05ad"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"65142751a4c703f43358041269ff5cd01a814f13","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"b795a1fb_4a42cf1a","in_reply_to":"9cbbf3a5_ade4b28a","updated":"2023-08-16 07:31:01.000000000","message":"Done","commit_id":"2bcfbf111a64bf4ec36de4f9b6f11849ce1e05ad"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ef10ef6179f45a43bd3f3559edd9ab8db7549b2f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":15,"id":"909e253b_509ac621","in_reply_to":"df2c2241_fac87d45","updated":"2023-08-14 14:01:54.000000000","message":"ya thats an alternitivle to jsut removing the monkey_patch here https://github.com/openstack/nova/blob/e8d7380759619871b7537d8c3010df31da9f3a4d/nova/api/openstack/__init__.py#L20","commit_id":"2bcfbf111a64bf4ec36de4f9b6f11849ce1e05ad"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"702366b84edfb341720cf3a99c6d131b6fa9da46","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"c7854bb9_f676dd00","updated":"2023-08-18 02:10:09.000000000","message":"OK, I\u0027m not sure I did the boolean flag handling in a good way but I was thinking about eventlet patching \"as early as possible\" and avoiding importing modules that import more modules.\n\nAnother way I considered was adding a new dedicated enable_eventlet_patching.py file that has nothing in it other than the flag. I wasn\u0027t sure which way would be preferred or if there\u0027s another way I haven\u0027t thought of.\n\nI left the Depends-On on to test against multiple native threads with WSGI. AFAIK currently everything is running with the default of threads\u003d1.","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"fa9cc520f6325314880c6dccbfa83e4e4abc7cdc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"d0104bd5_503be0a6","updated":"2023-08-18 07:53:32.000000000","message":"Bleh, this isn\u0027t working right .. looks like it\u0027s still patching under WSGI. Need to debug","commit_id":"4f32355e70d3d7d1e3eff20071b316857d5365be"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c857c298349489f68a2ecae445320e703b49799a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"e928e2cd_f85384c8","updated":"2023-08-18 05:46:16.000000000","message":"Trying without the Depends-On","commit_id":"4f32355e70d3d7d1e3eff20071b316857d5365be"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"456dde7b160d87a5879945984d5328048141e064","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":19,"id":"5ba6f073_234613cc","updated":"2023-08-18 10:07:47.000000000","message":"Copying my top level comment from PS16 as not \"Resolved\" so it\u0027s more discoverable:\n\n\u003e OK, I\u0027m not sure I did the boolean flag handling in a good way but I was thinking about eventlet patching \"as early as possible\" and avoiding importing modules that import more modules.\n\u003e\n\u003e Another way I considered was adding a new dedicated enable_eventlet_patching.py file that has nothing in it other than the flag. I wasn\u0027t sure which way would be preferred or if there\u0027s another way I haven\u0027t thought of.\n\u003e\n\u003e I left the Depends-On on to test against multiple native threads with WSGI. AFAIK currently everything is running with the default of threads\u003d1.","commit_id":"f713b0f52b56e2c26654928d39faec7fa6418b00"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"88758ca9824145deea85783d23989c3dc3ba9307","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"2d058c29_52730607","updated":"2023-08-22 19:12:48.000000000","message":"Unrelated to the review discussion, I have noticed keystone fail with a 500 error when wsgi threads is set \u003e 1 via the Depends-On ... which is affecting our ability to get as complete of test runs on this change.\n\nI looked at the keystone code but haven\u0027t been able to figure out how to fix it yet. Maybe it would just be a matter of adding a lock around the global policy enforcer init [1]:\n\n```\nAug 22 01:06:34.685577 np0035008942 devstack@keystone.service[47694]: ERROR keystone   File \"/opt/stack/keystone/keystone/api/domains.py\", line 113, in post\nAug 22 01:06:34.685577 np0035008942 devstack@keystone.service[47694]: ERROR keystone     ENFORCER.enforce_call(action\u003d\u0027identity:create_domain\u0027)\nAug 22 01:06:34.685577 np0035008942 devstack@keystone.service[47694]: ERROR keystone   File \"/opt/stack/keystone/keystone/common/rbac_enforcer/enforcer.py\", line 455, in enforce_call\nAug 22 01:06:34.685577 np0035008942 devstack@keystone.service[47694]: ERROR keystone     enforcer_obj._enforce(\nAug 22 01:06:34.685577 np0035008942 devstack@keystone.service[47694]: ERROR keystone   File \"/opt/stack/keystone/keystone/common/rbac_enforcer/enforcer.py\", line 127, in _enforce\nAug 22 01:06:34.685577 np0035008942 devstack@keystone.service[47694]: ERROR keystone     self._check_deprecated_rule(action)\nAug 22 01:06:34.689279 np0035008942 devstack@keystone.service[47694]: ERROR keystone   File \"/opt/stack/keystone/keystone/common/rbac_enforcer/enforcer.py\", line 96, in _check_deprecated_rule\nAug 22 01:06:34.689279 np0035008942 devstack@keystone.service[47694]: ERROR keystone     _emit_warning()\nAug 22 01:06:34.689279 np0035008942 devstack@keystone.service[47694]: ERROR keystone   File \"/opt/stack/keystone/keystone/common/rbac_enforcer/enforcer.py\", line 82, in _emit_warning\nAug 22 01:06:34.689279 np0035008942 devstack@keystone.service[47694]: ERROR keystone     if not self._enforcer._warning_emitted:\nAug 22 01:06:34.689279 np0035008942 devstack@keystone.service[47694]: ERROR keystone AttributeError: \u0027Enforcer\u0027 object has no attribute \u0027_warning_emitted\u0027\nAug 22 01:06:34.689279 np0035008942 devstack@keystone.service[47694]: ERROR keystone \nAug 22 01:06:34.689279 np0035008942 devstack@keystone.service[47694]: [pid: 47694|app: 0|req: 2/15] 10.209.128.207 () {68 vars in 1290 bytes} [Tue Aug 22 01:06:34 2023] POST /identity/v3/domains \u003d\u003e generated 0 bytes in 533 msecs (HTTP/1.1 500) 0 headers in 0 bytes (0 switches on core 1)\n```\n\n[1] https://zuul.opendev.org/t/openstack/build/af86792d15284fbd9db06cf5e5954fb4/log/controller/logs/screen-keystone.txt#1318","commit_id":"f713b0f52b56e2c26654928d39faec7fa6418b00"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ee2d1a4e134302f573029d1e9b951a1bc0fee399","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"a365c27c_0e3c2440","updated":"2023-08-22 07:32:35.000000000","message":"recheck ++ functions-common:get_or_create_domain:861 :   oscwrap --os-cloud devstack-system-admin domain create Default --description \u0027\u0027 --or-show -f value -c id\nInternal Server Error (HTTP 500)","commit_id":"f713b0f52b56e2c26654928d39faec7fa6418b00"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"463eeb9a093a4f316affb0f2d398f951c83f9391","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"fb45f9e5_72069db2","updated":"2023-08-22 00:38:23.000000000","message":"recheck urllib3.exceptions.ReadTimeoutError: HTTPSConnectionPool(host\u003d\u0027158.69.65.167\u0027, port\u003d443): Read timed out. (read timeout\u003d60)","commit_id":"f713b0f52b56e2c26654928d39faec7fa6418b00"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4af10c33d0f50c208d6da1bdc4ce05e86b544a25","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":22,"id":"a379dfc6_847dd4e3","updated":"2023-08-30 10:12:29.000000000","message":"recheck ERROR keystone AttributeError: \u0027Enforcer\u0027 object has no attribute \u0027_warning_emitted\u0027\n\n```\nAug 24 03:44:05.783484 np0035037029 devstack@keystone.service[62735]: ERROR keystone AttributeError: \u0027Enforcer\u0027 object has no attribute \u0027_warning_emitted\u0027\nAug 24 03:44:05.783484 np0035037029 devstack@keystone.service[62735]: ERROR keystone \nAug 24 03:44:05.783484 np0035037029 devstack@keystone.service[62735]: [pid: 62735|app: 0|req: 2/18] 158.69.78.16 () {64 vars in 1258 bytes} [Thu Aug 24 03:44:05 2023] GET /identity/v3/domains?name\u003dDefault \u003d\u003e generated 0 bytes in 482 msecs (HTTP/1.1 500) 0 headers in 0 bytes (0 switches on core 1)\n```\n\nCI keeps failing related to the problem with keystone and multiple wsgi threads leading to 500 errors. Not sure how probable it would be to get a passing run on this patch given that issue.","commit_id":"ac6b3df593ea1c488a45204898b135f8cb9e4498"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0f03756e5705b1c9b49a032864053deadf14a31f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"9cd004b5_67faca48","updated":"2023-08-31 06:35:45.000000000","message":"recheck urllib3.exceptions.ReadTimeoutError: HTTPSConnectionPool(host\u003d\u002710.209.0.107\u0027, port\u003d443): Read timed out. (read timeout\u003d60)\n\nIt\u0027s weird bc AFAICT nova-api sent the request along fine all the way to Cinder where the volume snapshot was successfully created, yet the actual snapshot request to nova-api (which should have responded with a 202) never responded ??? and so the test gave up.","commit_id":"9aa65c30610e686d006af14a639d38cdb091300b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"49631b0028dd8d83dad214ce1283e254d45ffde2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"ab7f8105_adf6c934","updated":"2023-09-15 22:55:46.000000000","message":"recheck urllib3.exceptions.ReadTimeoutError: HTTPSConnectionPool(host\u003d\u002710.209.0.243\u0027, port\u003d443): Read timed out. (read timeout\u003d60)","commit_id":"9aa65c30610e686d006af14a639d38cdb091300b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d7fed7db2562534c5e0e926df3edcd85ce1d1336","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"84971ba2_732d5af6","updated":"2023-10-05 20:21:17.000000000","message":"recheck urllib3.exceptions.ReadTimeoutError: HTTPSConnectionPool(host\u003d\u0027173.231.255.110\u0027, port\u003d443): Read timed out. (read timeout\u003d60)","commit_id":"9aa65c30610e686d006af14a639d38cdb091300b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"888e587e1cd1bee7da8d415c261ab6b23376b397","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":25,"id":"293dda6d_d48b18c5","updated":"2024-01-05 07:11:37.000000000","message":"recheck create_image urllib3.exceptions.ReadTimeoutError: HTTPSConnectionPool(host\u003d\u0027217.182.140.169\u0027, port\u003d443): Read timed out. (read timeout\u003d60)","commit_id":"dd0becced1bd51c302d1b5012a24687a21ca9afa"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"51f4c1109023b7c1e4d3179a360bedae18895b18","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"2889d690_cedf0c8d","updated":"2024-01-09 20:15:25.000000000","message":"I have concerns about this patch because I\u0027m seeing pretty consistently there will be at least one CI job failure due to a (presumably uwsgi) thread seemingly getting stuck for \u003e 60s and timing out in responding to the client. It\u0027s always with nova-api. I\u0027ve tried different things like changing all appearances of eventlet to native threads and trying different settings and number of uwsgi threads in the Depends-On devstack patch. I\u0027m still seeing the problem.\n\nOn top of all that, I have seen a number of instances of people having their uwsgi threads get stuck (https://github.com/unbit/uwsgi/issues/1925) when I go searching around. I can\u0027t tell if there\u0027s a problem with the nova code or uwsgi or if it\u0027s just \"too many threads\" resulting in this behavior? I\u0027ve seen a fair number of posts online where the conclusion of someone was \"use the ProcessPoolExecutor instead of the ThreadPoolExecutor because uwsgi threads don\u0027t work properly\" ...\n\nIt would be nice to try this with something other than uwsgi, like gunicorn or something like that to see if the problem appears there as well.","commit_id":"ed1db260ca693dbfeaab4b19c7608d610cfe0e34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"306f6c7509ce772565b53bd1d6581ab9ad9c2423","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"56847143_c26635ad","updated":"2024-01-06 07:24:20.000000000","message":"recheck dep updated","commit_id":"ed1db260ca693dbfeaab4b19c7608d610cfe0e34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c687d547e39f6b6b2eba3b10e88a29573dfc9a88","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"0e599340_97e57adb","updated":"2024-01-09 01:02:29.000000000","message":"recheck testing","commit_id":"ed1db260ca693dbfeaab4b19c7608d610cfe0e34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"23ec7158038af4f18e6bafa9766c7643154231dd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"18407bd4_eae4a3e6","updated":"2024-01-09 10:58:02.000000000","message":"recheck testing","commit_id":"ed1db260ca693dbfeaab4b19c7608d610cfe0e34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ba8daa6c29c7c3c1e6d9c74a4bdcc21718a819f3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"7df195bd_328dcfde","updated":"2024-01-09 03:24:52.000000000","message":"recheck testing","commit_id":"ed1db260ca693dbfeaab4b19c7608d610cfe0e34"}],"nova/conf/service.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6228b3c4585e7da1e78464e1b1321d3580b5be83","unresolved":true,"context_lines":[{"line_number":121,"context_line":"number of processes."},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"When running in WSGI mode, this option represents the number of threads per"},{"line_number":124,"context_line":"instance of the OpenStack API service."},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"Possible Values:"},{"line_number":127,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"287f8d98_2e85f111","line":124,"updated":"2023-08-16 15:55:13.000000000","message":"This isn\u0027t quite right - it\u0027s the number of threads in our threadpool for scattering multi-cell operations. Since we don\u0027t have any other threadpools, that\u0027s the specific thing we use it for, but we might want to make the actual thing we way more generic. The number of threads is the number of request threads (set by uwsgi) plus any others we spin up when scattering.","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ae55b0d9ea4ce9912e5326f35b97171f8be8c678","unresolved":true,"context_lines":[{"line_number":121,"context_line":"number of processes."},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"When running in WSGI mode, this option represents the number of threads per"},{"line_number":124,"context_line":"instance of the OpenStack API service."},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"Possible Values:"},{"line_number":127,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"4a8b9911_e59017f9","line":124,"in_reply_to":"287f8d98_2e85f111","updated":"2023-08-16 17:14:20.000000000","message":"Oh, right.. will update it.","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"702366b84edfb341720cf3a99c6d131b6fa9da46","unresolved":false,"context_lines":[{"line_number":121,"context_line":"number of processes."},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"When running in WSGI mode, this option represents the number of threads per"},{"line_number":124,"context_line":"instance of the OpenStack API service."},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"Possible Values:"},{"line_number":127,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"9c3ef79a_dc0f76a4","line":124,"in_reply_to":"4a8b9911_e59017f9","updated":"2023-08-18 02:10:09.000000000","message":"Done","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"}],"nova/context.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1b7f03fdc9c4173c65f370e6566f04092815847a","unresolved":false,"context_lines":[{"line_number":430,"context_line":"              be returned if the call to a cell raised an exception. The"},{"line_number":431,"context_line":"              exception will be logged."},{"line_number":432,"context_line":"    \"\"\""},{"line_number":433,"context_line":"    executor \u003d futurist.GreenThreadPoolExecutor()"},{"line_number":434,"context_line":"    results \u003d {}"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"    def gather_result(cell_mapping, fn, context, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_54b0cb8b","line":433,"range":{"start_line":433,"start_character":3,"end_line":433,"end_character":49},"updated":"2019-04-05 10:18:30.000000000","message":"this proably will work fine but i dont know if we want\nto recreate and destory this every time.","commit_id":"0b0003a49a957ad724f5be29dba07d521fc66dfb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ce796e65a2f4fc00d54401a21133e7e9abd89718","unresolved":false,"context_lines":[{"line_number":430,"context_line":"              be returned if the call to a cell raised an exception. The"},{"line_number":431,"context_line":"              exception will be logged."},{"line_number":432,"context_line":"    \"\"\""},{"line_number":433,"context_line":"    executor \u003d futurist.GreenThreadPoolExecutor()"},{"line_number":434,"context_line":"    results \u003d {}"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"    def gather_result(cell_mapping, fn, context, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_607dfc1a","line":433,"range":{"start_line":433,"start_character":3,"end_line":433,"end_character":49},"in_reply_to":"5fc1f717_54b0cb8b","updated":"2019-04-05 14:55:52.000000000","message":"\u003e this proably will work fine but i dont know if we want\n \u003e to recreate and destory this every time.\n\nGood point, I\u0027ll push a new PS.","commit_id":"0b0003a49a957ad724f5be29dba07d521fc66dfb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b9809a00a4dd1060c81b33bac31a665a9080f1c8","unresolved":false,"context_lines":[{"line_number":430,"context_line":"              be returned if the call to a cell raised an exception. The"},{"line_number":431,"context_line":"              exception will be logged."},{"line_number":432,"context_line":"    \"\"\""},{"line_number":433,"context_line":"    executor \u003d futurist.GreenThreadPoolExecutor()"},{"line_number":434,"context_line":"    results \u003d {}"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"    def gather_result(cell_mapping, fn, context, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_9444f375","line":433,"range":{"start_line":433,"start_character":3,"end_line":433,"end_character":49},"in_reply_to":"5fc1f717_54b0cb8b","updated":"2019-04-05 10:21:14.000000000","message":"by the way we still need to monkeypatch as this used the greenlet executor explcitly.\n\nif we want to be able to not monkeypatch in the future we will want to dynamically choose the executro to use.\n\nthat can be in a different patch however just wanted to point that out.","commit_id":"0b0003a49a957ad724f5be29dba07d521fc66dfb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a81e31661120212498ca618f75405a40da530405","unresolved":false,"context_lines":[{"line_number":430,"context_line":"              be returned if the call to a cell raised an exception. The"},{"line_number":431,"context_line":"              exception will be logged."},{"line_number":432,"context_line":"    \"\"\""},{"line_number":433,"context_line":"    executor \u003d futurist.GreenThreadPoolExecutor()"},{"line_number":434,"context_line":"    results \u003d {}"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"    def gather_result(cell_mapping, fn, context, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_a0944437","line":433,"range":{"start_line":433,"start_character":3,"end_line":433,"end_character":49},"in_reply_to":"5fc1f717_9444f375","updated":"2019-04-05 14:56:56.000000000","message":"\u003e by the way we still need to monkeypatch as this used the greenlet\n \u003e executor explcitly.\n \u003e \n \u003e if we want to be able to not monkeypatch in the future we will want\n \u003e to dynamically choose the executro to use.\n \u003e \n \u003e that can be in a different patch however just wanted to point that\n \u003e out.\n\nIs there a way to detect whether we\u0027re running under wsgi or not (to do dynamic executor choice)? I looked briefly and didn\u0027t see how.","commit_id":"0b0003a49a957ad724f5be29dba07d521fc66dfb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"00c9e70035f5ad9cabe8be8cc9dbc71a71910eb3","unresolved":false,"context_lines":[{"line_number":430,"context_line":"              be returned if the call to a cell raised an exception. The"},{"line_number":431,"context_line":"              exception will be logged."},{"line_number":432,"context_line":"    \"\"\""},{"line_number":433,"context_line":"    executor \u003d futurist.GreenThreadPoolExecutor()"},{"line_number":434,"context_line":"    results \u003d {}"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"    def gather_result(cell_mapping, fn, context, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_c00e308b","line":433,"range":{"start_line":433,"start_character":3,"end_line":433,"end_character":49},"in_reply_to":"5fc1f717_a0944437","updated":"2019-04-05 15:09:36.000000000","message":"I also wonder if we should set max_workers to something here. osapi_compute_workers? Or something else? Or leave it as-is?","commit_id":"0b0003a49a957ad724f5be29dba07d521fc66dfb"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"115ebbb2aa0abc10deb6d0b9a570f364bb313b6f","unresolved":false,"context_lines":[{"line_number":430,"context_line":"              be returned if the call to a cell raised an exception. The"},{"line_number":431,"context_line":"              exception will be logged."},{"line_number":432,"context_line":"    \"\"\""},{"line_number":433,"context_line":"    executor \u003d futurist.GreenThreadPoolExecutor()"},{"line_number":434,"context_line":"    results \u003d {}"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"    def gather_result(cell_mapping, fn, context, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_a3d8ce51","line":433,"range":{"start_line":433,"start_character":3,"end_line":433,"end_character":49},"in_reply_to":"5fc1f717_a0944437","updated":"2019-04-05 15:35:12.000000000","message":"Not sure... @cdent, any ideas?","commit_id":"0b0003a49a957ad724f5be29dba07d521fc66dfb"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"2740e14b07fb97fee99895bd2c58d3ef396664be","unresolved":false,"context_lines":[{"line_number":430,"context_line":"              be returned if the call to a cell raised an exception. The"},{"line_number":431,"context_line":"              exception will be logged."},{"line_number":432,"context_line":"    \"\"\""},{"line_number":433,"context_line":"    executor \u003d futurist.GreenThreadPoolExecutor()"},{"line_number":434,"context_line":"    results \u003d {}"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"    def gather_result(cell_mapping, fn, context, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_a3fbee78","line":433,"range":{"start_line":433,"start_character":3,"end_line":433,"end_character":49},"in_reply_to":"5fc1f717_a3d8ce51","updated":"2019-04-05 15:45:29.000000000","message":"I reckon it is fine as is. The underlying lib sets a very big max_workers\u003d1000 default which we\u0027re unlikely to ever hit. Assuming we are always using Theads, GreenThreads or asyncio and never sub-processes letting the library decide should be fine.\n\nIn any case api workers (which is always relatively small) would be too low for this kind of thing and trying to guess at an optimal number is unlikely to land us anywhere that works well for every situation. So may as well let it have its way.","commit_id":"0b0003a49a957ad724f5be29dba07d521fc66dfb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1b7f03fdc9c4173c65f370e6566f04092815847a","unresolved":false,"context_lines":[{"line_number":464,"context_line":"            LOG.warning(\u0027Timed out waiting for response from cell %s\u0027,"},{"line_number":465,"context_line":"                        cell_uuid)"},{"line_number":466,"context_line":""},{"line_number":467,"context_line":"    executor.shutdown()"},{"line_number":468,"context_line":"    return results"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_d4611b13","line":467,"range":{"start_line":467,"start_character":4,"end_line":467,"end_character":23},"updated":"2019-04-05 10:18:30.000000000","message":"this might not be called if\n results[cell_uuid] \u003d future.result()\nraise an exception.\n\ni would proably do\n\ntry:\n  futurist.waiters.wait_for_all(results.values(), timeout\u003dtimeout)\n\n    # Fill in results from the futures or timeouts.\n    for cell_uuid, future in results.items():\n        if future.done():\n            results[cell_uuid] \u003d future.result()\n        else:\n            future.cancel()\n            results[cell_uuid] \u003d did_not_respond_sentinel\n            LOG.warning(\u0027Timed out waiting for response from cell %s\u0027,\n                        cell_uuid)\n\nfinally:\n    executor.shutdown()\n\nreturn results","commit_id":"0b0003a49a957ad724f5be29dba07d521fc66dfb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5bfde305f16ae2b284c5685845d7d8f5674b6174","unresolved":false,"context_lines":[{"line_number":464,"context_line":"            LOG.warning(\u0027Timed out waiting for response from cell %s\u0027,"},{"line_number":465,"context_line":"                        cell_uuid)"},{"line_number":466,"context_line":""},{"line_number":467,"context_line":"    executor.shutdown()"},{"line_number":468,"context_line":"    return results"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_8a8128c6","line":467,"range":{"start_line":467,"start_character":4,"end_line":467,"end_character":23},"in_reply_to":"5fc1f717_cfee628a","updated":"2019-04-05 12:29:27.000000000","message":"shush you with your logical arguments :)\n\nyou are right.","commit_id":"0b0003a49a957ad724f5be29dba07d521fc66dfb"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"bffdc33df71d72f6b9b1ca475e2dc6785ae9abf0","unresolved":false,"context_lines":[{"line_number":464,"context_line":"            LOG.warning(\u0027Timed out waiting for response from cell %s\u0027,"},{"line_number":465,"context_line":"                        cell_uuid)"},{"line_number":466,"context_line":""},{"line_number":467,"context_line":"    executor.shutdown()"},{"line_number":468,"context_line":"    return results"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_cfee628a","line":467,"range":{"start_line":467,"start_character":4,"end_line":467,"end_character":23},"in_reply_to":"5fc1f717_d4611b13","updated":"2019-04-05 11:52:53.000000000","message":"\u003e this might not be called if\n \u003e results[cell_uuid] \u003d future.result()\n \u003e raise an exception.\n\nBut the gather_result functor never raises an exception. It always returns a value.","commit_id":"0b0003a49a957ad724f5be29dba07d521fc66dfb"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"71d4ac7afe2d4b7ef4a552819bd682a156f51d87","unresolved":false,"context_lines":[{"line_number":434,"context_line":"    \"\"\""},{"line_number":435,"context_line":"    global EXECUTOR"},{"line_number":436,"context_line":"    if not EXECUTOR:"},{"line_number":437,"context_line":"        EXECUTOR \u003d futurist.GreenThreadPoolExecutor()"},{"line_number":438,"context_line":"    results \u003d {}"},{"line_number":439,"context_line":""},{"line_number":440,"context_line":"    def gather_result(cell_mapping, fn, context, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":5,"id":"5fc1f717_2db91278","line":437,"range":{"start_line":437,"start_character":0,"end_line":437,"end_character":53},"updated":"2019-04-08 11:43:17.000000000","message":"Couple of things here. Firstly his needs a lock, or we risk having multiple executors depending on the threading model in use.\n\nSecondly, this hardcodes a dependency on eventlet, which we\u0027re explicitly trying to avoid here. I think we only need one of these for a nova service. I suggest moving this intialisation to nova.utils and selecting the executor based on config. e.g.:\n\nnova.utils.futurist_executor():\n\n  global EXECUTOR\n  if not EXECUTOR:\n    with lock:\n      if not EXECUTOR:\n        if CONF.thread_model \u003d\u003d \u0027eventlet\u0027:\n          EXECUTOR \u003d greenthread...\n        else:\n          whatever\n  return EXECUTOR\n\nThis allows us to have different executors for different services, and also to use synchronous executor in tests if that\u0027s useful.","commit_id":"0481bdf956ee8b3fe4d4cbd122958fb830315a0b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d78497e88447c3bd7a9dc151892f7e245806717e","unresolved":false,"context_lines":[{"line_number":434,"context_line":"    \"\"\""},{"line_number":435,"context_line":"    global EXECUTOR"},{"line_number":436,"context_line":"    if not EXECUTOR:"},{"line_number":437,"context_line":"        EXECUTOR \u003d futurist.GreenThreadPoolExecutor()"},{"line_number":438,"context_line":"    results \u003d {}"},{"line_number":439,"context_line":""},{"line_number":440,"context_line":"    def gather_result(cell_mapping, fn, context, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":5,"id":"5fc1f717_19856aad","line":437,"range":{"start_line":437,"start_character":0,"end_line":437,"end_character":53},"in_reply_to":"5fc1f717_2db91278","updated":"2019-04-08 22:28:42.000000000","message":"\u003e Couple of things here. Firstly his needs a lock, or we risk having\n \u003e multiple executors depending on the threading model in use.\n\nGood point, will fix.\n\n \u003e Secondly, this hardcodes a dependency on eventlet, which we\u0027re\n \u003e explicitly trying to avoid here. I think we only need one of these\n \u003e for a nova service. I suggest moving this intialisation to\n \u003e nova.utils and selecting the executor based on config. e.g.:\n \u003e \n \u003e nova.utils.futurist_executor():\n \u003e \n \u003e global EXECUTOR\n \u003e if not EXECUTOR:\n \u003e with lock:\n \u003e if not EXECUTOR:\n \u003e if CONF.thread_model \u003d\u003d \u0027eventlet\u0027:\n \u003e EXECUTOR \u003d greenthread...\n \u003e else:\n \u003e whatever\n \u003e return EXECUTOR\n \u003e \n \u003e This allows us to have different executors for different services,\n \u003e and also to use synchronous executor in tests if that\u0027s useful.\n\nThis change is _not_ explicitly trying to avoid a dependency on eventlet. This is a first step toward being able to do that in a separate patch. This is intended to be an improvement over the current DIY eventlet green thread scatter-gather and a step toward being able to swap out executors.","commit_id":"0481bdf956ee8b3fe4d4cbd122958fb830315a0b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"71d4ac7afe2d4b7ef4a552819bd682a156f51d87","unresolved":false,"context_lines":[{"line_number":463,"context_line":"        if future.done():"},{"line_number":464,"context_line":"            results[cell_uuid] \u003d future.result()"},{"line_number":465,"context_line":"        else:"},{"line_number":466,"context_line":"            future.cancel()"},{"line_number":467,"context_line":"            results[cell_uuid] \u003d did_not_respond_sentinel"},{"line_number":468,"context_line":"            LOG.warning(\u0027Timed out waiting for response from cell %s\u0027,"},{"line_number":469,"context_line":"                        cell_uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5fc1f717_3ebcdada","line":466,"updated":"2019-04-08 11:43:17.000000000","message":"This won\u0027t work, except with the GreenThreadExecutor, which we explicitly don\u0027t want to use. IIUC it *can\u0027t* work with ThreadPoolExecutor, which I assume we\u0027d use by default if not using eventlet, because python threads aren\u0027t killable. It also doesn\u0027t work with ProcessPoolExecutor, although I guess it theoretically could. Anyway, I tested it and:\n\nGreenThreadExecutor(): cancellable\nThreadPoolExecutor(): not cancellable\nProcessPoolExecutor(): not cancellable\n\nNote that in my test the \u0027work\u0027 was eventlet.sleep(100), for GreenThread and time.sleep(100) for the other 2. It\u0027s possible that if the green thread was blocking on some other operation it might not be killable, but probably not.\n\nI don\u0027t think we can even pass in a thread-safe tombstone to the workers, because none of the workers I looked at are blocking on a cancellable operation. Looks to be DB operations, so we just have to wait.\n\nI\u0027m not sure what to make of this, tbh. What\u0027s the implication of not cancelling a future whose result you aren\u0027t going to use? It will obviously continue to run to completion, but is it going to leak memory on completion because the result wasn\u0027t fetched (a la unix zombie processes), or will it be garbage collected because the associated future has gone out of scope? I suspect the latter, but might be worth:\n\na) Checking.\nb) A comment noting that this doesn\u0027t actually do anything.","commit_id":"0481bdf956ee8b3fe4d4cbd122958fb830315a0b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d78497e88447c3bd7a9dc151892f7e245806717e","unresolved":false,"context_lines":[{"line_number":463,"context_line":"        if future.done():"},{"line_number":464,"context_line":"            results[cell_uuid] \u003d future.result()"},{"line_number":465,"context_line":"        else:"},{"line_number":466,"context_line":"            future.cancel()"},{"line_number":467,"context_line":"            results[cell_uuid] \u003d did_not_respond_sentinel"},{"line_number":468,"context_line":"            LOG.warning(\u0027Timed out waiting for response from cell %s\u0027,"},{"line_number":469,"context_line":"                        cell_uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5fc1f717_396866f2","line":466,"in_reply_to":"5fc1f717_3ebcdada","updated":"2019-04-08 22:28:42.000000000","message":"See my comment above. This change is using the GreenThreadExecutor as a replacement for the DIY stuff we currently have, as an incremental improvement to make it easier to make it configurable as a next step. Canceling is appropriate for green threads. In a future patch that adds the ability to configure the executor type, we will need to deal with the issue you raise here.","commit_id":"0481bdf956ee8b3fe4d4cbd122958fb830315a0b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"24c8cc2958f5e71d7dc151b0c1c5bcd606eb9236","unresolved":false,"context_lines":[{"line_number":435,"context_line":"              exception will be logged."},{"line_number":436,"context_line":"    \"\"\""},{"line_number":437,"context_line":"    global EXECUTOR"},{"line_number":438,"context_line":"    if not EXECUTOR:"},{"line_number":439,"context_line":"        with lockutils.lock(_EXECUTOR_LOCK):"},{"line_number":440,"context_line":"            # Check again while locked in case of a race."},{"line_number":441,"context_line":"            if not EXECUTOR:"},{"line_number":442,"context_line":"                EXECUTOR \u003d futurist.GreenThreadPoolExecutor()"},{"line_number":443,"context_line":"    results \u003d {}"},{"line_number":444,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3fce034c_c819e288","line":441,"range":{"start_line":438,"start_character":4,"end_line":441,"end_character":28},"updated":"2019-04-12 00:01:18.000000000","message":"it always make me happy when people do double check locking correctly :)","commit_id":"f08979fe5ee4fbecbc463371ab4252a566fc5fe6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"43c3bb181ea5dbfb872571577fc7c4711b0ec09c","unresolved":false,"context_lines":[{"line_number":54,"context_line":"CELL_TIMEOUT \u003d 60"},{"line_number":55,"context_line":"# Futurist executor"},{"line_number":56,"context_line":"EXECUTOR \u003d None"},{"line_number":57,"context_line":"_EXECUTOR_LOCK \u003d \u0027executor\u0027"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"class _ContextAuthPlugin(plugin.BaseAuthPlugin):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_83e5b0aa","line":57,"updated":"2019-04-25 16:59:23.000000000","message":"Am I missing where this is used more than once? If not, I\u0027d prefer it just in the lockutils call.","commit_id":"b9e1d69ba68bcdd568e685cee435cb9b6fc496a5"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3ff9fb2f55f52aad4daed9b55389a718d0967c7c","unresolved":false,"context_lines":[{"line_number":54,"context_line":"CELL_TIMEOUT \u003d 60"},{"line_number":55,"context_line":"# Futurist executor"},{"line_number":56,"context_line":"EXECUTOR \u003d None"},{"line_number":57,"context_line":"_EXECUTOR_LOCK \u003d \u0027executor\u0027"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"class _ContextAuthPlugin(plugin.BaseAuthPlugin):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_a37c74b5","line":57,"in_reply_to":"ffb9cba7_83e5b0aa","updated":"2019-04-25 17:08:26.000000000","message":"No, it\u0027s only used once. Can do.","commit_id":"b9e1d69ba68bcdd568e685cee435cb9b6fc496a5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"43c3bb181ea5dbfb872571577fc7c4711b0ec09c","unresolved":false,"context_lines":[{"line_number":440,"context_line":"        with lockutils.lock(_EXECUTOR_LOCK):"},{"line_number":441,"context_line":"            # Check again while locked in case of a race."},{"line_number":442,"context_line":"            if not EXECUTOR:"},{"line_number":443,"context_line":"                EXECUTOR \u003d futurist.GreenThreadPoolExecutor()"},{"line_number":444,"context_line":"    results \u003d {}"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    def gather_result(cell_uuid, fn, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_c304084d","line":443,"updated":"2019-04-25 16:59:23.000000000","message":"Does this necessarily need to be done late such that we need to do the lock dance? Meaning, we can\u0027t do it during import for some reason?","commit_id":"b9e1d69ba68bcdd568e685cee435cb9b6fc496a5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2a12ee271ecd4cebb024c7989cf87a34e27fb78c","unresolved":false,"context_lines":[{"line_number":440,"context_line":"        with lockutils.lock(_EXECUTOR_LOCK):"},{"line_number":441,"context_line":"            # Check again while locked in case of a race."},{"line_number":442,"context_line":"            if not EXECUTOR:"},{"line_number":443,"context_line":"                EXECUTOR \u003d futurist.GreenThreadPoolExecutor()"},{"line_number":444,"context_line":"    results \u003d {}"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    def gather_result(cell_uuid, fn, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_1585775a","line":443,"in_reply_to":"ffb9cba7_434d7898","updated":"2019-04-25 17:32:13.000000000","message":"That would make this all simpler.","commit_id":"b9e1d69ba68bcdd568e685cee435cb9b6fc496a5"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3ff9fb2f55f52aad4daed9b55389a718d0967c7c","unresolved":false,"context_lines":[{"line_number":440,"context_line":"        with lockutils.lock(_EXECUTOR_LOCK):"},{"line_number":441,"context_line":"            # Check again while locked in case of a race."},{"line_number":442,"context_line":"            if not EXECUTOR:"},{"line_number":443,"context_line":"                EXECUTOR \u003d futurist.GreenThreadPoolExecutor()"},{"line_number":444,"context_line":"    results \u003d {}"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    def gather_result(cell_uuid, fn, *args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_434d7898","line":443,"in_reply_to":"ffb9cba7_c304084d","updated":"2019-04-25 17:08:26.000000000","message":"Good point, I don\u0027t think there\u0027s any reason it has to be here and not at import time. Will change.","commit_id":"b9e1d69ba68bcdd568e685cee435cb9b6fc496a5"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ca1fbb90b6d5932dd9d19bbf4045a59e893a4334","unresolved":false,"context_lines":[{"line_number":468,"context_line":"        if future.done():"},{"line_number":469,"context_line":"            results[cell_uuid] \u003d future.result()"},{"line_number":470,"context_line":"        else:"},{"line_number":471,"context_line":"            future.cancel()"},{"line_number":472,"context_line":"            results[cell_uuid] \u003d did_not_respond_sentinel"},{"line_number":473,"context_line":"            LOG.warning(\u0027Timed out waiting for response from cell %s\u0027,"},{"line_number":474,"context_line":"                        cell_uuid)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_1577b7a2","line":471,"range":{"start_line":471,"start_character":12,"end_line":471,"end_character":27},"updated":"2019-04-25 17:23:03.000000000","message":"This is where I\u0027m unclear how to handle this with native threading -- mdbooth mentioned this in an earlier PS that native threads are not cancelable, so what happens in the timeout situation? Unfinished threads just stay around?","commit_id":"b9e1d69ba68bcdd568e685cee435cb9b6fc496a5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2a12ee271ecd4cebb024c7989cf87a34e27fb78c","unresolved":false,"context_lines":[{"line_number":468,"context_line":"        if future.done():"},{"line_number":469,"context_line":"            results[cell_uuid] \u003d future.result()"},{"line_number":470,"context_line":"        else:"},{"line_number":471,"context_line":"            future.cancel()"},{"line_number":472,"context_line":"            results[cell_uuid] \u003d did_not_respond_sentinel"},{"line_number":473,"context_line":"            LOG.warning(\u0027Timed out waiting for response from cell %s\u0027,"},{"line_number":474,"context_line":"                        cell_uuid)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_35ed1b25","line":471,"range":{"start_line":471,"start_character":12,"end_line":471,"end_character":27},"in_reply_to":"ffb9cba7_1577b7a2","updated":"2019-04-25 17:32:13.000000000","message":"You can just put the timeout in the thread itself. You can also set the threads to \"daemon\" (a terrible name for it) so that they don\u0027t block exit of the process if they stick around and then you can leave a stuck one to exit on its own.\n\nFuturist has to be doing something to support cancel-ability as it\u0027s just using the same primitives. I\u0027m guessing that this cancel is actually a condition variable that the thread pokes regularly to check to see if it needs to stop. It\u0027s just a wrapper, nothing magic.\n\nBut anyway, not saying we shouldn\u0027t use it, just commenting :)","commit_id":"b9e1d69ba68bcdd568e685cee435cb9b6fc496a5"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"162f7476510dcd2c622dbe00252ff80416194eac","unresolved":false,"context_lines":[{"line_number":468,"context_line":"        if future.done():"},{"line_number":469,"context_line":"            results[cell_uuid] \u003d future.result()"},{"line_number":470,"context_line":"        else:"},{"line_number":471,"context_line":"            future.cancel()"},{"line_number":472,"context_line":"            results[cell_uuid] \u003d did_not_respond_sentinel"},{"line_number":473,"context_line":"            LOG.warning(\u0027Timed out waiting for response from cell %s\u0027,"},{"line_number":474,"context_line":"                        cell_uuid)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9cba7_954ba729","line":471,"range":{"start_line":471,"start_character":12,"end_line":471,"end_character":27},"in_reply_to":"ffb9cba7_35ed1b25","updated":"2019-04-25 18:15:34.000000000","message":"Thanks. Looks like futurist uses daemon\u003dTrue for Thread futures, so that\u0027s a good thing:\n\nhttps://github.com/openstack/futurist/blob/873d5341ee58176a4dcd59545059d753a8649f02/futurist/_thread.py#L66\n\nThe following has already been said on IRC but including here for completeness:\n\nI don\u0027t find anything in the futurist source to support cancel-ability. The cancel() method returns True/False if cancel() was successful:\n\nhttps://docs.openstack.org/futurist/latest/reference/index.html#futurist.Future.cancel\n\nand is just passing through to the concurrency futures:\n\nhttps://docs.python.org/3/library/concurrent.futures.html#concurrent.futures.Future.cancel\n\nwhich is doing this:\n\nhttps://github.com/python/cpython/blob/d7befad328ad1a6d1f812be2bf154c1cd1e01fbc/Lib/concurrent/futures/_base.py#L352\n\nand also reiterates that running futures cannot be canceled.","commit_id":"b9e1d69ba68bcdd568e685cee435cb9b6fc496a5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9d19c49e511da027709727b28b01f2f0c124b1ee","unresolved":false,"context_lines":[{"line_number":57,"context_line":"    return futurist.ThreadPoolExecutor()"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"EXECUTOR \u003d create_executor()"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"class _ContextAuthPlugin(plugin.BaseAuthPlugin):"}],"source_content_type":"text/x-python","patch_set":14,"id":"dfbec78f_a207a61c","line":60,"updated":"2019-05-08 11:47:15.000000000","message":"I feel the chance are small but, could we avoid yet another global?","commit_id":"011eae40f64d227c31a5be297de5dabbd42eebef"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"79108a9591989667eedf45b3a5dd86ec4e1e1ad3","unresolved":false,"context_lines":[{"line_number":57,"context_line":"    return futurist.ThreadPoolExecutor()"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"EXECUTOR \u003d create_executor()"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"class _ContextAuthPlugin(plugin.BaseAuthPlugin):"}],"source_content_type":"text/x-python","patch_set":14,"id":"dfbec78f_79c161e4","line":60,"in_reply_to":"dfbec78f_a207a61c","updated":"2019-05-08 17:24:38.000000000","message":"We could. Earlier PS had it as a local but in that case we have to tear it down and recreate it for every scatter gather call, which seemed wasteful.","commit_id":"011eae40f64d227c31a5be297de5dabbd42eebef"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9d19c49e511da027709727b28b01f2f0c124b1ee","unresolved":false,"context_lines":[{"line_number":451,"context_line":"        return result"},{"line_number":452,"context_line":""},{"line_number":453,"context_line":"    for cell_mapping in cell_mappings:"},{"line_number":454,"context_line":"        results[cell_mapping.uuid] \u003d None"},{"line_number":455,"context_line":"        with target_cell(context, cell_mapping) as cctxt:"},{"line_number":456,"context_line":"            future \u003d EXECUTOR.submit(gather_result, cell_mapping, fn, cctxt,"},{"line_number":457,"context_line":"                                     *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":14,"id":"dfbec78f_e25fde3e","line":454,"updated":"2019-05-08 11:47:15.000000000","message":"Why do we need this? As far as I see we never check for None below.","commit_id":"011eae40f64d227c31a5be297de5dabbd42eebef"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"79108a9591989667eedf45b3a5dd86ec4e1e1ad3","unresolved":false,"context_lines":[{"line_number":451,"context_line":"        return result"},{"line_number":452,"context_line":""},{"line_number":453,"context_line":"    for cell_mapping in cell_mappings:"},{"line_number":454,"context_line":"        results[cell_mapping.uuid] \u003d None"},{"line_number":455,"context_line":"        with target_cell(context, cell_mapping) as cctxt:"},{"line_number":456,"context_line":"            future \u003d EXECUTOR.submit(gather_result, cell_mapping, fn, cctxt,"},{"line_number":457,"context_line":"                                     *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":14,"id":"dfbec78f_196dc503","line":454,"in_reply_to":"dfbec78f_e25fde3e","updated":"2019-05-08 17:24:38.000000000","message":"Hm, based on what\u0027s presently here, there\u0027s no need for this. Must be leftover residue from an earlier PS.","commit_id":"011eae40f64d227c31a5be297de5dabbd42eebef"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9d19c49e511da027709727b28b01f2f0c124b1ee","unresolved":false,"context_lines":[{"line_number":462,"context_line":""},{"line_number":463,"context_line":"    # Fill in results from the futures or timeouts."},{"line_number":464,"context_line":"    for cell_uuid, future in results.items():"},{"line_number":465,"context_line":"        if future.done():"},{"line_number":466,"context_line":"            results[cell_uuid] \u003d future.result()"},{"line_number":467,"context_line":"        else:"},{"line_number":468,"context_line":"            # NOTE(melwitt): Running futures cannot be canceled, so this will"}],"source_content_type":"text/x-python","patch_set":14,"id":"dfbec78f_0220f2af","line":465,"range":{"start_line":465,"start_character":11,"end_line":465,"end_character":17},"updated":"2019-05-08 11:47:15.000000000","message":"Based on L454 this can be None. So either L454 is not needed or we need an extra `is None` check before future.done() is called","commit_id":"011eae40f64d227c31a5be297de5dabbd42eebef"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"79108a9591989667eedf45b3a5dd86ec4e1e1ad3","unresolved":false,"context_lines":[{"line_number":462,"context_line":""},{"line_number":463,"context_line":"    # Fill in results from the futures or timeouts."},{"line_number":464,"context_line":"    for cell_uuid, future in results.items():"},{"line_number":465,"context_line":"        if future.done():"},{"line_number":466,"context_line":"            results[cell_uuid] \u003d future.result()"},{"line_number":467,"context_line":"        else:"},{"line_number":468,"context_line":"            # NOTE(melwitt): Running futures cannot be canceled, so this will"}],"source_content_type":"text/x-python","patch_set":14,"id":"dfbec78f_3970c9dc","line":465,"range":{"start_line":465,"start_character":11,"end_line":465,"end_character":17},"in_reply_to":"dfbec78f_0220f2af","updated":"2019-05-08 17:24:38.000000000","message":"Agreed that L454 is not needed.","commit_id":"011eae40f64d227c31a5be297de5dabbd42eebef"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9d19c49e511da027709727b28b01f2f0c124b1ee","unresolved":false,"context_lines":[{"line_number":465,"context_line":"        if future.done():"},{"line_number":466,"context_line":"            results[cell_uuid] \u003d future.result()"},{"line_number":467,"context_line":"        else:"},{"line_number":468,"context_line":"            # NOTE(melwitt): Running futures cannot be canceled, so this will"},{"line_number":469,"context_line":"            # return False in most cases, but we will try it anyway."},{"line_number":470,"context_line":"            # The futurist.ThreadPoolExecutor runs threads with daemon\u003dTrue, so"},{"line_number":471,"context_line":"            # they will not block a shutdown and can be left to exit on their"},{"line_number":472,"context_line":"            # own while we are running."}],"source_content_type":"text/x-python","patch_set":14,"id":"dfbec78f_2509802b","line":469,"range":{"start_line":468,"start_character":29,"end_line":469,"end_character":41},"updated":"2019-05-08 11:47:15.000000000","message":"So not even timeout cancels the future? Will this cause that threads will hang around forever leaking OS resources?\n\nThis feels dangerous.\n\n// later \n\nI\u0027ve just checked that futurist does not cancel the threads \n\nimport futurist\nimport time\n\nEXECUTOR \u003d futurist.ThreadPoolExecutor()\n\nfuture \u003d EXECUTOR.submit(time.sleep, 100)\n\ntime.sleep(10)\nprint(\u0027cancel\u0027)\nfuture.cancel()\n\ntime.sleep(1000)\n\n\n//\nBoth thread of this code runs until I kill the process itself.\n\nSo we might need to implement our own cancellation / timeout inside scatter_gather","commit_id":"011eae40f64d227c31a5be297de5dabbd42eebef"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"79108a9591989667eedf45b3a5dd86ec4e1e1ad3","unresolved":false,"context_lines":[{"line_number":465,"context_line":"        if future.done():"},{"line_number":466,"context_line":"            results[cell_uuid] \u003d future.result()"},{"line_number":467,"context_line":"        else:"},{"line_number":468,"context_line":"            # NOTE(melwitt): Running futures cannot be canceled, so this will"},{"line_number":469,"context_line":"            # return False in most cases, but we will try it anyway."},{"line_number":470,"context_line":"            # The futurist.ThreadPoolExecutor runs threads with daemon\u003dTrue, so"},{"line_number":471,"context_line":"            # they will not block a shutdown and can be left to exit on their"},{"line_number":472,"context_line":"            # own while we are running."}],"source_content_type":"text/x-python","patch_set":14,"id":"dfbec78f_c802f4b4","line":469,"range":{"start_line":468,"start_character":29,"end_line":469,"end_character":41},"in_reply_to":"dfbec78f_2509802b","updated":"2019-05-08 17:24:38.000000000","message":"\u003e So not even timeout cancels the future? Will this cause that\n \u003e threads will hang around forever leaking OS resources?\n \u003e \n \u003e This feels dangerous.\n \u003e \n \u003e // later\n \u003e \n \u003e I\u0027ve just checked that futurist does not cancel the threads\n \u003e \n \u003e import futurist\n \u003e import time\n \u003e \n \u003e EXECUTOR \u003d futurist.ThreadPoolExecutor()\n \u003e \n \u003e future \u003d EXECUTOR.submit(time.sleep, 100)\n \u003e \n \u003e time.sleep(10)\n \u003e print(\u0027cancel\u0027)\n \u003e future.cancel()\n \u003e \n \u003e time.sleep(1000)\n \u003e \n \u003e \n \u003e //\n \u003e Both thread of this code runs until I kill the process itself.\n \u003e \n \u003e So we might need to implement our own cancellation / timeout inside\n \u003e scatter_gather\n\nI think that is because you didn\u0027t use a wait call before cancel():\n\nfuturist.waiters.wait_for_all([future], timeout\u003d1)\n\nwhen I do this locally, the thread with time.sleep(100) finishes after 100 seconds.\n\nimport futurist\nimport futurist.waiters\nimport time\n\nexecutor \u003d futurist.ThreadPoolExecutor()\nfuture \u003d executor.submit(time.sleep, 100)\nprint(\u0027sleeping 10\u0027)\ntime.sleep(10)\nprint(\u0027started waiting\u0027)\nfuturist.waiters.wait_for_all([future], timeout\u003d1)\nprint(\u0027finished waiting. canceling\u0027)\nfuture.cancel()\nwhile True:\n    print(\u0027checking for done\u0027)\n    if future.done():\n        print(\u0027thread finished\u0027)\n        break\n    time.sleep(10)","commit_id":"011eae40f64d227c31a5be297de5dabbd42eebef"},{"author":{"_account_id":6926,"name":"Bogdan Dobrelya","email":"bdobreli@redhat.com","username":"bogdando"},"change_message_id":"f95c78c007a0434a2b4668a68aebd352e6a8afb4","unresolved":false,"context_lines":[{"line_number":465,"context_line":"        if future.done():"},{"line_number":466,"context_line":"            results[cell_uuid] \u003d future.result()"},{"line_number":467,"context_line":"        else:"},{"line_number":468,"context_line":"            # NOTE(melwitt): Running futures cannot be canceled, so this will"},{"line_number":469,"context_line":"            # return False in most cases, but we will try it anyway."},{"line_number":470,"context_line":"            # The futurist.ThreadPoolExecutor runs threads with daemon\u003dTrue, so"},{"line_number":471,"context_line":"            # they will not block a shutdown and can be left to exit on their"},{"line_number":472,"context_line":"            # own while we are running."}],"source_content_type":"text/x-python","patch_set":14,"id":"dfbec78f_7998811a","line":469,"range":{"start_line":468,"start_character":29,"end_line":469,"end_character":41},"in_reply_to":"dfbec78f_2509802b","updated":"2019-05-08 15:43:03.000000000","message":"as my local testing shows, a futurist\u0027s thread remains sleeping/running until the main process exists. Such a thread cannot be terminated or exited in Python before that AFAIK. According to the futurist\u0027s docs, running and completed threads cannot be canceled neither. May be periodics fit the case better?","commit_id":"011eae40f64d227c31a5be297de5dabbd42eebef"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6b8edd49ab9bfd3d06f6697dc5ea7c65bf788c01","unresolved":false,"context_lines":[{"line_number":465,"context_line":"        if future.done():"},{"line_number":466,"context_line":"            results[cell_uuid] \u003d future.result()"},{"line_number":467,"context_line":"        else:"},{"line_number":468,"context_line":"            # NOTE(melwitt): Running futures cannot be canceled, so this will"},{"line_number":469,"context_line":"            # return False in most cases, but we will try it anyway."},{"line_number":470,"context_line":"            # The futurist.ThreadPoolExecutor runs threads with daemon\u003dTrue, so"},{"line_number":471,"context_line":"            # they will not block a shutdown and can be left to exit on their"},{"line_number":472,"context_line":"            # own while we are running."}],"source_content_type":"text/x-python","patch_set":14,"id":"dfbec78f_8b84f65f","line":469,"range":{"start_line":468,"start_character":29,"end_line":469,"end_character":41},"in_reply_to":"dfbec78f_68874811","updated":"2019-05-08 17:59:39.000000000","message":"\u003e Taking a step back, if the call inside the thread were to block\n \u003e forever, we would be in trouble anyway if we weren\u0027t using threads,\n \u003e right?\n\nNo, not really. In an eventlet world, your IO is done non-blocking so that eventlet can yield to other greenthreads to do work while you wait. If we determine that something is taking too long (i.e. the timeout case) then you just never schedule the waiting thread again. You might end up with a connection to the DB that takes a long time to time out or something, but it\u0027s also likely that eventlet can just close that socket (which you can do at any time).\n\nIf we\u0027re spawning real threads, we (the process/thread that did the spawning) doesn\u0027t really have any control over the other thread in that way. Above is why eventlet threads are easily cancelable and native threads are not.\n\nIn order to make them cancelable in a similar fashion, as gibi points out, we would have to basically re-implement eventlet (the infrastructure that it provides) ourself, which we\u0027re definitely going to get wrong and I don\u0027t think we should do.\n \n \u003e With this code, we wait for the threads for N seconds, if they take\n \u003e longer than that, they will quit in the background after they\n \u003e method they\u0027re running (for example: InstanceList.get_by_filters)\n \u003e returns and we have already moved on and will not care about their\n \u003e result.\n\nNot if they\u0027re blocked doing uninterruptible system IO. If that eventually unblocks or times out, then yes they will eventually free up. Gibi\u0027s point, which is very valid (and as I mentioned in Denver) is that if we spawn a bunch of things very quickly (as we would if the API is being hit hard when something on the system or network starts to hang), we will immediately run out of workers in the threadpool and hang until things start to time out (if they\u0027re going to).\n\nLooks like concurrent.futures defaults to 5x$CPUs thread workers. Say you have 8 CPUs and 5 cells for the sake of argument -- that means you can do 8 scatter/gather lists before you\u0027re hung and can\u0027t do anything else until something times out, which could be a minute or two if the network goes away. Longer if the DB is thrashing.\n\nHonestly I\u0027m not sure what to suggest here. I really like what eventlet gets us, and we have a bunch of code all over nova that just kinda behaves this way. Moving to native threads gets us...something, but at the expense of this just working.","commit_id":"011eae40f64d227c31a5be297de5dabbd42eebef"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"79108a9591989667eedf45b3a5dd86ec4e1e1ad3","unresolved":false,"context_lines":[{"line_number":465,"context_line":"        if future.done():"},{"line_number":466,"context_line":"            results[cell_uuid] \u003d future.result()"},{"line_number":467,"context_line":"        else:"},{"line_number":468,"context_line":"            # NOTE(melwitt): Running futures cannot be canceled, so this will"},{"line_number":469,"context_line":"            # return False in most cases, but we will try it anyway."},{"line_number":470,"context_line":"            # The futurist.ThreadPoolExecutor runs threads with daemon\u003dTrue, so"},{"line_number":471,"context_line":"            # they will not block a shutdown and can be left to exit on their"},{"line_number":472,"context_line":"            # own while we are running."}],"source_content_type":"text/x-python","patch_set":14,"id":"dfbec78f_68874811","line":469,"range":{"start_line":468,"start_character":29,"end_line":469,"end_character":41},"in_reply_to":"dfbec78f_7998811a","updated":"2019-05-08 17:24:38.000000000","message":"Taking a step back, if the call inside the thread were to block forever, we would be in trouble anyway if we weren\u0027t using threads, right?\n\nWith this code, we wait for the threads for N seconds, if they take longer than that, they will quit in the background after they method they\u0027re running (for example: InstanceList.get_by_filters) returns and we have already moved on and will not care about their result.\n\nNormally when you want to halt a thread in the middle of it, you can use a threading.Event variable, but if you check it after you make a long blocking call, you won\u0027t be able to return and quit the thread until the long call returns. Example:\n\ndef run_in_thread(stopevent):\n  // query database\n  if stopevent.is_set():\n    return\n\nwith timeutils.StopWatch as timer:\n  while True:\n      if timer.elapsed() \u003e CELL_TIMEOUT:\n          stopevent.set()\n          break\n      sleep(0.01)\n\nAFAICT this still has the same problem of being at the mercy of the blocking database query and if that takes very long to return, the thread will stay around until then. Am I missing something?","commit_id":"011eae40f64d227c31a5be297de5dabbd42eebef"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4371486064999d25b1931329268a4e540307987a","unresolved":false,"context_lines":[{"line_number":465,"context_line":"        if future.done():"},{"line_number":466,"context_line":"            results[cell_uuid] \u003d future.result()"},{"line_number":467,"context_line":"        else:"},{"line_number":468,"context_line":"            # NOTE(melwitt): Running futures cannot be canceled, so this will"},{"line_number":469,"context_line":"            # return False in most cases, but we will try it anyway."},{"line_number":470,"context_line":"            # The futurist.ThreadPoolExecutor runs threads with daemon\u003dTrue, so"},{"line_number":471,"context_line":"            # they will not block a shutdown and can be left to exit on their"},{"line_number":472,"context_line":"            # own while we are running."}],"source_content_type":"text/x-python","patch_set":14,"id":"dfbec78f_8ba156ac","line":469,"range":{"start_line":468,"start_character":29,"end_line":469,"end_character":41},"in_reply_to":"dfbec78f_8b84f65f","updated":"2019-05-08 18:21:12.000000000","message":"Thanks for explaining it in detail. I think I understand all the aspects about it now.\n\nI agree that eventlet is providing nice behavior here. I was trying to help address the issues that are happening because of eventlet monkey patching in nova-api (AMQP heartbeats not working):\n\nhttp://lists.openstack.org/pipermail/openstack-discuss/2019-May/005864.html\n\nIs there anything else we could do in this case ^? Should deployers never run with mod_wsgi and use uwsgi instead? And use the mod_proxy_uwsgi to run nova-api as an all-in-one?\n\nhttps://docs.openstack.org/nova/latest/user/wsgi.html\n\nApologies, I don\u0027t know much/anything about wsgi.","commit_id":"011eae40f64d227c31a5be297de5dabbd42eebef"},{"author":{"_account_id":23561,"name":"iain MacDonnell","email":"iain.macdonnell@oracle.com","username":"imacdonn"},"change_message_id":"88c5b6691bb62fa6660e957d85f012603d174084","unresolved":false,"context_lines":[{"line_number":465,"context_line":"        if future.done():"},{"line_number":466,"context_line":"            results[cell_uuid] \u003d future.result()"},{"line_number":467,"context_line":"        else:"},{"line_number":468,"context_line":"            # NOTE(melwitt): Running futures cannot be canceled, so this will"},{"line_number":469,"context_line":"            # return False in most cases, but we will try it anyway."},{"line_number":470,"context_line":"            # The futurist.ThreadPoolExecutor runs threads with daemon\u003dTrue, so"},{"line_number":471,"context_line":"            # they will not block a shutdown and can be left to exit on their"},{"line_number":472,"context_line":"            # own while we are running."}],"source_content_type":"text/x-python","patch_set":14,"id":"dfbec78f_fe7692d8","line":469,"range":{"start_line":468,"start_character":29,"end_line":469,"end_character":41},"in_reply_to":"dfbec78f_8ba156ac","updated":"2019-05-08 19:48:39.000000000","message":"FWIW, the problem discussed in that ML thread (and https://bugs.launchpad.net/nova/+bug/1825584) seems to affect mod_wsgi and uWSGI - I originally encountered it with uWSGI + nginx.","commit_id":"011eae40f64d227c31a5be297de5dabbd42eebef"},{"author":{"_account_id":6926,"name":"Bogdan Dobrelya","email":"bdobreli@redhat.com","username":"bogdando"},"change_message_id":"668cad9092dbdfc7d20b471d392969290d656432","unresolved":false,"context_lines":[{"line_number":465,"context_line":"        if future.done():"},{"line_number":466,"context_line":"            results[cell_uuid] \u003d future.result()"},{"line_number":467,"context_line":"        else:"},{"line_number":468,"context_line":"            # NOTE(melwitt): Running futures cannot be canceled, so this will"},{"line_number":469,"context_line":"            # return False in most cases, but we will try it anyway."},{"line_number":470,"context_line":"            # The futurist.ThreadPoolExecutor runs threads with daemon\u003dTrue, so"},{"line_number":471,"context_line":"            # they will not block a shutdown and can be left to exit on their"},{"line_number":472,"context_line":"            # own while we are running."}],"source_content_type":"text/x-python","patch_set":14,"id":"dfbec78f_f9e0defc","line":469,"range":{"start_line":468,"start_character":29,"end_line":469,"end_character":41},"in_reply_to":"dfbec78f_c802f4b4","updated":"2019-05-09 14:46:40.000000000","message":"\u003e \u003e So not even timeout cancels the future? Will this cause that\n \u003e \u003e threads will hang around forever leaking OS resources?\n \u003e \u003e\n \u003e \u003e This feels dangerous.\n \u003e \u003e\n \u003e \u003e // later\n \u003e \u003e\n \u003e \u003e I\u0027ve just checked that futurist does not cancel the threads\n \u003e \u003e\n \u003e \u003e import futurist\n \u003e \u003e import time\n \u003e \u003e\n \u003e \u003e EXECUTOR \u003d futurist.ThreadPoolExecutor()\n \u003e \u003e\n \u003e \u003e future \u003d EXECUTOR.submit(time.sleep, 100)\n \u003e \u003e\n \u003e \u003e time.sleep(10)\n \u003e \u003e print(\u0027cancel\u0027)\n \u003e \u003e future.cancel()\n \u003e \u003e\n \u003e \u003e time.sleep(1000)\n \u003e \u003e\n \u003e \u003e\n \u003e \u003e //\n \u003e \u003e Both thread of this code runs until I kill the process itself.\n \u003e \u003e\n \u003e \u003e So we might need to implement our own cancellation / timeout\n \u003e inside\n \u003e \u003e scatter_gather\n \u003e \n \u003e I think that is because you didn\u0027t use a wait call before cancel():\n \u003e \n \u003e futurist.waiters.wait_for_all([future], timeout\u003d1)\n \u003e \n \u003e when I do this locally, the thread with time.sleep(100) finishes\n \u003e after 100 seconds.\n \u003e \n \u003e import futurist\n \u003e import futurist.waiters\n \u003e import time\n \u003e \n \u003e executor \u003d futurist.ThreadPoolExecutor()\n \u003e future \u003d executor.submit(time.sleep, 100)\n \u003e print(\u0027sleeping 10\u0027)\n \u003e time.sleep(10)\n \u003e print(\u0027started waiting\u0027)\n \u003e futurist.waiters.wait_for_all([future], timeout\u003d1)\n \u003e print(\u0027finished waiting. canceling\u0027)\n \u003e future.cancel()\n \u003e while True:\n \u003e print(\u0027checking for done\u0027)\n \u003e if future.done():\n \u003e print(\u0027thread finished\u0027)\n \u003e break\n \u003e time.sleep(10)\n\nWith my version [0] of local test (sorry it looks ugly), I can see the worker can never be canceled and always retains its thread in the system, either done or executing, until the main process exists, see [1]\n\n[0] https://gist.github.com/bogdando/830d8e7e5cabcee375e0e9e885b9705c\n[1] http://paste.openstack.org/show/qHT6K9TQlu7YCcYI7pGo/","commit_id":"011eae40f64d227c31a5be297de5dabbd42eebef"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d9012c194b53718eab0b4baaa4d2ec76bcc96f98","unresolved":false,"context_lines":[{"line_number":465,"context_line":"        if future.done():"},{"line_number":466,"context_line":"            results[cell_uuid] \u003d future.result()"},{"line_number":467,"context_line":"        else:"},{"line_number":468,"context_line":"            # NOTE(melwitt): Running futures cannot be canceled, so this will"},{"line_number":469,"context_line":"            # return False in most cases, but we will try it anyway."},{"line_number":470,"context_line":"            # The futurist.ThreadPoolExecutor runs threads with daemon\u003dTrue, so"},{"line_number":471,"context_line":"            # they will not block a shutdown and can be left to exit on their"},{"line_number":472,"context_line":"            # own while we are running."}],"source_content_type":"text/x-python","patch_set":14,"id":"dfbec78f_7e77ebd5","line":469,"range":{"start_line":468,"start_character":29,"end_line":469,"end_character":41},"in_reply_to":"dfbec78f_f9e0defc","updated":"2019-05-09 22:08:11.000000000","message":"\u003e With my version [0] of local test (sorry it looks ugly), I can see\n \u003e the worker can never be canceled and always retains its thread in\n \u003e the system, either done or executing, until the main process\n \u003e exists, see [1]\n \u003e \n \u003e [0] https://gist.github.com/bogdando/830d8e7e5cabcee375e0e9e885b9705c\n \u003e [1] http://paste.openstack.org/show/qHT6K9TQlu7YCcYI7pGo/\n\nOK, thanks for that. It looks like moving to real threads isn\u0027t going to be a good answer here, for many reasons discussed in this review.\n\nI\u0027m just not sure what we can do to address the problem now. We have a real dependency on eventlet in nova-api, so I don\u0027t think we can just remove eventlet monkey patching.\n\nIain, can you elaborate a bit on what you meant about running nova-api \"standalone\" and how it avoids the issue? Because so far, that sounds like the only option.","commit_id":"011eae40f64d227c31a5be297de5dabbd42eebef"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"f2b79e5b5ea858b42a802824ca0371d2d1d3a9c6","unresolved":false,"context_lines":[{"line_number":465,"context_line":"        if future.done():"},{"line_number":466,"context_line":"            results[cell_uuid] \u003d future.result()"},{"line_number":467,"context_line":"        else:"},{"line_number":468,"context_line":"            # NOTE(melwitt): Running futures cannot be canceled, so this will"},{"line_number":469,"context_line":"            # return False in most cases, but we will try it anyway."},{"line_number":470,"context_line":"            # The futurist.ThreadPoolExecutor runs threads with daemon\u003dTrue, so"},{"line_number":471,"context_line":"            # they will not block a shutdown and can be left to exit on their"},{"line_number":472,"context_line":"            # own while we are running."}],"source_content_type":"text/x-python","patch_set":14,"id":"dfbec78f_21208bb0","line":469,"range":{"start_line":468,"start_character":29,"end_line":469,"end_character":41},"in_reply_to":"dfbec78f_fe7692d8","updated":"2019-05-08 21:07:50.000000000","message":"Thanks for the clarification, Iain. I lost track of whether it was mod_wsgi, uWSGI, or both.\n\nI\u0027m not sure what\u0027s the way forward here at this point.","commit_id":"011eae40f64d227c31a5be297de5dabbd42eebef"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"512d9c45ea535a504fe032bbf7d59aade959f25c","unresolved":true,"context_lines":[{"line_number":52,"context_line":"def executor_singleton():"},{"line_number":53,"context_line":"    global EXECUTOR"},{"line_number":54,"context_line":"    if EXECUTOR is None:"},{"line_number":55,"context_line":"        EXECUTOR \u003d futurist.ThreadPoolExecutor()"},{"line_number":56,"context_line":"    return EXECUTOR"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"bf4a77fe_31889b05","line":55,"updated":"2023-08-14 14:01:43.000000000","message":"Aside from thread being monkeypatched, this will always (try to) be native threads, right? Doing this will mean any scatter/gather from other services doesn\u0027t use greenlet and nova-api will forever be native-only. I think we need to either detect if we\u0027re running in eventlet mode (which means we can\u0027t run this from anything that runs import-time) or have a flag. Couldn\u0027t we set a flag from `nova/monkey_patch.py` that tells us which to use here?","commit_id":"2bcfbf111a64bf4ec36de4f9b6f11849ce1e05ad"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a6ea6146fbfe071df037301e8b158fb37bef104a","unresolved":true,"context_lines":[{"line_number":52,"context_line":"def executor_singleton():"},{"line_number":53,"context_line":"    global EXECUTOR"},{"line_number":54,"context_line":"    if EXECUTOR is None:"},{"line_number":55,"context_line":"        EXECUTOR \u003d futurist.ThreadPoolExecutor()"},{"line_number":56,"context_line":"    return EXECUTOR"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"eae0e717_9dcd0e8c","line":55,"in_reply_to":"bf4a77fe_31889b05","updated":"2023-08-14 14:11:15.000000000","message":"Also, we might want to set the pool size from our existing knob (`osapi_compute_workers`). In standalone mode, that\u0027s the number of processes we fork from the parent, but in wsgi mode that\u0027s controlled by the wsgi container. So in this case it\u0027d be the number of thread we create for each instance of us that the wsgi container spawns. That\u0027s a similar sort of calculus and thus I think it\u0027d make sense to re-use it (with a conf help update of course).","commit_id":"2bcfbf111a64bf4ec36de4f9b6f11849ce1e05ad"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"65142751a4c703f43358041269ff5cd01a814f13","unresolved":false,"context_lines":[{"line_number":52,"context_line":"def executor_singleton():"},{"line_number":53,"context_line":"    global EXECUTOR"},{"line_number":54,"context_line":"    if EXECUTOR is None:"},{"line_number":55,"context_line":"        EXECUTOR \u003d futurist.ThreadPoolExecutor()"},{"line_number":56,"context_line":"    return EXECUTOR"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7a1099c3_8f13f2a9","line":55,"in_reply_to":"eae0e717_9dcd0e8c","updated":"2023-08-16 07:31:01.000000000","message":"Done","commit_id":"2bcfbf111a64bf4ec36de4f9b6f11849ce1e05ad"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"512d9c45ea535a504fe032bbf7d59aade959f25c","unresolved":true,"context_lines":[{"line_number":431,"context_line":"            if not isinstance(e, exception.NovaException):"},{"line_number":432,"context_line":"                LOG.exception(\u0027Error gathering result from cell %s\u0027, cell_uuid)"},{"line_number":433,"context_line":"            result \u003d e"},{"line_number":434,"context_line":"        return result"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"    for cell_mapping in cell_mappings:"},{"line_number":437,"context_line":"        with target_cell(context, cell_mapping) as cctxt:"}],"source_content_type":"text/x-python","patch_set":15,"id":"bba25bb8_8402df44","line":434,"updated":"2023-08-14 14:01:43.000000000","message":"Is there some reason we need to stop using the queue here? Technically we already wait for them all to finish before we proceed, but the queue allows us to do work while we wait for some and I think the initial goal was to turn this into an iterator for cases where our caller can make progress while we wait for slow cells. Not a huge deal, but I guess I\u0027d rather not change it unless we need to.","commit_id":"2bcfbf111a64bf4ec36de4f9b6f11849ce1e05ad"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"65142751a4c703f43358041269ff5cd01a814f13","unresolved":true,"context_lines":[{"line_number":431,"context_line":"            if not isinstance(e, exception.NovaException):"},{"line_number":432,"context_line":"                LOG.exception(\u0027Error gathering result from cell %s\u0027, cell_uuid)"},{"line_number":433,"context_line":"            result \u003d e"},{"line_number":434,"context_line":"        return result"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"    for cell_mapping in cell_mappings:"},{"line_number":437,"context_line":"        with target_cell(context, cell_mapping) as cctxt:"}],"source_content_type":"text/x-python","patch_set":15,"id":"b9fd6ac1_14eb4ef7","line":434,"in_reply_to":"8f41f6c8_0889cdc5","updated":"2023-08-16 07:31:01.000000000","message":"I can\u0027t remember exactly but I think it\u0027s probably because I didn\u0027t see that using the queue was currently accomplishing anything different than waiting for all with a timeout?","commit_id":"2bcfbf111a64bf4ec36de4f9b6f11849ce1e05ad"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6228b3c4585e7da1e78464e1b1321d3580b5be83","unresolved":false,"context_lines":[{"line_number":431,"context_line":"            if not isinstance(e, exception.NovaException):"},{"line_number":432,"context_line":"                LOG.exception(\u0027Error gathering result from cell %s\u0027, cell_uuid)"},{"line_number":433,"context_line":"            result \u003d e"},{"line_number":434,"context_line":"        return result"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"    for cell_mapping in cell_mappings:"},{"line_number":437,"context_line":"        with target_cell(context, cell_mapping) as cctxt:"}],"source_content_type":"text/x-python","patch_set":15,"id":"bfd22fe3_c4d2776f","line":434,"in_reply_to":"b9fd6ac1_14eb4ef7","updated":"2023-08-16 15:55:13.000000000","message":"It\u0027s just a good practice of allowing the main thread to collect results as they come in, instead of waiting for all the threads to finish before we process any data. If we scatter for a reason that doesn\u0027t care about sorting, we can generate results for our caller as they come in instead of waiting.","commit_id":"2bcfbf111a64bf4ec36de4f9b6f11849ce1e05ad"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ba97c0d97a1f4c08c2b39af7cc2059a0bd217574","unresolved":true,"context_lines":[{"line_number":431,"context_line":"            if not isinstance(e, exception.NovaException):"},{"line_number":432,"context_line":"                LOG.exception(\u0027Error gathering result from cell %s\u0027, cell_uuid)"},{"line_number":433,"context_line":"            result \u003d e"},{"line_number":434,"context_line":"        return result"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"    for cell_mapping in cell_mappings:"},{"line_number":437,"context_line":"        with target_cell(context, cell_mapping) as cctxt:"}],"source_content_type":"text/x-python","patch_set":15,"id":"8f41f6c8_0889cdc5","line":434,"in_reply_to":"bba25bb8_8402df44","updated":"2023-08-14 14:40:00.000000000","message":"Oh, maybe you removed this because we were importing queue from eventlet. However, that\u0027s just a wrapper around the real threading.Queue, which works the same way:\n\nhttps://docs.python.org/3/library/queue.html","commit_id":"2bcfbf111a64bf4ec36de4f9b6f11849ce1e05ad"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6228b3c4585e7da1e78464e1b1321d3580b5be83","unresolved":true,"context_lines":[{"line_number":36,"context_line":"import nova.conf"},{"line_number":37,"context_line":"from nova import exception"},{"line_number":38,"context_line":"from nova.i18n import _"},{"line_number":39,"context_line":"import nova.monkey_patch"},{"line_number":40,"context_line":"from nova import objects"},{"line_number":41,"context_line":"from nova import policy"},{"line_number":42,"context_line":"from nova import utils"}],"source_content_type":"text/x-python","patch_set":16,"id":"84fea340_e0ffed89","line":39,"updated":"2023-08-16 15:55:13.000000000","message":"I think we need to only import this if we\u0027re going to use it because it does the patching. I\u0027m hoping we\u0027re going to have some flag to avoid the need for the operators to set the environment variable. This is working now because I\u0027m hacking the variable into place in the devstack patch.\n\nPerhaps we need to set some flag or variable from `cmd/*` which won\u0027t get set in the bare wsgi case, and then base our patching behavior *and* our threadpool selection on that?","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"702366b84edfb341720cf3a99c6d131b6fa9da46","unresolved":false,"context_lines":[{"line_number":36,"context_line":"import nova.conf"},{"line_number":37,"context_line":"from nova import exception"},{"line_number":38,"context_line":"from nova.i18n import _"},{"line_number":39,"context_line":"import nova.monkey_patch"},{"line_number":40,"context_line":"from nova import objects"},{"line_number":41,"context_line":"from nova import policy"},{"line_number":42,"context_line":"from nova import utils"}],"source_content_type":"text/x-python","patch_set":16,"id":"67cfb5aa_2665000f","line":39,"in_reply_to":"41b829bc_66c15e9d","updated":"2023-08-18 02:10:09.000000000","message":"Done","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ae55b0d9ea4ce9912e5326f35b97171f8be8c678","unresolved":true,"context_lines":[{"line_number":36,"context_line":"import nova.conf"},{"line_number":37,"context_line":"from nova import exception"},{"line_number":38,"context_line":"from nova.i18n import _"},{"line_number":39,"context_line":"import nova.monkey_patch"},{"line_number":40,"context_line":"from nova import objects"},{"line_number":41,"context_line":"from nova import policy"},{"line_number":42,"context_line":"from nova import utils"}],"source_content_type":"text/x-python","patch_set":16,"id":"41b829bc_66c15e9d","line":39,"in_reply_to":"84fea340_e0ffed89","updated":"2023-08-16 17:14:20.000000000","message":"OK. I had thought you meant a flag to communicate the existing patch/not-patched state to nova/context.py without any other changes.\n\nI can do the cmd/* thing.","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6228b3c4585e7da1e78464e1b1321d3580b5be83","unresolved":true,"context_lines":[{"line_number":65,"context_line":"        else:"},{"line_number":66,"context_line":"            executor_class \u003d futurist.GreenThreadPoolExecutor"},{"line_number":67,"context_line":"        workers \u003d CONF.osapi_compute_workers or processutils.get_worker_count()"},{"line_number":68,"context_line":"        EXECUTOR \u003d executor_class(max_workers\u003dworkers)"},{"line_number":69,"context_line":"    return EXECUTOR"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"6272e0bc_0a009d8d","line":68,"updated":"2023-08-16 15:55:13.000000000","message":"This will limit the eventlet case to that number of threads, which is a sort of regression from what we have today, I think. Right now we\u0027re unbounded (AFAIK) but the default for `GreenThreadPoolExecutor` is 1000, so our default will be way smaller than we need for that mode. IMHO, just set `max_workers` for the native case and not for the green case.","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"702366b84edfb341720cf3a99c6d131b6fa9da46","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        else:"},{"line_number":66,"context_line":"            executor_class \u003d futurist.GreenThreadPoolExecutor"},{"line_number":67,"context_line":"        workers \u003d CONF.osapi_compute_workers or processutils.get_worker_count()"},{"line_number":68,"context_line":"        EXECUTOR \u003d executor_class(max_workers\u003dworkers)"},{"line_number":69,"context_line":"    return EXECUTOR"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"22a9784d_fc614033","line":68,"in_reply_to":"55f2927a_c0f36a09","updated":"2023-08-18 02:10:09.000000000","message":"Done","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ae55b0d9ea4ce9912e5326f35b97171f8be8c678","unresolved":true,"context_lines":[{"line_number":65,"context_line":"        else:"},{"line_number":66,"context_line":"            executor_class \u003d futurist.GreenThreadPoolExecutor"},{"line_number":67,"context_line":"        workers \u003d CONF.osapi_compute_workers or processutils.get_worker_count()"},{"line_number":68,"context_line":"        EXECUTOR \u003d executor_class(max_workers\u003dworkers)"},{"line_number":69,"context_line":"    return EXECUTOR"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"55f2927a_c0f36a09","line":68,"in_reply_to":"6272e0bc_0a009d8d","updated":"2023-08-16 17:14:20.000000000","message":"OK, can do. I had thought it would be expected to be symmetrical.","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6228b3c4585e7da1e78464e1b1321d3580b5be83","unresolved":true,"context_lines":[{"line_number":467,"context_line":"    with timeutils.StopWatch() as timer:"},{"line_number":468,"context_line":"        while len(results) \u003c len(threads):"},{"line_number":469,"context_line":"            try:"},{"line_number":470,"context_line":"                cell_uuid, result \u003d queue.get_nowait()"},{"line_number":471,"context_line":"                results[cell_uuid] \u003d result"},{"line_number":472,"context_line":"            except empty_exception:"},{"line_number":473,"context_line":"                # The Empty exception is raised if no item was immediately"}],"source_content_type":"text/x-python","patch_set":16,"id":"275fb772_871ed5ed","line":470,"updated":"2023-08-16 15:55:13.000000000","message":"`queue.get()` should yield in the eventlet case and block (as desired) in the native case. Doing this `get_nowait()`/`sleep(0)` method will burn CPU in a tight loop while everything is waiting on IO. Why not use the blocking get like before?","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ef90be31874d36c95661a7ff6dcff36c98f05ce8","unresolved":true,"context_lines":[{"line_number":467,"context_line":"    with timeutils.StopWatch() as timer:"},{"line_number":468,"context_line":"        while len(results) \u003c len(threads):"},{"line_number":469,"context_line":"            try:"},{"line_number":470,"context_line":"                cell_uuid, result \u003d queue.get_nowait()"},{"line_number":471,"context_line":"                results[cell_uuid] \u003d result"},{"line_number":472,"context_line":"            except empty_exception:"},{"line_number":473,"context_line":"                # The Empty exception is raised if no item was immediately"}],"source_content_type":"text/x-python","patch_set":16,"id":"a1fdc805_e44b1aca","line":470,"in_reply_to":"077cdf36_650b907e","updated":"2023-08-16 18:32:31.000000000","message":"Right now it all blocks the caller. However, the point is that if we get something off the queue we can hand it to the caller (as a generator, which was the original plan) and then go grab the next thing. If there\u0027s nothing in the queue, there\u0027s nothing for them to work on, but as soon as there is, we can give it to them if we don\u0027t wait for them all to finish before we even start.","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"702366b84edfb341720cf3a99c6d131b6fa9da46","unresolved":false,"context_lines":[{"line_number":467,"context_line":"    with timeutils.StopWatch() as timer:"},{"line_number":468,"context_line":"        while len(results) \u003c len(threads):"},{"line_number":469,"context_line":"            try:"},{"line_number":470,"context_line":"                cell_uuid, result \u003d queue.get_nowait()"},{"line_number":471,"context_line":"                results[cell_uuid] \u003d result"},{"line_number":472,"context_line":"            except empty_exception:"},{"line_number":473,"context_line":"                # The Empty exception is raised if no item was immediately"}],"source_content_type":"text/x-python","patch_set":16,"id":"d1ad6ec6_f6b136ea","line":470,"in_reply_to":"138256d2_2e94c6e7","updated":"2023-08-18 02:10:09.000000000","message":"Done","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ae55b0d9ea4ce9912e5326f35b97171f8be8c678","unresolved":true,"context_lines":[{"line_number":467,"context_line":"    with timeutils.StopWatch() as timer:"},{"line_number":468,"context_line":"        while len(results) \u003c len(threads):"},{"line_number":469,"context_line":"            try:"},{"line_number":470,"context_line":"                cell_uuid, result \u003d queue.get_nowait()"},{"line_number":471,"context_line":"                results[cell_uuid] \u003d result"},{"line_number":472,"context_line":"            except empty_exception:"},{"line_number":473,"context_line":"                # The Empty exception is raised if no item was immediately"}],"source_content_type":"text/x-python","patch_set":16,"id":"077cdf36_650b907e","line":470,"in_reply_to":"275fb772_871ed5ed","updated":"2023-08-16 17:14:20.000000000","message":"OK, I guess I got confused by your saying \"let the caller make progress while waiting for scatter gather to happen\" and I was thinking if this blocks waiting for a queue item to become available, then it blocks the caller?\n\nI\u0027ll change it back.","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e954bef080cb9ffcfe56b0bfcd4e247519fa5ce4","unresolved":true,"context_lines":[{"line_number":467,"context_line":"    with timeutils.StopWatch() as timer:"},{"line_number":468,"context_line":"        while len(results) \u003c len(threads):"},{"line_number":469,"context_line":"            try:"},{"line_number":470,"context_line":"                cell_uuid, result \u003d queue.get_nowait()"},{"line_number":471,"context_line":"                results[cell_uuid] \u003d result"},{"line_number":472,"context_line":"            except empty_exception:"},{"line_number":473,"context_line":"                # The Empty exception is raised if no item was immediately"}],"source_content_type":"text/x-python","patch_set":16,"id":"138256d2_2e94c6e7","line":470,"in_reply_to":"a1fdc805_e44b1aca","updated":"2023-08-17 04:49:34.000000000","message":"OK, I don\u0027t remember the original plan. Thanks for explaining it.","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"dc257fb67b9294c3188bab52a3636ffed9d36e85","unresolved":true,"context_lines":[{"line_number":61,"context_line":"        else:"},{"line_number":62,"context_line":"            EXECUTOR \u003d futurist.ThreadPoolExecutor("},{"line_number":63,"context_line":"                max_workers\u003dCONF.osapi_compute_workers)"},{"line_number":64,"context_line":"    return EXECUTOR"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"class _ContextAuthPlugin(plugin.BaseAuthPlugin):"}],"source_content_type":"text/x-python","patch_set":19,"id":"c4e3ae46_f9582436","line":64,"updated":"2023-08-18 10:19:36.000000000","message":"Just realized I need to update this too to handle the environment variable override to opt in for eventlet (if we want that ... see nova/monkey_patch.py).","commit_id":"f713b0f52b56e2c26654928d39faec7fa6418b00"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6c887cb46cba4ea5ca9c8a3c929990285cec4041","unresolved":true,"context_lines":[{"line_number":61,"context_line":"        else:"},{"line_number":62,"context_line":"            EXECUTOR \u003d futurist.ThreadPoolExecutor("},{"line_number":63,"context_line":"                max_workers\u003dCONF.osapi_compute_workers)"},{"line_number":64,"context_line":"    return EXECUTOR"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"class _ContextAuthPlugin(plugin.BaseAuthPlugin):"}],"source_content_type":"text/x-python","patch_set":19,"id":"da623864_5004ebbc","line":64,"in_reply_to":"c4e3ae46_f9582436","updated":"2023-08-22 13:51:20.000000000","message":"I dunno, if we default to native and only set the flag in the `cmd/*` things then we\u0027ll not break the docs thing (which is why we have the environment variable, IIRC), right?","commit_id":"f713b0f52b56e2c26654928d39faec7fa6418b00"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"88758ca9824145deea85783d23989c3dc3ba9307","unresolved":true,"context_lines":[{"line_number":61,"context_line":"        else:"},{"line_number":62,"context_line":"            EXECUTOR \u003d futurist.ThreadPoolExecutor("},{"line_number":63,"context_line":"                max_workers\u003dCONF.osapi_compute_workers)"},{"line_number":64,"context_line":"    return EXECUTOR"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"class _ContextAuthPlugin(plugin.BaseAuthPlugin):"}],"source_content_type":"text/x-python","patch_set":19,"id":"fdb2b433_d86d0a46","line":64,"in_reply_to":"da623864_5004ebbc","updated":"2023-08-22 19:12:48.000000000","message":"Hm, yeah, I would think probably. I\u0027m a bit wary of removing the environment variable in case other things are using it for other reasons ... and also it could double as a way for people to opt-in to the old behavior if they for some reason need to. Maybe there\u0027s another way or a better way to enable that.","commit_id":"f713b0f52b56e2c26654928d39faec7fa6418b00"}],"nova/monkey_patch.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f48580eef46742f661f5ece97892ea309f843614","unresolved":true,"context_lines":[{"line_number":79,"context_line":"                    \"imported prior to eventlet monkey patching: %s. This \""},{"line_number":80,"context_line":"                    \"warning can usually be ignored if the caller is only \""},{"line_number":81,"context_line":"                    \"importing and not executing nova code.\","},{"line_number":82,"context_line":"                    \u0027, \u0027.join(problems))"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"# NOTE(melwitt): This double negative is weird but it\u0027s only here as a way for"}],"source_content_type":"text/x-python","patch_set":19,"id":"f5b9a9b2_9452ac54","line":82,"updated":"2023-08-22 12:14:36.000000000","message":"instead of setting\n\nnova.ENABLE_EVENTLET_PATCHING \u003d True  in other files by the way it might be better to default it to False and then only set it to true in _monkey_patch() here\nif we have monkey patched.\n\nwitht that said eventlet has funcction to tell you if we are monky patched so im not really sure i like having this global.","commit_id":"f713b0f52b56e2c26654928d39faec7fa6418b00"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"88758ca9824145deea85783d23989c3dc3ba9307","unresolved":true,"context_lines":[{"line_number":79,"context_line":"                    \"imported prior to eventlet monkey patching: %s. This \""},{"line_number":80,"context_line":"                    \"warning can usually be ignored if the caller is only \""},{"line_number":81,"context_line":"                    \"importing and not executing nova code.\","},{"line_number":82,"context_line":"                    \u0027, \u0027.join(problems))"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"# NOTE(melwitt): This double negative is weird but it\u0027s only here as a way for"}],"source_content_type":"text/x-python","patch_set":19,"id":"22e0bac2_8096397e","line":82,"in_reply_to":"f5b9a9b2_9452ac54","updated":"2023-08-22 19:12:48.000000000","message":"That\u0027s a bit different than what this is doing. This is a flag that instructs whether to monkey patch at all ... which is different than indicating that we have patched after the fact. There is indeed an oslo_utils.eventletutils.is_monkey_patched() utility for doing what you describe. I actually used it at one point while working on this and realized it\u0027s backward from what Dan was describing which is deciding whether to monkey patch, not checking whether we did already patch.","commit_id":"f713b0f52b56e2c26654928d39faec7fa6418b00"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"dc257fb67b9294c3188bab52a3636ffed9d36e85","unresolved":true,"context_lines":[{"line_number":88,"context_line":"opt_in_for_eventlet_patching \u003d ("},{"line_number":89,"context_line":"    os.environ.get(\u0027OS_NOVA_DISABLE_EVENTLET_PATCHING\u0027, \u0027\u0027).lower()"},{"line_number":90,"context_line":"        in (\u00270\u0027, \u0027false\u0027, \u0027no\u0027))"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"# The ENABLE_EVENTLET_PATCHING flag is set to True only by code that requires"},{"line_number":93,"context_line":"# eventlet, such as nova/cmd/* and nova/tests/*."},{"line_number":94,"context_line":"if nova.ENABLE_EVENTLET_PATCHING or opt_in_for_eventlet_patching:"}],"source_content_type":"text/x-python","patch_set":19,"id":"660c2b42_660218d7","line":91,"updated":"2023-08-18 10:19:36.000000000","message":"Or alternatively I could add:\n```\nif opt_in_for_eventlet_patching:\n    nova.ENABLE_EVENTLET_PATCHING \u003d True\n```\nto avoid needing to handle the environment variable directly in nova/context.py.","commit_id":"f713b0f52b56e2c26654928d39faec7fa6418b00"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"88758ca9824145deea85783d23989c3dc3ba9307","unresolved":true,"context_lines":[{"line_number":88,"context_line":"opt_in_for_eventlet_patching \u003d ("},{"line_number":89,"context_line":"    os.environ.get(\u0027OS_NOVA_DISABLE_EVENTLET_PATCHING\u0027, \u0027\u0027).lower()"},{"line_number":90,"context_line":"        in (\u00270\u0027, \u0027false\u0027, \u0027no\u0027))"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"# The ENABLE_EVENTLET_PATCHING flag is set to True only by code that requires"},{"line_number":93,"context_line":"# eventlet, such as nova/cmd/* and nova/tests/*."},{"line_number":94,"context_line":"if nova.ENABLE_EVENTLET_PATCHING or opt_in_for_eventlet_patching:"}],"source_content_type":"text/x-python","patch_set":19,"id":"4581999a_c1236e6a","line":91,"in_reply_to":"2058026c_cf170e1c","updated":"2023-08-22 19:12:48.000000000","message":"\u003e if your changing this then can we use bool_from_string form oslo strutils\noslo https://github.com/openstack/oslo.utils/blob/master/oslo_utils/strutils.py#L157-L158\n\nAh, yeah I should definitely use bool_from_string instead of the manual thing here.\n\n\u003e I also think we could maybe improve things by not patching on import, and just call the patcher function from places explicitly when we need to do it. That would clear up when and where it\u0027s being done, when and where we\u0027re honoring the environment variable, etc. I know that means we\u0027d have to import it and call the function before other imports, but I think I\u0027d prefer that to the magic import-means-patch we have now.\n\u003e\n\u003e I do think what melwitt has here is better than what we had before, which is that importing anything in cmd will cause us to set the flag to true. That\u0027s better than anything that imports nova.monkey_patch but it\u0027s also still just a tad less discoverable than being totally explicit, I think. Maybe it\u0027s worth doing it this way to avoid the \"import, run, import everything else\" workflow we\u0027d have to do.\n\nAt one point I did do the import, run, import everything else and ran into https://www.flake8rules.com/rules/E402.html. The `# noqa` annotation didn\u0027t get around it but just now in looking at it again I\u0027m reminded there is an ignore list we can use in tox.ini, so we could do that.\n\nI could give that another try to see if we like that better. I too am a little wary about the \"magic\" and that this is just less magic than the current magic 😛","commit_id":"f713b0f52b56e2c26654928d39faec7fa6418b00"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6c887cb46cba4ea5ca9c8a3c929990285cec4041","unresolved":true,"context_lines":[{"line_number":88,"context_line":"opt_in_for_eventlet_patching \u003d ("},{"line_number":89,"context_line":"    os.environ.get(\u0027OS_NOVA_DISABLE_EVENTLET_PATCHING\u0027, \u0027\u0027).lower()"},{"line_number":90,"context_line":"        in (\u00270\u0027, \u0027false\u0027, \u0027no\u0027))"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"# The ENABLE_EVENTLET_PATCHING flag is set to True only by code that requires"},{"line_number":93,"context_line":"# eventlet, such as nova/cmd/* and nova/tests/*."},{"line_number":94,"context_line":"if nova.ENABLE_EVENTLET_PATCHING or opt_in_for_eventlet_patching:"}],"source_content_type":"text/x-python","patch_set":19,"id":"2058026c_cf170e1c","line":91,"in_reply_to":"53ede2cc_1f0fee96","updated":"2023-08-22 13:51:20.000000000","message":"Yeah, I think I\u0027d prefer to only have the environment variable be honored either here or in `cmd/*`.\n\nI also think we could maybe improve things by *not* patching on import, and just call the patcher function from places explicitly when we need to do it. That would clear up when and where it\u0027s being done, when and where we\u0027re honoring the environment variable, etc. I know that means we\u0027d have to import it and call the function before other imports, but I think I\u0027d prefer that to the magic import-means-patch we have now.\n\nI do think what melwitt has here is better than what we had before, which is that importing anything in `cmd` will cause us to set the flag to true. That\u0027s better than anything that imports `nova.monkey_patch` but it\u0027s also still just a tad less discoverable than being totally explicit, I think. Maybe it\u0027s worth doing it this way to avoid the \"import, run, import everything else\" workflow we\u0027d have to do.","commit_id":"f713b0f52b56e2c26654928d39faec7fa6418b00"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f48580eef46742f661f5ece97892ea309f843614","unresolved":true,"context_lines":[{"line_number":88,"context_line":"opt_in_for_eventlet_patching \u003d ("},{"line_number":89,"context_line":"    os.environ.get(\u0027OS_NOVA_DISABLE_EVENTLET_PATCHING\u0027, \u0027\u0027).lower()"},{"line_number":90,"context_line":"        in (\u00270\u0027, \u0027false\u0027, \u0027no\u0027))"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"# The ENABLE_EVENTLET_PATCHING flag is set to True only by code that requires"},{"line_number":93,"context_line":"# eventlet, such as nova/cmd/* and nova/tests/*."},{"line_number":94,"context_line":"if nova.ENABLE_EVENTLET_PATCHING or opt_in_for_eventlet_patching:"}],"source_content_type":"text/x-python","patch_set":19,"id":"53ede2cc_1f0fee96","line":91,"in_reply_to":"660c2b42_660218d7","updated":"2023-08-22 12:14:36.000000000","message":"if your changing this then can we use bool_from_string form oslo strutils \noslo https://github.com/openstack/oslo.utils/blob/master/oslo_utils/strutils.py#L157-L158\n\nso just do \n```\ndisable_monkey_patching \u003d bool_from_string(\n    os.environ.get(\u0027OS_NOVA_DISABLE_EVENTLET_PATCHING\u0027, \u0027\u0027)\n)\n\nif disable_monkey_patching:\n    nova.ENABLE_EVENTLET_PATCHING \u003d False\n    \n# The ENABLE_EVENTLET_PATCHING flag is set to True only by code that requires\n# eventlet, such as nova/cmd/* and nova/tests/*.\nif nova.ENABLE_EVENTLET_PATCHING:\n    _monkey_patch()\n```\n\nim still not sure this is better the the currnt approch where this is only imported if monky patching is requried.","commit_id":"f713b0f52b56e2c26654928d39faec7fa6418b00"}],"nova/test.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9d19c49e511da027709727b28b01f2f0c124b1ee","unresolved":false,"context_lines":[{"line_number":310,"context_line":"        self.flags(build_failure_weight_multiplier\u003d0.0,"},{"line_number":311,"context_line":"                   group\u003d\u0027filter_scheduler\u0027)"},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"        # NOTE(melwitt): Avoid mixing native threads with eventlet green"},{"line_number":314,"context_line":"        # threads in the test environment (deadlock can occur)."},{"line_number":315,"context_line":"        self.useFixture(fixtures.MonkeyPatch(\u0027futurist.ThreadPoolExecutor\u0027,"},{"line_number":316,"context_line":"                                             futurist.GreenThreadPoolExecutor))"},{"line_number":317,"context_line":"        # Create the executor again after patching."},{"line_number":318,"context_line":"        context.EXECUTOR \u003d context.create_executor()"},{"line_number":319,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"dfbec78f_85506c0c","line":316,"range":{"start_line":313,"start_character":0,"end_line":316,"end_character":79},"updated":"2019-05-08 11:47:15.000000000","message":"Does this mean that we are not testing with the thread executor in our tests but we are running with that executor? It feels we are loosing test coverage by this.\n\nIs this just temporary until eventlet is fully removed from the api?","commit_id":"011eae40f64d227c31a5be297de5dabbd42eebef"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"79108a9591989667eedf45b3a5dd86ec4e1e1ad3","unresolved":false,"context_lines":[{"line_number":310,"context_line":"        self.flags(build_failure_weight_multiplier\u003d0.0,"},{"line_number":311,"context_line":"                   group\u003d\u0027filter_scheduler\u0027)"},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"        # NOTE(melwitt): Avoid mixing native threads with eventlet green"},{"line_number":314,"context_line":"        # threads in the test environment (deadlock can occur)."},{"line_number":315,"context_line":"        self.useFixture(fixtures.MonkeyPatch(\u0027futurist.ThreadPoolExecutor\u0027,"},{"line_number":316,"context_line":"                                             futurist.GreenThreadPoolExecutor))"},{"line_number":317,"context_line":"        # Create the executor again after patching."},{"line_number":318,"context_line":"        context.EXECUTOR \u003d context.create_executor()"},{"line_number":319,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"dfbec78f_08a32ca0","line":316,"range":{"start_line":313,"start_character":0,"end_line":316,"end_character":79},"in_reply_to":"dfbec78f_85506c0c","updated":"2019-05-08 17:24:38.000000000","message":"I agree, but when I leave this as a ThreadPoolExecutor, the unit tests deadlock (but the functional tests do not). I had thought the deadlock happens because the native threads will not yield back to the eventlet green threads. Do you know of any way I could keep ThreadPoolExecutor in the test environments and not deadlock with other eventlet code?","commit_id":"011eae40f64d227c31a5be297de5dabbd42eebef"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9d19c49e511da027709727b28b01f2f0c124b1ee","unresolved":false,"context_lines":[{"line_number":315,"context_line":"        self.useFixture(fixtures.MonkeyPatch(\u0027futurist.ThreadPoolExecutor\u0027,"},{"line_number":316,"context_line":"                                             futurist.GreenThreadPoolExecutor))"},{"line_number":317,"context_line":"        # Create the executor again after patching."},{"line_number":318,"context_line":"        context.EXECUTOR \u003d context.create_executor()"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"    def _setup_cells(self):"},{"line_number":321,"context_line":"        \"\"\"Setup a normal cellsv2 environment."}],"source_content_type":"text/x-python","patch_set":14,"id":"dfbec78f_25574005","line":318,"range":{"start_line":318,"start_character":8,"end_line":318,"end_character":52},"updated":"2019-05-08 11:47:15.000000000","message":"global :/","commit_id":"011eae40f64d227c31a5be297de5dabbd42eebef"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a05c553a78618f275ecaf06b19af3f731d37a0ea","unresolved":true,"context_lines":[{"line_number":325,"context_line":"        # the currently running test to fail."},{"line_number":326,"context_line":"        self.useFixture(nova_fixtures.GreenThreadPoolShutdownWait())"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"        # NOTE(melwitt): Avoid mixing native threads with eventlet green"},{"line_number":329,"context_line":"        # threads (in nova-compute, for example) in the test environment"},{"line_number":330,"context_line":"        # (deadlock can occur)."},{"line_number":331,"context_line":"        self.useFixture(fixtures.MonkeyPatch(\u0027futurist.ThreadPoolExecutor\u0027,"}],"source_content_type":"text/x-python","patch_set":15,"id":"ec4c20fa_4eacc731","line":328,"updated":"2023-08-14 11:05:46.000000000","message":"can you also remove https://github.com/openstack/nova/blob/e8d7380759619871b7537d8c3010df31da9f3a4d/nova/api/openstack/__init__.py#L20\nso that we dont mix eventlets with thread in the wsgi case.\n\nusing the nova-api concole script is already patched by https://github.com/openstack/nova/blob/e8d7380759619871b7537d8c3010df31da9f3a4d/nova/cmd/__init__.py#L16\n\nso we should be able to remove the one in the api module","commit_id":"2bcfbf111a64bf4ec36de4f9b6f11849ce1e05ad"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f3e8b311142d91fa7217140d4a4b8a99cb54d7cf","unresolved":false,"context_lines":[{"line_number":325,"context_line":"        # the currently running test to fail."},{"line_number":326,"context_line":"        self.useFixture(nova_fixtures.GreenThreadPoolShutdownWait())"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"        # NOTE(melwitt): Avoid mixing native threads with eventlet green"},{"line_number":329,"context_line":"        # threads (in nova-compute, for example) in the test environment"},{"line_number":330,"context_line":"        # (deadlock can occur)."},{"line_number":331,"context_line":"        self.useFixture(fixtures.MonkeyPatch(\u0027futurist.ThreadPoolExecutor\u0027,"}],"source_content_type":"text/x-python","patch_set":15,"id":"3163282d_769a2007","line":328,"in_reply_to":"ec4c20fa_4eacc731","updated":"2024-01-09 13:14:41.000000000","message":"Acknowledged","commit_id":"2bcfbf111a64bf4ec36de4f9b6f11849ce1e05ad"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6228b3c4585e7da1e78464e1b1321d3580b5be83","unresolved":true,"context_lines":[{"line_number":328,"context_line":"        # NOTE(melwitt): Avoid mixing native threads with eventlet green"},{"line_number":329,"context_line":"        # threads (in nova-compute, for example) in the test environment"},{"line_number":330,"context_line":"        # (deadlock can occur)."},{"line_number":331,"context_line":"        self.useFixture(fixtures.MonkeyPatch(\u0027futurist.ThreadPoolExecutor\u0027,"},{"line_number":332,"context_line":"                                             futurist.GreenThreadPoolExecutor))"},{"line_number":333,"context_line":"        # Reset the executor again after patching."},{"line_number":334,"context_line":"        context.EXECUTOR \u003d None"}],"source_content_type":"text/x-python","patch_set":16,"id":"459c5b57_26d27de0","line":331,"updated":"2023-08-16 15:55:13.000000000","message":"Shouldn\u0027t we just pick one and make sure we\u0027re stuck on it instead of doing this? I think I\u0027d rather make the wrong one a poison pill that raises an exception if you try to use it than mapping one to the other. That way we know we\u0027re controlling the model everywhere, and if we don\u0027t somewhere, we get an error.","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f3e8b311142d91fa7217140d4a4b8a99cb54d7cf","unresolved":false,"context_lines":[{"line_number":328,"context_line":"        # NOTE(melwitt): Avoid mixing native threads with eventlet green"},{"line_number":329,"context_line":"        # threads (in nova-compute, for example) in the test environment"},{"line_number":330,"context_line":"        # (deadlock can occur)."},{"line_number":331,"context_line":"        self.useFixture(fixtures.MonkeyPatch(\u0027futurist.ThreadPoolExecutor\u0027,"},{"line_number":332,"context_line":"                                             futurist.GreenThreadPoolExecutor))"},{"line_number":333,"context_line":"        # Reset the executor again after patching."},{"line_number":334,"context_line":"        context.EXECUTOR \u003d None"}],"source_content_type":"text/x-python","patch_set":16,"id":"777876df_b7918a31","line":331,"in_reply_to":"3c537eac_802d8043","updated":"2024-01-09 13:14:41.000000000","message":"Acknowledged","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ae55b0d9ea4ce9912e5326f35b97171f8be8c678","unresolved":true,"context_lines":[{"line_number":328,"context_line":"        # NOTE(melwitt): Avoid mixing native threads with eventlet green"},{"line_number":329,"context_line":"        # threads (in nova-compute, for example) in the test environment"},{"line_number":330,"context_line":"        # (deadlock can occur)."},{"line_number":331,"context_line":"        self.useFixture(fixtures.MonkeyPatch(\u0027futurist.ThreadPoolExecutor\u0027,"},{"line_number":332,"context_line":"                                             futurist.GreenThreadPoolExecutor))"},{"line_number":333,"context_line":"        # Reset the executor again after patching."},{"line_number":334,"context_line":"        context.EXECUTOR \u003d None"}],"source_content_type":"text/x-python","patch_set":16,"id":"3c537eac_802d8043","line":331,"in_reply_to":"459c5b57_26d27de0","updated":"2023-08-16 17:14:20.000000000","message":"OK, sure.","commit_id":"aaae218c38b6bddcb406c72fd5b331ca16d5a21e"}],"nova/tests/fixtures.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"71d4ac7afe2d4b7ef4a552819bd682a156f51d87","unresolved":false,"context_lines":[{"line_number":1102,"context_line":"            \u0027nova.utils.spawn\u0027, _FakeGreenThread))"},{"line_number":1103,"context_line":""},{"line_number":1104,"context_line":""},{"line_number":1105,"context_line":"class SynchronousThreadPoolExecutorFixture(fixtures.Fixture):"},{"line_number":1106,"context_line":"    \"\"\"Make GreenThreadPoolExecutor.submit() synchronous."},{"line_number":1107,"context_line":""},{"line_number":1108,"context_line":"    The function passed to submit() will be executed and a mock.Mock"},{"line_number":1109,"context_line":"    object will be returned as the Future where Future.result() will"},{"line_number":1110,"context_line":"    return the result of the call to the submitted function."},{"line_number":1111,"context_line":"    \"\"\""},{"line_number":1112,"context_line":"    def __init__(self, done_return_values\u003dNone):"},{"line_number":1113,"context_line":"        super(SynchronousThreadPoolExecutorFixture, self).__init__()"},{"line_number":1114,"context_line":"        self._done_return_values \u003d done_return_values"},{"line_number":1115,"context_line":"        self.future_mocks \u003d []"},{"line_number":1116,"context_line":""},{"line_number":1117,"context_line":"    def setUp(self):"},{"line_number":1118,"context_line":"        super(SynchronousThreadPoolExecutorFixture, self).setUp()"},{"line_number":1119,"context_line":""},{"line_number":1120,"context_line":"        def fake_submit(_self, fn, *args, **kwargs):"},{"line_number":1121,"context_line":"            result \u003d fn(*args, **kwargs)"},{"line_number":1122,"context_line":"            future \u003d mock.Mock(spec\u003dfuturist.Future)"},{"line_number":1123,"context_line":"            future.result.return_value \u003d result"},{"line_number":1124,"context_line":"            if self._done_return_values:"},{"line_number":1125,"context_line":"                value \u003d self._done_return_values.pop(0)"},{"line_number":1126,"context_line":"                future.done.return_value \u003d value"},{"line_number":1127,"context_line":"            self.future_mocks.append(future)"},{"line_number":1128,"context_line":"            return future"},{"line_number":1129,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":1130,"context_line":"            \u0027futurist.GreenThreadPoolExecutor.submit\u0027,"},{"line_number":1131,"context_line":"            fake_submit))"},{"line_number":1132,"context_line":""},{"line_number":1133,"context_line":""},{"line_number":1134,"context_line":"class BannedDBSchemaOperations(fixtures.Fixture):"}],"source_content_type":"text/x-python","patch_set":5,"id":"5fc1f717_5e19a6d4","line":1131,"range":{"start_line":1105,"start_character":0,"end_line":1131,"end_character":25},"updated":"2019-04-08 11:43:17.000000000","message":"We should delete this entirely, and just configure the synchronous executor in nova config for tests.","commit_id":"0481bdf956ee8b3fe4d4cbd122958fb830315a0b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d78497e88447c3bd7a9dc151892f7e245806717e","unresolved":false,"context_lines":[{"line_number":1102,"context_line":"            \u0027nova.utils.spawn\u0027, _FakeGreenThread))"},{"line_number":1103,"context_line":""},{"line_number":1104,"context_line":""},{"line_number":1105,"context_line":"class SynchronousThreadPoolExecutorFixture(fixtures.Fixture):"},{"line_number":1106,"context_line":"    \"\"\"Make GreenThreadPoolExecutor.submit() synchronous."},{"line_number":1107,"context_line":""},{"line_number":1108,"context_line":"    The function passed to submit() will be executed and a mock.Mock"},{"line_number":1109,"context_line":"    object will be returned as the Future where Future.result() will"},{"line_number":1110,"context_line":"    return the result of the call to the submitted function."},{"line_number":1111,"context_line":"    \"\"\""},{"line_number":1112,"context_line":"    def __init__(self, done_return_values\u003dNone):"},{"line_number":1113,"context_line":"        super(SynchronousThreadPoolExecutorFixture, self).__init__()"},{"line_number":1114,"context_line":"        self._done_return_values \u003d done_return_values"},{"line_number":1115,"context_line":"        self.future_mocks \u003d []"},{"line_number":1116,"context_line":""},{"line_number":1117,"context_line":"    def setUp(self):"},{"line_number":1118,"context_line":"        super(SynchronousThreadPoolExecutorFixture, self).setUp()"},{"line_number":1119,"context_line":""},{"line_number":1120,"context_line":"        def fake_submit(_self, fn, *args, **kwargs):"},{"line_number":1121,"context_line":"            result \u003d fn(*args, **kwargs)"},{"line_number":1122,"context_line":"            future \u003d mock.Mock(spec\u003dfuturist.Future)"},{"line_number":1123,"context_line":"            future.result.return_value \u003d result"},{"line_number":1124,"context_line":"            if self._done_return_values:"},{"line_number":1125,"context_line":"                value \u003d self._done_return_values.pop(0)"},{"line_number":1126,"context_line":"                future.done.return_value \u003d value"},{"line_number":1127,"context_line":"            self.future_mocks.append(future)"},{"line_number":1128,"context_line":"            return future"},{"line_number":1129,"context_line":"        self.useFixture(fixtures.MonkeyPatch("},{"line_number":1130,"context_line":"            \u0027futurist.GreenThreadPoolExecutor.submit\u0027,"},{"line_number":1131,"context_line":"            fake_submit))"},{"line_number":1132,"context_line":""},{"line_number":1133,"context_line":""},{"line_number":1134,"context_line":"class BannedDBSchemaOperations(fixtures.Fixture):"}],"source_content_type":"text/x-python","patch_set":5,"id":"5fc1f717_99c2bae8","line":1131,"range":{"start_line":1105,"start_character":0,"end_line":1131,"end_character":25},"in_reply_to":"5fc1f717_5e19a6d4","updated":"2019-04-08 22:28:42.000000000","message":"I will look into that.","commit_id":"0481bdf956ee8b3fe4d4cbd122958fb830315a0b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"24c8cc2958f5e71d7dc151b0c1c5bcd606eb9236","unresolved":false,"context_lines":[{"line_number":1103,"context_line":""},{"line_number":1104,"context_line":""},{"line_number":1105,"context_line":"class SynchronousThreadPoolExecutorFixture(fixtures.Fixture):"},{"line_number":1106,"context_line":"    \"\"\"Make GreenThreadPoolExecutor.submit() synchronous."},{"line_number":1107,"context_line":""},{"line_number":1108,"context_line":"    The function passed to submit() will be executed and a mock.Mock"},{"line_number":1109,"context_line":"    object will be returned as the Future where Future.result() will"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fce034c_9bf7a2f6","line":1106,"range":{"start_line":1106,"start_character":11,"end_line":1106,"end_character":45},"updated":"2019-04-12 00:01:18.000000000","message":"you know you could do this by \nhaveing the fixture just replace the greenthreadpoolExecutor with the SynchronousExecutor but that a seperate issue from the one your trying to fix in this patch.\n\nhttps://github.com/openstack/futurist/blob/23dfd9e9a8d5f785ca64d33d3cac6d29c82c6c6a/futurist/_futures.py#L230","commit_id":"f08979fe5ee4fbecbc463371ab4252a566fc5fe6"}],"nova/tests/unit/test_context.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"bffdc33df71d72f6b9b1ca475e2dc6785ae9abf0","unresolved":false,"context_lines":[{"line_number":386,"context_line":"                          mock.call.target_cell().__exit__(None, None, None)]"},{"line_number":387,"context_line":"        manager.assert_has_calls(expected_calls)"},{"line_number":388,"context_line":"        mock_wait.assert_called_once()"},{"line_number":389,"context_line":"        self.assertEqual(1, fixture.future_mocks[0].done.call_count)"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"    @mock.patch(\u0027futurist.waiters.wait_for_all\u0027)"},{"line_number":392,"context_line":"    @mock.patch(\u0027nova.context.LOG.warning\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_ef96beef","line":389,"range":{"start_line":389,"start_character":8,"end_line":389,"end_character":68},"updated":"2019-04-05 11:52:53.000000000","message":"or:\n\nfixture.future_mocks[0].done.assert_called_once_with()","commit_id":"0b0003a49a957ad724f5be29dba07d521fc66dfb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ce796e65a2f4fc00d54401a21133e7e9abd89718","unresolved":false,"context_lines":[{"line_number":386,"context_line":"                          mock.call.target_cell().__exit__(None, None, None)]"},{"line_number":387,"context_line":"        manager.assert_has_calls(expected_calls)"},{"line_number":388,"context_line":"        mock_wait.assert_called_once()"},{"line_number":389,"context_line":"        self.assertEqual(1, fixture.future_mocks[0].done.call_count)"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"    @mock.patch(\u0027futurist.waiters.wait_for_all\u0027)"},{"line_number":392,"context_line":"    @mock.patch(\u0027nova.context.LOG.warning\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_5d3eab29","line":389,"range":{"start_line":389,"start_character":8,"end_line":389,"end_character":68},"in_reply_to":"5fc1f717_ef96beef","updated":"2019-04-05 14:55:52.000000000","message":"Ah, oops, this is a holdover from an earlier version of the patch I had locally.","commit_id":"0b0003a49a957ad724f5be29dba07d521fc66dfb"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"bffdc33df71d72f6b9b1ca475e2dc6785ae9abf0","unresolved":false,"context_lines":[{"line_number":423,"context_line":"        self.assertIn(mock_get_inst.return_value, results.values())"},{"line_number":424,"context_line":"        self.assertIn(context.did_not_respond_sentinel, results.values())"},{"line_number":425,"context_line":"        mock_wait.assert_called_once()"},{"line_number":426,"context_line":"        fixture.future_mocks[0].done.assert_called_once_with()"},{"line_number":427,"context_line":"        fixture.future_mocks[1].cancel.assert_called_once_with()"},{"line_number":428,"context_line":"        self.assertTrue(mock_log_warning.called)"},{"line_number":429,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_afa0b643","line":426,"updated":"2019-04-05 11:52:53.000000000","message":"ah, like you did here :)","commit_id":"0b0003a49a957ad724f5be29dba07d521fc66dfb"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"bffdc33df71d72f6b9b1ca475e2dc6785ae9abf0","unresolved":false,"context_lines":[{"line_number":466,"context_line":"        self.assertTrue(mock_log_exception.called)"},{"line_number":467,"context_line":"        mock_wait.assert_called_once()"},{"line_number":468,"context_line":"        self.assertEqual(1, fixture.future_mocks[0].done.call_count)"},{"line_number":469,"context_line":"        self.assertEqual(1, fixture.future_mocks[1].done.call_count)"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"        # Now run it again with a NovaException to see it\u0027s not logged."},{"line_number":472,"context_line":"        mock_log_exception.reset_mock()"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_2f8cc6d5","line":469,"updated":"2019-04-05 11:52:53.000000000","message":"but not here :)","commit_id":"0b0003a49a957ad724f5be29dba07d521fc66dfb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ce796e65a2f4fc00d54401a21133e7e9abd89718","unresolved":false,"context_lines":[{"line_number":466,"context_line":"        self.assertTrue(mock_log_exception.called)"},{"line_number":467,"context_line":"        mock_wait.assert_called_once()"},{"line_number":468,"context_line":"        self.assertEqual(1, fixture.future_mocks[0].done.call_count)"},{"line_number":469,"context_line":"        self.assertEqual(1, fixture.future_mocks[1].done.call_count)"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"        # Now run it again with a NovaException to see it\u0027s not logged."},{"line_number":472,"context_line":"        mock_log_exception.reset_mock()"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_3d415fa7","line":469,"in_reply_to":"5fc1f717_2f8cc6d5","updated":"2019-04-05 14:55:52.000000000","message":"ooops :)","commit_id":"0b0003a49a957ad724f5be29dba07d521fc66dfb"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"24c8cc2958f5e71d7dc151b0c1c5bcd606eb9236","unresolved":false,"context_lines":[{"line_number":368,"context_line":"        def fake_wait_for_all(fs, timeout\u003dNone):"},{"line_number":369,"context_line":"            for f in fs:"},{"line_number":370,"context_line":"                self.assertIsInstance(f, futurist.Future)"},{"line_number":371,"context_line":"            self.assertEqual(60, timeout)"},{"line_number":372,"context_line":""},{"line_number":373,"context_line":"        mock_wait.side_effect \u003d fake_wait_for_all"},{"line_number":374,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3fce034c_fbe58639","line":371,"range":{"start_line":371,"start_character":11,"end_line":371,"end_character":41},"updated":"2019-04-12 00:01:18.000000000","message":"do we really want to hard code this especially since timeout is not defaulted to 60 above?","commit_id":"f08979fe5ee4fbecbc463371ab4252a566fc5fe6"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"24c8cc2958f5e71d7dc151b0c1c5bcd606eb9236","unresolved":false,"context_lines":[{"line_number":408,"context_line":"                                       uuid\u003duuids.cell1)"},{"line_number":409,"context_line":"        mappings \u003d objects.CellMappingList(objects\u003d[mapping0, mapping1])"},{"line_number":410,"context_line":""},{"line_number":411,"context_line":"        # Assert the wait call in a side_effect because the call_args will"},{"line_number":412,"context_line":"        # change by the time we normally assert at the end."},{"line_number":413,"context_line":"        def fake_wait_for_all(fs, timeout\u003dNone):"},{"line_number":414,"context_line":"            for f in fs:"},{"line_number":415,"context_line":"                self.assertIsInstance(f, futurist.Future)"},{"line_number":416,"context_line":"            self.assertEqual(30, timeout)"},{"line_number":417,"context_line":""},{"line_number":418,"context_line":"        mock_wait.side_effect \u003d fake_wait_for_all"},{"line_number":419,"context_line":""},{"line_number":420,"context_line":"        results \u003d context.scatter_gather_cells("},{"line_number":421,"context_line":"            ctxt, mappings, 30, objects.InstanceList.get_by_filters)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fce034c_5bc29a87","line":418,"range":{"start_line":411,"start_character":7,"end_line":418,"end_character":49},"updated":"2019-04-12 00:01:18.000000000","message":"this is the same as above just with a different timeout value.\n\ncan we extract this out into something reusable?","commit_id":"f08979fe5ee4fbecbc463371ab4252a566fc5fe6"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"24c8cc2958f5e71d7dc151b0c1c5bcd606eb9236","unresolved":false,"context_lines":[{"line_number":448,"context_line":"        mock_get_inst.side_effect \u003d [mock.sentinel.instances,"},{"line_number":449,"context_line":"                                     test.TestingException()]"},{"line_number":450,"context_line":""},{"line_number":451,"context_line":"        # Assert the wait call in a side_effect because the call_args will"},{"line_number":452,"context_line":"        # change by the time we normally assert at the end."},{"line_number":453,"context_line":"        def fake_wait_for_all(fs, timeout\u003dNone):"},{"line_number":454,"context_line":"            for f in fs:"},{"line_number":455,"context_line":"                self.assertIsInstance(f, futurist.Future)"},{"line_number":456,"context_line":"            self.assertEqual(30, timeout)"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"        mock_wait.side_effect \u003d fake_wait_for_all"},{"line_number":459,"context_line":""},{"line_number":460,"context_line":"        results \u003d context.scatter_gather_cells("},{"line_number":461,"context_line":"            ctxt, mappings, 30, objects.InstanceList.get_by_filters)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fce034c_5b47fa1e","line":458,"range":{"start_line":451,"start_character":8,"end_line":458,"end_character":49},"updated":"2019-04-12 00:01:18.000000000","message":"again this is the same as above","commit_id":"f08979fe5ee4fbecbc463371ab4252a566fc5fe6"}],"nova/tests/unit/test_fixtures.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"24c8cc2958f5e71d7dc151b0c1c5bcd606eb9236","unresolved":false,"context_lines":[{"line_number":432,"context_line":"            fixtures.SynchronousThreadPoolExecutorFixture("},{"line_number":433,"context_line":"                done_return_values\u003d[True, False]))"},{"line_number":434,"context_line":"        tester \u003d mock.MagicMock()"},{"line_number":435,"context_line":"        executor \u003d futurist.GreenThreadPoolExecutor()"},{"line_number":436,"context_line":"        future1 \u003d executor.submit(tester.function, \u0027foo\u0027, bar\u003d\u0027bar\u0027)"},{"line_number":437,"context_line":"        future2 \u003d executor.submit(tester.function, \u0027foo\u0027, bar\u003d\u0027bar\u0027)"},{"line_number":438,"context_line":"        self.assertTrue(future1.done())"}],"source_content_type":"text/x-python","patch_set":6,"id":"3fce034c_9ba9e2e0","line":435,"range":{"start_line":435,"start_character":7,"end_line":435,"end_character":53},"updated":"2019-04-12 00:01:18.000000000","message":"ok so this gets patched by the fixture above\n\nhttps://review.openstack.org/#/c/650172/6/nova/tests/fixtures.py@1130\n\ni guess this makes sense.","commit_id":"f08979fe5ee4fbecbc463371ab4252a566fc5fe6"}]}