Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/next-release/enhancement-config-81168.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"type": "enhancement",
"category": "config",
"description": "Add support for TCP Keep-Alive configuration via BOTOCORE_TCP_KEEPALIVE environment variable"
}
16 changes: 7 additions & 9 deletions botocore/args.py
Original file line number Diff line number Diff line change
Expand Up @@ -348,9 +348,7 @@ def compute_client_args(
'protocol': protocol,
'config_kwargs': config_kwargs,
's3_config': s3_config,
'socket_options': self._compute_socket_options(
scoped_config, client_config
),
'socket_options': self._compute_socket_options(client_config),
}

def _compute_inject_host_prefix(self, client_config, config_kwargs):
Expand Down Expand Up @@ -553,16 +551,16 @@ def _resolve_endpoint(
service_name, region_name, endpoint_url, is_secure
)

def _compute_socket_options(self, scoped_config, client_config=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is a private API so it shouldn't be used by anyone, but are we confident this will not break any customers?

I did a quick search and it doesn't look like we use this elsewhere in our repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per checking our code, the _compute_socket_options is only used in return statement of the function get_client_args(). Before, the scoped_keepalive checks the config file. Now it is taking care of tcp_keepalive as it also checks the config file. I suppose this should not break as the functionality of scoped_keepalive is now covered by tcp_keepalive.

def _compute_socket_options(self, client_config=None):
# This disables Nagle's algorithm and is the default socket options
# in urllib3.

# looks at Config object, config file and Environment variable
socket_options = [(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1)]
client_keepalive = client_config and client_config.tcp_keepalive
scoped_keepalive = scoped_config and self._ensure_boolean(
scoped_config.get("tcp_keepalive", False)
)
# Enables TCP Keepalive if specified in client config object or shared config file.
if client_keepalive or scoped_keepalive:
tcp_keepalive = self._config_store.get_config_variable('tcp_keepalive')

if client_keepalive or tcp_keepalive:
socket_options.append((socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1))
return socket_options

Expand Down
6 changes: 6 additions & 0 deletions botocore/configprovider.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,12 @@
None,
None,
),
'tcp_keepalive': (
'tcp_keepalive',
'BOTOCORE_TCP_KEEPALIVE',
None,
utils.ensure_boolean,
),
}
# A mapping for the s3 specific configuration vars. These are the configuration
# vars that typically go in the s3 section of the config file. This mapping
Expand Down
35 changes: 22 additions & 13 deletions tests/unit/test_args.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ def call_get_client_args(self, **override_kwargs):
'partition_data': {},
}
call_kwargs.update(**override_kwargs)

return self.args_create.get_client_args(**call_kwargs)

def call_compute_client_args(self, **override_kwargs):
Expand Down Expand Up @@ -240,10 +241,10 @@ def test_region_does_not_resolve_if_not_s3_and_endpoint_url_provided(self):
)
self.assertEqual(client_args['client_config'].region_name, None)

def test_tcp_keepalive_enabled_scoped_config(self):
scoped_config = {'tcp_keepalive': 'true'}
def test_tcp_keepalive_enabled_config_store(self):
self.config_store.set_config_variable('tcp_keepalive', True)
with mock.patch('botocore.args.EndpointCreator') as m:
self.call_get_client_args(scoped_config=scoped_config)
self.call_get_client_args()
self.assert_create_endpoint_call(
m,
socket_options=self.default_socket_options
Expand All @@ -252,30 +253,28 @@ def test_tcp_keepalive_enabled_scoped_config(self):

def test_tcp_keepalive_not_specified(self):
with mock.patch('botocore.args.EndpointCreator') as m:
self.call_get_client_args(scoped_config={}, client_config=None)
self.call_get_client_args(client_config=None)
self.assert_create_endpoint_call(
m, socket_options=self.default_socket_options
)
self.call_get_client_args(
scoped_config=None, client_config=Config()
)
self.call_get_client_args(client_config=Config())
self.assert_create_endpoint_call(
m, socket_options=self.default_socket_options
)

def test_tcp_keepalive_enabled_if_set_anywhere(self):
with mock.patch('botocore.args.EndpointCreator') as m:
self.config_store.set_config_variable('tcp_keepalive', True)
self.call_get_client_args(
scoped_config={'tcp_keepalive': 'true'},
client_config=Config(tcp_keepalive=False),
)
self.assert_create_endpoint_call(
m,
socket_options=self.default_socket_options
+ [(socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1)],
)
self.config_store.set_config_variable('tcp_keepalive', False)
self.call_get_client_args(
scoped_config={'tcp_keepalive': 'false'},
client_config=Config(tcp_keepalive=True),
)
self.assert_create_endpoint_call(
Expand All @@ -284,18 +283,28 @@ def test_tcp_keepalive_enabled_if_set_anywhere(self):
+ [(socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1)],
)

def test_tcp_keepalive_enabled_environment_variable(self):
with mock.patch.dict('os.environ', {'BOTOCORE_TCP_KEEPALIVE': 'true'}):
with mock.patch('botocore.args.EndpointCreator') as m:
self.call_get_client_args()
self.assert_create_endpoint_call(
m,
socket_options=self.default_socket_options
+ [(socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1)],
)

def test_tcp_keepalive_explicitly_disabled(self):
scoped_config = {'tcp_keepalive': 'false'}
self.config_store.set_config_variable('tcp_keepalive', False)
with mock.patch('botocore.args.EndpointCreator') as m:
self.call_get_client_args(scoped_config=scoped_config)
self.call_get_client_args()
self.assert_create_endpoint_call(
m, socket_options=self.default_socket_options
)

def test_tcp_keepalive_enabled_case_insensitive(self):
scoped_config = {'tcp_keepalive': 'True'}
self.config_store.set_config_variable('tcp_keepalive', True)
with mock.patch('botocore.args.EndpointCreator') as m:
self.call_get_client_args(scoped_config=scoped_config)
self.call_get_client_args()
self.assert_create_endpoint_call(
m,
socket_options=self.default_socket_options
Expand Down