Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Waiting.resume() idempotent (#285) #286

Closed
wants to merge 1 commit into from

Conversation

sebaB003
Copy link
Contributor

@sebaB003 sebaB003 commented Jul 1, 2024

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.

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.
@sebaB003
Copy link
Contributor Author

sebaB003 commented Jul 1, 2024

I executed the tests on my local machine running python 3.11 and all tests passed, in the github workflow the tests on python 3.7 failed, maybe some dependencies are broken. Also python 3.7 have been discontinued 1 year ago, we could remove the test.

2024-07-01_15-22

@sphuber
Copy link
Collaborator

sphuber commented Jul 1, 2024

Thanks @sebaB003 . I have fixed the CI, cherry-picked your fix that was merged onto master and released a patch version v0.21.11: https://pypi.org/project/plumpy/0.21.11/
Let me know if that works.

Thanks for your contributions!

@sphuber sphuber closed this Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants