Skip to content

Commit

Permalink
Transports: better handling of empty list
Browse files Browse the repository at this point in the history
* BaseWorker: accept an empty list in the command setter. It's its
  default value, there is no point in forbidding a client to set it
  to the same value.
* ClusterShellWorker: return immediately if there are no target hosts.

Bug: T174911
Change-Id: I0d2bec28dd8381104fd56f22194dbeffb8b30570
  • Loading branch information
volans- committed Sep 8, 2017
1 parent 8e4ec8d commit 06e8e82
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 4 deletions.
14 changes: 11 additions & 3 deletions cumin/tests/unit/transports/test_clustershell.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,10 @@ def test_execute_custom_handler(self):
assert kwargs['handler'] == self.worker._handler_instance

def test_execute_no_commands(self):
"""Calling execute() without commands should raise WorkerError."""
with pytest.raises(WorkerError, match=r'commands must be a non-empty list'):
self.worker.commands = []
"""Calling execute() without commands should return immediately."""
self.worker.handler = ConcreteBaseEventHandler
self.worker.commands = None
assert self.worker.execute() is None
assert not self.worker.task.shell.called

def test_execute_one_command_no_mode(self):
Expand All @@ -106,6 +107,13 @@ def test_execute_one_command_no_mode(self):
with pytest.raises(RuntimeError, match=r'An EventHandler is mandatory\.'):
self.worker.execute()

def test_execute_no_target_hosts(self):
"""Calling execute() with a target without hosts should return immediately."""
self.target.hosts = []
self.worker.handler = ConcreteBaseEventHandler
assert self.worker.execute() is None
assert not self.worker.task.shell.called

def test_execute_wrong_mode(self):
"""Calling execute() without setting the mode with multiple commands should raise RuntimeError."""
with pytest.raises(RuntimeError, match=r'An EventHandler is mandatory\.'):
Expand Down
2 changes: 2 additions & 0 deletions cumin/tests/unit/transports/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,8 @@ def test_commands_setter(self):
assert self.worker._commands == self.commands
self.worker.commands = None
assert self.worker._commands is None
self.worker.commands = []
assert self.worker._commands == []
self.worker.commands = ['command1', 'command2']
assert self.worker._commands == self.commands

Expand Down
2 changes: 1 addition & 1 deletion cumin/transports/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ def commands(self, value):
self._commands = value
return

validate_list('commands', value)
validate_list('commands', value, allow_empty=True)
commands = []
for command in value:
if isinstance(command, Command):
Expand Down
4 changes: 4 additions & 0 deletions cumin/transports/clustershell.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ def execute(self):
self.logger.warning('No commands provided')
return

if not self.target.hosts:
self.logger.warning('No target hosts provided')
return

if self.handler is None:
raise RuntimeError('An EventHandler is mandatory.')

Expand Down

0 comments on commit 06e8e82

Please sign in to comment.