Description of changes:
This PR removes the redundant creation of botocore configs (which are just duplicates of the config used in get_boto_session) found in get_sagemaker_session and get_bedrock_runtime_client.
Since we set the default client config used by the botocore session underlying the boto3 session that is used to create all of our clients (sagemaker, bedrock, etc), we don't need to pass the config parameter to .client().
Note that it is very awkward to validate that the correct botocore config (i.e. our adaptive retry config) gets used when we create the sagemaker and bedrock clients, so I have not added a new unit test that checks this. Here is the logic flow for how the default botocore config ends up getting used:
boto3.session.Session.client is called with config=None (since we don't explicitly pass a config)
self._session.create_client is called with config=None where self refers to a boto3.session.Session instance
self._session is the botocore session that manually create in get_boto_session. Thus, self._session.create_client calls the create_client method in botocore.session.Session.
The following code from botocore.session.Session.create_client is of relevance:
default_client_config = self.get_default_client_config() # this returns the adaptive retry config that we set as default
...
elif default_client_config is not None:
config = default_client_config # this `config` variable is used when we create the client
and
client = client_creator.create_client(
service_name=service_name,
region_name=region_name,
is_secure=use_ssl,
endpoint_url=endpoint_url,
verify=verify,
credentials=credentials,
scoped_config=self.get_scoped_config(),
client_config=config, # OUR CONFIG GETS USED HERE
api_version=api_version,
auth_token=auth_token,
)
If I were to mock things, I would want to mock the instance method create_client. But since create_client is called with a whole bunch of different args which aren't themselves easily mockable, there's no clean way to just check that the config in client_config=config is our adaptive retry config.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Description of changes: This PR removes the redundant creation of botocore configs (which are just duplicates of the config used in
get_boto_session) found inget_sagemaker_sessionandget_bedrock_runtime_client.Since we set the default client config used by the botocore session underlying the boto3 session that is used to create all of our clients (sagemaker, bedrock, etc), we don't need to pass the
configparameter to.client().Note that it is very awkward to validate that the correct botocore config (i.e. our adaptive retry config) gets used when we create the sagemaker and bedrock clients, so I have not added a new unit test that checks this. Here is the logic flow for how the default botocore config ends up getting used:
boto3.session.Session.clientis called withconfig=None(since we don't explicitly pass a config)self._session.create_clientis called withconfig=Nonewhereselfrefers to aboto3.session.Sessioninstanceself._sessionis the botocore session that manually create inget_boto_session. Thus,self._session.create_clientcalls thecreate_clientmethod inbotocore.session.Session.botocore.session.Session.create_clientis of relevance:and
If I were to mock things, I would want to mock the instance method
create_client. But sincecreate_clientis called with a whole bunch of different args which aren't themselves easily mockable, there's no clean way to just check that theconfiginclient_config=configis our adaptive retry config.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.