-
Notifications
You must be signed in to change notification settings - Fork 107
Add PYTHON_THREADPOOL_THREAD_COUNT app setting #744
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -15,6 +15,7 @@ | |||
|
|||
# Feature Flags (app settings) | |||
PYTHON_ROLLBACK_CWD_PATH = "PYTHON_ROLLBACK_CWD_PATH" | |||
PYTHON_THREADPOOL_THREAD_COUNT = "PYTHON_THREADPOOL_THREAD_COUNT" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stefanushinardi - Could you please check if you want to change the name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - putting my reasoning here for future reference: it makes sense to put the PYTHON_* prefix for this as without it users might get confused and use this config for other languages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
some nits.
Description
This is a dependency of enabling async thread pool. Getting this change into the codebase will allow us configure the threadpool more easily when benchmarking the async threadpool performance.
Fixes #675
Now we allow customer to configure number of thread in the synchronous threadpool by setting PYTHON_THREADPOOL_THREAD_COUNT application setting. If the application settings is not set, we will default it to 1.
I'm wondering whether we should call this PYTHON_SYNC_THREADPOOL_THREAD_COUNT.
If we introduce async thread pool later on, should we introduce a new PYTHON_ASYNC_THREADPOOL_THREAD_COUNT app settings? It is possible that if a function app mixes both sync and async functions, the actual number of threads will be 2x. Adding this new setting will has a better the transparency for customer to control their function app, but will make the app setting more complicate.
In my opinion, both async threadpool and sync threadpool should use the value in PYTHON_THREADPOOL_THREAD_COUNT.
PR information
Quality of Code and Contribution Guidelines