Skip to content

Commit

Permalink
Add command-line argument --timeout, to configure network timeout
Browse files Browse the repository at this point in the history
The default connect timeout is five seconds now. The default read
timeout is "infinite".
  • Loading branch information
amotl committed Jan 29, 2024
1 parent a70f8a7 commit d0dfab2
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 1 deletion.
5 changes: 4 additions & 1 deletion CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ Changes for crash

Unreleased
==========

- Added command-line argument ``--timeout``, to configure network timeout
values in seconds. The default connect timeout is five seconds now,
the default read timeout is the default setting of the ``socket`` module,
which is "infinite" by default.

2024/01/12 0.30.2
=================
Expand Down
57 changes: 57 additions & 0 deletions crate/crash/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ def _conf_or_default(key, value):
parser.add_argument('--hosts', type=str, nargs='*',
default=_conf_or_default('hosts', ['localhost:4200']),
help='connect to HOSTS.', metavar='HOSTS')
parser.add_argument('--timeout', type=str, metavar='TIMEOUT',
help='Configure network timeout in "<connect_sec>" or "<connect_sec>,<read_sec>" format. '
'Defaults to "5,-1" where "-1" means infinite timeout.', default="5")
parser.add_argument(
'--verify-ssl',
choices=(True, False),
Expand Down Expand Up @@ -618,8 +621,62 @@ def save_and_exit():
save_and_exit()


INFINITE_TIMEOUT = -1
INFINITE_TIMEOUTS = [None, INFINITE_TIMEOUT, str(INFINITE_TIMEOUT)]


def _decode_timeout(value):
"""
Decode single timeout value, respecting the `INFINITE_TIMEOUT` surrogate.
"""
if value in INFINITE_TIMEOUTS:
return None
else:
return float(value)


def _decode_timeouts(timeout):
"""
Decode connect/read timeout values from tuple or string.
Variant 1: (connect, read)
Variant 2: connect,read
"""

if timeout is None:
timeouts = (INFINITE_TIMEOUT, INFINITE_TIMEOUT)
elif isinstance(timeout, (int, float)):
timeouts = (timeout, INFINITE_TIMEOUT)
elif isinstance(timeout, str):
timeouts = timeout.split(",")
elif isinstance(timeout, (list, tuple)):
timeouts = timeout
else:
raise TypeError(f"Cannot decode timeout value from type `{type(timeout)}`, "
f"expected format `<connect_sec>,<read_sec>`")

if len(timeouts) == 1:
connect_timeout, read_timeout = timeouts[0], INFINITE_TIMEOUT
elif len(timeouts) == 2:
connect_timeout, read_timeout = timeouts[0], timeouts[1]
else:
raise ValueError(f"Cannot decode timeout `{timeout}`, "
f"expected format `<connect_sec>,<read_sec>`")

return urllib3.Timeout(connect=_decode_timeout(connect_timeout), read=_decode_timeout(read_timeout))


def _create_shell(crate_hosts, error_trace, output_writer, is_tty, args,
timeout=None, password=None):

# Explicit "timeout" function argument takes precedence.
if timeout is not None:
timeout = _decode_timeouts(timeout)

# Probe `--timeout`` command line argument second.
elif args.timeout is not None:
timeout = _decode_timeouts(args.timeout)

return CrateShell(crate_hosts,
error_trace=error_trace,
output_writer=output_writer,
Expand Down
7 changes: 7 additions & 0 deletions docs/run.rst
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ The ``crash`` executable supports multiple command-line options:
| | command will succeed if at least one |
| | connection is successful. |
+-------------------------------+----------------------------------------------+
| ``--timeout <TIMEOUT>`` | Configure network timeout in "<connect_sec>" |
| | or "<connect_sec>,<read_sec>" format. |
| | |
| | The default value is "5,-1", configuring a |
| | connect timeout of five seconds with |
| | infinite read timeout. |
+-------------------------------+----------------------------------------------+
| ``--history <FILENAME>`` | Use ``<FILENAME>`` as a history file. |
| | |
| | Defaults to the ``crash_history`` file in |
Expand Down
31 changes: 31 additions & 0 deletions tests/test_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

from crate.crash.command import (
Result,
_decode_timeout,
_decode_timeouts,
get_information_schema_query,
host_and_port,
stmt_type,
Expand Down Expand Up @@ -112,6 +114,35 @@ def test_stmt_type(self):
self.assertEqual(stmt_type(' SELECT 1 ;'), 'SELECT')
self.assertEqual(stmt_type('\nSELECT\n1\n;\n'), 'SELECT')

def test_decode_timeout_success(self):
self.assertEqual(_decode_timeout(None), None)
self.assertEqual(_decode_timeout(-1), None)
self.assertEqual(_decode_timeout(42.42), 42.42)
self.assertEqual(_decode_timeout("42.42"), 42.42)

def test_decode_timeouts_success(self):
# `_decode_timeouts` returns an urllib3.Timeout instance.
self.assertEqual(str(_decode_timeouts(None)), 'Timeout(connect=None, read=None, total=None)')
self.assertEqual(str(_decode_timeouts(-1)), 'Timeout(connect=None, read=None, total=None)')
self.assertEqual(str(_decode_timeouts("-1")), 'Timeout(connect=None, read=None, total=None)')
self.assertEqual(str(_decode_timeouts(42.42)), 'Timeout(connect=42.42, read=None, total=None)')
self.assertEqual(str(_decode_timeouts("42.42")), 'Timeout(connect=42.42, read=None, total=None)')
self.assertEqual(str(_decode_timeouts((42.42, 84.84))), 'Timeout(connect=42.42, read=84.84, total=None)')
self.assertEqual(str(_decode_timeouts('42.42, 84.84')), 'Timeout(connect=42.42, read=84.84, total=None)')
self.assertEqual(str(_decode_timeouts((-1, 42.42))), 'Timeout(connect=None, read=42.42, total=None)')
self.assertEqual(str(_decode_timeouts("-1, 42.42")), 'Timeout(connect=None, read=42.42, total=None)')

def test_decode_timeouts_failure(self):
with self.assertRaises(TypeError) as ecm:
_decode_timeouts({})
self.assertEqual(str(ecm.exception), "Cannot decode timeout value from type `<class 'dict'>`, "
"expected format `<connect_sec>,<read_sec>`")

with self.assertRaises(ValueError) as ecm:
_decode_timeouts([])
self.assertEqual(str(ecm.exception), "Cannot decode timeout `[]`, "
"expected format `<connect_sec>,<read_sec>`")


class TestGetInformationSchemaQuery(TestCase):

Expand Down

0 comments on commit d0dfab2

Please sign in to comment.