Skip to content

Commit

Permalink
Make Waiting.resume() idempotent (#285)
Browse files Browse the repository at this point in the history
Calling `Waiting.resume()` when it had already been resumed would raise
an exception. Here, the method is made idempotent by checking first
whether the future has already been resolved. This fix ensures the
behavior matches the behavior of the other state transitions: calling
`play` on an already running process and calling `pause` on an already
paused process isn't rising any error.

Cherry-pick: 20e5898
  • Loading branch information
sebaB003 authored and sphuber committed Jul 1, 2024
1 parent 94df0df commit a79497b
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 6 deletions.
4 changes: 4 additions & 0 deletions src/plumpy/process_states.py
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,10 @@ async def execute(self) -> State: # type: ignore # pylint: disable=invalid-over

def resume(self, value: Any = NULL) -> None:
assert self._waiting_future is not None, 'Not yet waiting'

if self._waiting_future.done():
return

self._waiting_future.set_result(value)


Expand Down
11 changes: 5 additions & 6 deletions test/rmq/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,15 @@
version: '3.4'

services:

rabbit:
image: rabbitmq:3.8.3-management
container_name: plumpy-rmq
image: rabbitmq:3-management-alpine
container_name: plumpy_rmq
ports:
- 5672:5672
- 15672:15672
environment:
RABBITMQ_DEFAULT_USER: guest
RABBITMQ_DEFAULT_PASS: guest
ports:
- '5672:5672'
- '15672:15672'
healthcheck:
test: rabbitmq-diagnostics -q ping
interval: 30s
Expand Down
25 changes: 25 additions & 0 deletions test/test_processes.py
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,31 @@ async def async_test():
loop.create_task(proc.step_until_terminated())
loop.run_until_complete(async_test())

def test_double_restart(self):
"""Test that consecutive restarts do not cause any issues, this is tested for concurrency reasons."""
loop = asyncio.get_event_loop()
proc = _RestartProcess()

async def async_test():
await utils.run_until_waiting(proc)

# Save the state of the process
saved_state = plumpy.Bundle(proc)

# Load a process from the saved state
loaded_proc = saved_state.unbundle()
self.assertEqual(loaded_proc.state, ProcessState.WAITING)

# Now resume it twice in succession
loaded_proc.resume()
loaded_proc.resume()

await loaded_proc.step_until_terminated()
self.assertEqual(loaded_proc.outputs, {'finished': True})

loop.create_task(proc.step_until_terminated())
loop.run_until_complete(async_test())

def test_wait_save_continue(self):
""" Test that process saved while in WAITING state restarts correctly when loaded """
loop = asyncio.get_event_loop()
Expand Down

0 comments on commit a79497b

Please sign in to comment.