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

Refactor to add ABANDONED message #392

Merged
merged 2 commits into from
Jul 10, 2021
Merged

Refactor to add ABANDONED message #392

merged 2 commits into from
Jul 10, 2021

Conversation

mdickinson
Copy link
Member

This is a slight cleanup of the BaseFuture and wrapper logic.

Previously, when execution of the background task's callable was abandoned due to cancellation, we acted as though it had run anyway, sending a STARTED message followed by a RETURNED message (with an argument of None) to the future. That was slightly smelly (inventing a return result when none existed), and involved sending and processing two messages when one would have been sufficient.

This PR introduces a new ABANDONED message for this. It also:

  • expands the CANCELLED future state into three different internal states, depending on whether the task's callable completed, failed, or was never executed (abandoned)
  • keeps the result and/or the exception information from the background task even when cancellation occurred. It's private, but it's there to be used during debugging if necessary
  • annotates the semi-private methods in the BaseFuture to add explicit state changes

This PR is pure refactoring: it introduces no user-visible changes. The tests for the internals of the BaseFuture did need to be updated to take account of the extra message type. I'll self-merge after self-review.

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.

None yet

1 participant