Skip to content

Commit d95ac9c

Browse files
committed
MOD-14486: Don't split space-separated moduleArgs into key-value pairs
The key-value pair splitting introduced in PR #243 changes how args are passed to Redis loadmodule, breaking modules that expect the original single-string format. This caused silent Redis server crashes in CI multiprocessing workers. Revert to pre-v0.7.22 behavior: when no semicolons are present, keep the entire string as a single arg. The case-insensitive arg matching from PR #243 is preserved.
1 parent 0f50113 commit d95ac9c

2 files changed

Lines changed: 28 additions & 28 deletions

File tree

RLTest/utils.py

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,8 @@ def fix_modulesArgs(modules, modulesArgs, defaultArgs=None, haveSeqs=True):
108108
# arg string is a string of words separated by whitespace.
109109
# arg string can be separated by semicolons into (logical) arg lists.
110110
# semicolons can be escaped with a backslash.
111-
# if no semicolons are present, the string is treated as space-separated key-value pairs,
112-
# where each consecutive pair of words forms a 'KEY VALUE' arg.
113-
# thus, 'K1 V1 K2 V2' becomes ['K1 V1', 'K2 V2']
114-
# an odd number of words without semicolons is an error.
111+
# if no semicolons are present, the entire string is kept as a single arg.
112+
# thus, 'K1 V1 K2 V2' becomes ['K1 V1 K2 V2']
115113
# for args with multiple values, semicolons are required:
116114
# thus, 'K1 V1; K2 V2 V3' becomes ['K1 V1', 'K2 V2 V3']
117115
# arg list is a list of arg strings.
@@ -121,15 +119,6 @@ def fix_modulesArgs(modules, modulesArgs, defaultArgs=None, haveSeqs=True):
121119
# case # 'args ...': arg string for a single module
122120
# transformed into [['arg', ...]]
123121
parts = split_by_semicolon(modulesArgs)
124-
if len(parts) == 1:
125-
# No semicolons found - treat as space-separated key-value pairs
126-
words = parts[0].split()
127-
if len(words) % 2 != 0:
128-
print(Colors.Bred(f"Error in args: odd number of words in key-value pairs: '{modulesArgs}'. "
129-
f"Use semicolons to separate args with multiple values (e.g. 'KEY1 V1; KEY2 V2 V3')."))
130-
sys.exit(1)
131-
if len(words) > 2:
132-
parts = [f"{words[i]} {words[i + 1]}" for i in range(0, len(words), 2)]
133122
modulesArgs = [parts]
134123
elif type(modulesArgs) == list:
135124
args = []

tests/unit/test_utils.py

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,33 +10,35 @@ def test_single_key_value_pair(self):
1010
result = fix_modulesArgs(['/mod.so'], 'WORKERS 4')
1111
self.assertEqual(result, [['WORKERS 4']])
1212

13-
# 2. Multiple key-value pairs without semicolons (new behavior)
13+
# 2. Multiple key-value pairs without semicolons - kept as single string
1414
def test_multiple_kv_pairs_no_semicolons(self):
1515
result = fix_modulesArgs(['/mod.so'], '_FREE_RESOURCE_ON_THREAD FALSE TIMEOUT 80 WORKERS 4')
16-
self.assertEqual(result, [['_FREE_RESOURCE_ON_THREAD FALSE', 'TIMEOUT 80', 'WORKERS 4']])
16+
self.assertEqual(result, [['_FREE_RESOURCE_ON_THREAD FALSE TIMEOUT 80 WORKERS 4']])
1717

1818
# 3. Semicolon-separated args (existing behavior)
1919
def test_semicolon_separated_args(self):
2020
result = fix_modulesArgs(['/mod.so'], 'KEY1 V1; KEY2 V2')
2121
self.assertEqual(result, [['KEY1 V1', 'KEY2 V2']])
2222

23-
# 4a. Odd number of words without semicolons - should error
24-
def test_odd_words_no_semicolons_exits(self):
25-
with self.assertRaises(SystemExit):
26-
fix_modulesArgs(['/mod.so'], 'FLAG TIMEOUT 80')
23+
# 4a. Odd number of words without semicolons - kept as single string (no error)
24+
def test_odd_words_no_semicolons_kept_as_single(self):
25+
result = fix_modulesArgs(['/mod.so'], 'FLAG TIMEOUT 80')
26+
self.assertEqual(result, [['FLAG TIMEOUT 80']])
2727

2828
# 4b. Odd number of words with semicolons - valid, semicolons split first
2929
def test_odd_words_with_semicolons_valid(self):
3030
result = fix_modulesArgs(['/mod.so'], 'FLAG; TIMEOUT 80')
3131
self.assertEqual(result, [['FLAG', 'TIMEOUT 80']])
3232

33-
# 5a. Space-separated string overrides matching defaults, non-matching defaults added
33+
# 5a. Space-separated string kept as single arg, non-matching defaults added
3434
def test_space_separated_overrides_defaults(self):
3535
defaults = [['WORKERS 8', 'TIMEOUT 60', 'EXTRA 1']]
3636
result = fix_modulesArgs(['/mod.so'], 'WORKERS 4 TIMEOUT 80', defaults)
37+
# Without semicolons, the string is kept as one arg (first word = WORKERS).
38+
# Only 'WORKERS 8' default is overridden; 'TIMEOUT 60' and 'EXTRA 1' remain.
3739
result_dict = {arg.split(' ')[0]: arg for arg in result[0]}
38-
self.assertEqual(result_dict['WORKERS'], 'WORKERS 4')
39-
self.assertEqual(result_dict['TIMEOUT'], 'TIMEOUT 80')
40+
self.assertEqual(result_dict['WORKERS'], 'WORKERS 4 TIMEOUT 80')
41+
self.assertEqual(result_dict['TIMEOUT'], 'TIMEOUT 60')
4042
self.assertEqual(result_dict['EXTRA'], 'EXTRA 1')
4143

4244
# 5b. Semicolon-separated string overrides matching defaults
@@ -48,13 +50,14 @@ def test_semicolon_separated_overrides_defaults(self):
4850
self.assertEqual(result_dict['TIMEOUT'], 'TIMEOUT 80')
4951
self.assertEqual(result_dict['EXTRA'], 'EXTRA 1')
5052

51-
# 5c. Space-separated explicit overrides some defaults, non-overlapping defaults are merged
53+
# 5c. Space-separated explicit kept as single arg, non-overlapping defaults are merged
5254
def test_space_separated_partial_override_with_defaults(self):
5355
defaults = [['_FREE_RESOURCE_ON_THREAD TRUE', 'TIMEOUT 100', 'WORKERS 8']]
5456
result = fix_modulesArgs(['/mod.so'], 'WORKERS 4 TIMEOUT 80', defaults)
57+
# Single arg 'WORKERS 4 TIMEOUT 80' overrides 'WORKERS 8' default only
5558
result_dict = {arg.split(' ')[0]: arg for arg in result[0]}
56-
self.assertEqual(result_dict['WORKERS'], 'WORKERS 4')
57-
self.assertEqual(result_dict['TIMEOUT'], 'TIMEOUT 80')
59+
self.assertEqual(result_dict['WORKERS'], 'WORKERS 4 TIMEOUT 80')
60+
self.assertEqual(result_dict['TIMEOUT'], 'TIMEOUT 100')
5861
self.assertEqual(result_dict['_FREE_RESOURCE_ON_THREAD'], '_FREE_RESOURCE_ON_THREAD TRUE')
5962

6063
# 6. None input with defaults - deep copy of defaults
@@ -93,11 +96,11 @@ def test_multi_module_with_defaults(self):
9396
self.assertEqual(dict2['K4'], 'K4 d4')
9497

9598

96-
# 9. Case-insensitive matching between explicit args and defaults (both directions)
99+
# 9. Case-insensitive matching between explicit args and defaults (semicolons)
97100
def test_case_insensitive_override(self):
98-
# Uppercase explicit overrides lowercase defaults
101+
# Uppercase explicit overrides lowercase defaults (using semicolons for multiple args)
99102
defaults = [['workers 8', 'timeout 60', 'EXTRA 1', 'MIxEd 7', 'lower true']]
100-
result = fix_modulesArgs(['/mod.so'], 'WORKERS 4 TIMEOUT 80 miXed 0 LOWER false', defaults)
103+
result = fix_modulesArgs(['/mod.so'], 'WORKERS 4; TIMEOUT 80; miXed 0; LOWER false', defaults)
101104
result_dict = {arg.split(' ')[0]: arg for arg in result[0]}
102105
self.assertEqual(result_dict['WORKERS'], 'WORKERS 4')
103106
self.assertEqual(result_dict['TIMEOUT'], 'TIMEOUT 80')
@@ -108,3 +111,11 @@ def test_case_insensitive_override(self):
108111
self.assertNotIn('timeout', result_dict)
109112
self.assertNotIn('MIxEd', result_dict)
110113
self.assertNotIn('lower', result_dict)
114+
115+
# 10. Regression test: space-separated moduleArgs without semicolons should NOT
116+
# be split into key-value pairs. They should be kept as a single arg string,
117+
# matching the pre-v0.7.22 behavior.
118+
def test_space_separated_args_not_split_into_pairs(self):
119+
result = fix_modulesArgs(['module.so'], 'DEFAULT_DIALECT 2 WORKERS 4 _FREE_RESOURCE_ON_THREAD FALSE')
120+
# Should be kept as ONE arg string, not split into pairs
121+
self.assertEqual(result, [['DEFAULT_DIALECT 2 WORKERS 4 _FREE_RESOURCE_ON_THREAD FALSE']])

0 commit comments

Comments
 (0)