jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/367823 )
Change subject: Transports: improve Command class
......................................................................
Transports: improve Command class
* Allow to set the ok_codes property of the transports.Command class to
an empty list to consider any return code as successful.
The case in which no return code should be treated successful has no
practical use.
* Improve command list validation of the transports.Command class to not
allow an empty list for the commands property.
Bug: T171679
Change-Id: I918f7c133b0ab0a7944d7dd316a47f8a0ae2d829
---
M cumin/tests/unit/transports/test_clustershell.py
M cumin/tests/unit/transports/test_init.py
M cumin/transports/__init__.py
M cumin/transports/clustershell.py
4 files changed, 27 insertions(+), 16 deletions(-)
Approvals:
Giuseppe Lavagetto: Looks good to me, but someone else must approve
jenkins-bot: Verified
Volans: Looks good to me, approved
diff --git a/cumin/tests/unit/transports/test_clustershell.py
b/cumin/tests/unit/transports/test_clustershell.py
index fda6706..12d63d8 100644
--- a/cumin/tests/unit/transports/test_clustershell.py
+++ b/cumin/tests/unit/transports/test_clustershell.py
@@ -88,9 +88,9 @@
'command1', nodes=self.nodes,
handler=self.worker._handler_instance, timeout=None)
def test_execute_no_commands(self):
- """Calling execute() without commands should return without doing
anything."""
- self.worker.commands = []
- self.worker.execute()
+ """Calling execute() without commands should raise WorkerError."""
+ with pytest.raises(WorkerError, match=r'commands must be a non-empty
list'):
+ self.worker.commands = []
assert not self.worker.task.shell.called
def test_execute_one_command_no_mode(self):
diff --git a/cumin/tests/unit/transports/test_init.py
b/cumin/tests/unit/transports/test_init.py
index bfd762f..6e78929 100644
--- a/cumin/tests/unit/transports/test_init.py
+++ b/cumin/tests/unit/transports/test_init.py
@@ -158,8 +158,11 @@
"""Should return the ok_codes set, [0] otherwise."""
# Test empty list
command = transports.Command('command1')
+ assert command.ok_codes == [0]
command.ok_codes = []
assert command.ok_codes == []
+ command.ok_codes = [1, 255]
+ assert command.ok_codes == [1, 255]
def test_ok_codes_setter(self):
"""Should set the ok_codes to its value, unset it if None is passed."""
@@ -179,7 +182,7 @@
command.ok_codes = []
assert command._ok_codes == []
- with pytest.raises(transports.WorkerError, match='ok_codes must be a
list or None'):
+ with pytest.raises(transports.WorkerError, match='ok_codes must be a
list'):
command.ok_codes = 'invalid_value'
message = 'must be a list of integers in the range'
@@ -460,14 +463,16 @@
"""Transports module functions test class."""
def test_validate_list(self):
- """Should raise a WorkerError if the argument is not a list or None."""
- transports.validate_list('Test', None)
- transports.validate_list('Test', [])
+ """Should raise a WorkerError if the argument is not a list."""
transports.validate_list('Test', ['value1'])
transports.validate_list('Test', ['value1', 'value2'])
+ transports.validate_list('Test', [], allow_empty=True)
+
+ with pytest.raises(transports.WorkerError, match=r'Test must be a
non-empty list'):
+ transports.validate_list('Test', [])
message = r'Test must be a list'
- for invalid_value in (0, 'invalid_value', {'invalid': 'value'}):
+ for invalid_value in (0, None, 'invalid_value', {'invalid': 'value'}):
with pytest.raises(transports.WorkerError, match=message):
transports.validate_list('Test', invalid_value)
diff --git a/cumin/transports/__init__.py b/cumin/transports/__init__.py
index fb5c4fe..b664492 100644
--- a/cumin/transports/__init__.py
+++ b/cumin/transports/__init__.py
@@ -32,7 +32,8 @@
command -- the command to execute.
timeout -- the command's timeout in seconds. [optional, default: None]
ok_codes -- a list of exit codes to be considered successful for the
command. The exit code 0 is considered
- successful by default, this option allows to override it.
[optional, default: None]
+ successful by default, if this option is set it override
it. If set to an empty list it means
+ that any code is considered successful. [optional,
default: None]
"""
self.command = command
self._timeout = None
@@ -127,7 +128,7 @@
self._ok_codes = value
return
- validate_list('ok_codes', value)
+ validate_list('ok_codes', value, allow_empty=True)
for code in value:
if not isinstance(code, int) or code < 0 or code > 255:
raise_error('ok_codes', 'must be a list of integers in the
range 0-255 or None', value)
@@ -408,15 +409,18 @@
self._batch_sleep = value
-def validate_list(property_name, value):
- """Helper to validate a list or None, raise WorkerError otherwise.
+def validate_list(property_name, value, allow_empty=False):
+ """Helper to validate a list, raise WorkerError otherwise.
Arguments:
property_name -- the name of the property to validate
value -- the value to validate
"""
- if value is not None and not isinstance(value, list):
- raise_error(property_name, 'must be a list or None', value)
+ if not isinstance(value, list):
+ raise_error(property_name, 'must be a list', value)
+
+ if not allow_empty and not value:
+ raise_error(property_name, 'must be a non-empty list', value)
def validate_positive_integer(property_name, value):
diff --git a/cumin/transports/clustershell.py b/cumin/transports/clustershell.py
index a2c46fb..4151f7c 100644
--- a/cumin/transports/clustershell.py
+++ b/cumin/transports/clustershell.py
@@ -547,7 +547,8 @@
try:
node = self.nodes[worker.current_node]
- if worker.current_rc in
node.commands[node.running_command_index].ok_codes:
+ ok_codes = node.commands[node.running_command_index].ok_codes
+ if worker.current_rc in ok_codes or not ok_codes:
self.pbar_ok.update()
self.counters['success'] += 1
new_state = State.success
@@ -691,7 +692,8 @@
try:
node = self.nodes[worker.current_node]
- if worker.current_rc in
node.commands[node.running_command_index].ok_codes:
+ ok_codes = node.commands[node.running_command_index].ok_codes
+ if worker.current_rc in ok_codes or not ok_codes:
if node.running_command_index == (len(node.commands) - 1):
self.pbar_ok.update()
self.counters['success'] += 1
--
To view, visit https://gerrit.wikimedia.org/r/367823
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I918f7c133b0ab0a7944d7dd316a47f8a0ae2d829
Gerrit-PatchSet: 2
Gerrit-Project: operations/software/cumin
Gerrit-Branch: master
Gerrit-Owner: Volans <[email protected]>
Gerrit-Reviewer: Faidon Liambotis <[email protected]>
Gerrit-Reviewer: Gehel <[email protected]>
Gerrit-Reviewer: Giuseppe Lavagetto <[email protected]>
Gerrit-Reviewer: Volans <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
