-
Notifications
You must be signed in to change notification settings - Fork 1.1k
User Agent 2.0 #2955
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
User Agent 2.0 #2955
Changes from 5 commits
e1a165f
f9d6a96
aa03f04
87dfca1
a04c941
34edd4b
bda23a9
e19e237
66ad86a
f08429b
eb7ded6
b0a29f8
d33a106
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 |
|---|---|---|
|
|
@@ -55,6 +55,9 @@ | |
| 'us-west-1', | ||
| 'us-west-2', | ||
| ] | ||
| # Maximum allowed length of the ``user_agent_appid`` config field. Longer | ||
| # values get truncated and result in a warning-level log message. | ||
| USERAGENT_APPID_MAXLEN = 50 | ||
|
|
||
|
|
||
| class ClientArgsCreator: | ||
|
|
@@ -66,13 +69,14 @@ def __init__( | |
| loader, | ||
| exceptions_factory, | ||
| config_store, | ||
| user_agent_creator, | ||
| ): | ||
| self._event_emitter = event_emitter | ||
| self._user_agent = user_agent | ||
| self._response_parser_factory = response_parser_factory | ||
| self._loader = loader | ||
| self._exceptions_factory = exceptions_factory | ||
| self._config_store = config_store | ||
| self._session_ua_creator = user_agent_creator | ||
|
|
||
| def get_client_args( | ||
| self, | ||
|
|
@@ -159,6 +163,11 @@ def get_client_args( | |
| event_emitter, | ||
| ) | ||
|
|
||
| # Copy the session's user agent factory and adds client configuration. | ||
| client_ua_creator = self._session_ua_creator.with_client_config( | ||
| new_config | ||
| ) | ||
|
|
||
| return { | ||
| 'serializer': serializer, | ||
| 'endpoint': endpoint, | ||
|
|
@@ -171,6 +180,7 @@ def get_client_args( | |
| 'partition': partition, | ||
| 'exceptions_factory': self._exceptions_factory, | ||
| 'endpoint_ruleset_resolver': ruleset_resolver, | ||
| 'user_agent_creator': client_ua_creator, | ||
| } | ||
|
|
||
| def compute_client_args( | ||
|
|
@@ -193,14 +203,6 @@ def compute_client_args( | |
| if raw_value is not None: | ||
| parameter_validation = ensure_boolean(raw_value) | ||
|
|
||
| # Override the user agent if specified in the client config. | ||
| user_agent = self._user_agent | ||
| if client_config is not None: | ||
| if client_config.user_agent is not None: | ||
| user_agent = client_config.user_agent | ||
| if client_config.user_agent_extra is not None: | ||
| user_agent += ' %s' % client_config.user_agent_extra | ||
|
|
||
| s3_config = self.compute_s3_config(client_config) | ||
| endpoint_config = self._compute_endpoint_config( | ||
| service_name=service_name, | ||
|
|
@@ -217,7 +219,6 @@ def compute_client_args( | |
| config_kwargs = dict( | ||
| region_name=endpoint_config['region_name'], | ||
| signature_version=endpoint_config['signature_version'], | ||
| user_agent=user_agent, | ||
| ) | ||
| if 'dualstack' in endpoint_variant_tags: | ||
| config_kwargs.update(use_dualstack_endpoint=True) | ||
|
|
@@ -234,9 +235,13 @@ def compute_client_args( | |
| client_cert=client_config.client_cert, | ||
| inject_host_prefix=client_config.inject_host_prefix, | ||
| tcp_keepalive=client_config.tcp_keepalive, | ||
| user_agent=client_config.user_agent, | ||
| user_agent_extra=client_config.user_agent_extra, | ||
| user_agent_appid=client_config.user_agent_appid, | ||
| ) | ||
| self._compute_retry_config(config_kwargs) | ||
| self._compute_connect_timeout(config_kwargs) | ||
| self._compute_user_agent_appid_config(config_kwargs) | ||
| s3_config = self.compute_s3_config(client_config) | ||
|
|
||
| is_s3_service = self._is_s3_service(service_name) | ||
|
|
@@ -249,7 +254,6 @@ def compute_client_args( | |
| return { | ||
| 'service_name': service_name, | ||
| 'parameter_validation': parameter_validation, | ||
| 'user_agent': user_agent, | ||
|
Contributor
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. This may also be problematic, I don't immediately know how we do this cleanly though.
Contributor
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. We can bring back this line together with lines 196-203 above where its value gets computed using the legacy method. There is no harm in returning the
Contributor
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. Looking at other libraries that may have issues with this, I can't find one that relies on user_agent. I agree this may cause unnecessary confusion, so let's opt to remove it. |
||
| 'endpoint_config': endpoint_config, | ||
| 'protocol': protocol, | ||
| 'config_kwargs': config_kwargs, | ||
|
|
@@ -646,3 +650,19 @@ def compute_endpoint_resolver_builtin_defaults( | |
| ), | ||
| EPRBuiltins.SDK_ENDPOINT: given_endpoint, | ||
| } | ||
|
|
||
| def _compute_user_agent_appid_config(self, config_kwargs): | ||
| user_agent_appid = config_kwargs.get('user_agent_appid') | ||
| if user_agent_appid is None: | ||
| user_agent_appid = self._config_store.get_config_variable( | ||
| 'user_agent_appid' | ||
| ) | ||
| if ( | ||
| user_agent_appid is not None | ||
| and len(user_agent_appid) > USERAGENT_APPID_MAXLEN | ||
| ): | ||
| logger.warning( | ||
| 'The configured value for user_agent_appid exceeds the ' | ||
| f'maximum length of {USERAGENT_APPID_MAXLEN} characters.' | ||
| ) | ||
| config_kwargs['user_agent_appid'] = user_agent_appid | ||
nateprewitt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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.
I think this needs to be optional or we may end up breaking current callers. That may change how we use the attribute below.
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.
bda23a9 is a refactor that prioritizes backwards compatible interfaces over everything (especially over avoiding duplication).