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

Demote ensure_computing to function #5153

Merged
merged 1 commit into from
Aug 2, 2021

Conversation

fjetter
Copy link
Member

@fjetter fjetter commented Aug 2, 2021

Follow up to #5128

@GPUtester
Copy link
Collaborator

Can one of the admins verify this patch?

@mrocklin
Copy link
Member

mrocklin commented Aug 2, 2021

Oh wow. I didn't realize that we were no longer doing anything async in this method. +1

@fjetter
Copy link
Member Author

fjetter commented Aug 2, 2021

I didn't realize that we were no longer doing anything async in this method

the execute is scheduled as part of the transition and the deserialization stuff is part of the execute itself. that let's us deal with possible deserialization exceptions the same way as for an ordinary compute failure (regarding the state machine). that's the essence of #5128

FWIW I would like to get rid of this function entirely since I believe everything that needs to be done here can be done as part of a transition which should make the entire thing a bit more streamlined imo. Currently, it's still required for pausing so we'll need to keep it regardless of my preference :)

@fjetter fjetter merged commit eaf05ac into dask:main Aug 2, 2021
@fjetter fjetter deleted the demote_ensure_computing_to_function branch August 2, 2021 14:10
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.

3 participants