-
-
Notifications
You must be signed in to change notification settings - Fork 204
Add support for executor configuration in AioConfig for file loading #1451
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
base: master
Are you sure you want to change the base?
Changes from all commits
0d9e208
c2d5b04
1b1bf4d
2ceb10a
5453a34
7357297
1d0b139
89e0db6
8a69233
72818b0
5435d4b
7e52111
918fb2e
845d2c4
ab7af01
104ab9b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| from aiobotocore.httpsession import AIOHTTPSession | ||
|
|
||
| DEFAULT_HTTP_SESSION_CLS = AIOHTTPSession |
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about remaining blocking file I/O? Is the scope of this PR limited to
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are the main pain points to my understanding, we can always expand upon this later |
jakob-keller marked this conversation as resolved.
Show resolved
Hide resolved
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
| import copy | ||
| import ssl | ||
| import sys | ||
| from concurrent.futures import ThreadPoolExecutor | ||
| from typing import Optional, TypedDict, Union | ||
|
|
||
| import botocore.client | ||
|
|
@@ -10,7 +11,7 @@ | |
| from botocore.exceptions import ParamValidationError | ||
|
|
||
| from ._constants import DEFAULT_KEEPALIVE_TIMEOUT | ||
| from .endpoint import DEFAULT_HTTP_SESSION_CLS | ||
| from ._types import DEFAULT_HTTP_SESSION_CLS | ||
| from .httpsession import AIOHTTPSession | ||
| from .httpxsession import HttpxSession | ||
|
|
||
|
|
@@ -38,23 +39,34 @@ class _ConnectorArgs(TypedDict): | |
|
|
||
| _HttpSessionType = Union[AIOHTTPSession, HttpxSession] | ||
|
|
||
| PARAM_SENTINAL = object() | ||
|
|
||
|
|
||
| class AioConfig(botocore.client.Config): | ||
| def __init__( | ||
| self, | ||
| connector_args: Optional[_ConnectorArgs] = None, | ||
| http_session_cls: type[_HttpSessionType] = DEFAULT_HTTP_SESSION_CLS, | ||
| load_executor: Optional[ThreadPoolExecutor] = PARAM_SENTINAL, | ||
| **kwargs, | ||
| ): | ||
| super().__init__(**kwargs) | ||
|
|
||
| self._validate_connector_args(connector_args, http_session_cls) | ||
jakob-keller marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| if load_executor not in (None, PARAM_SENTINAL) and not isinstance( | ||
| load_executor, ThreadPoolExecutor | ||
| ): | ||
| raise ParamValidationError( | ||
| report='load_executor value must be an instance of an Executor.' | ||
| ) | ||
|
|
||
| self.load_executor = load_executor | ||
|
|
||
| self.connector_args: _ConnectorArgs = ( | ||
| copy.copy(connector_args) if connector_args else {} | ||
| ) | ||
| self.http_session_cls: type[_HttpSessionType] = http_session_cls | ||
| self._validate_connector_args( | ||
jakob-keller marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| self.connector_args, self.http_session_cls | ||
| ) | ||
|
|
||
| if 'keepalive_timeout' not in self.connector_args: | ||
| self.connector_args['keepalive_timeout'] = ( | ||
|
|
@@ -72,6 +84,9 @@ def _validate_connector_args( | |
| connector_args: _ConnectorArgs, | ||
| http_session_cls: type[_HttpSessionType], | ||
| ) -> None: | ||
| if connector_args is None: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are changes from #1454 reverted on purpose? Why?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. validate before assignment
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| return | ||
|
|
||
| for k, v in connector_args.items(): | ||
| # verify_ssl is handled by verify parameter to create_client | ||
| if k == 'use_dns_cache': | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
run_in_executor() accepts instances of concurrent.futures.Executor and uses the default executor if
executorisNone. It might reduce friction, if we follow the standard library's approach.Then again, the function would be reduced to line 29 and could be dropped entirely.