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

Rename task_cancel lock and use as ctx manager #658

Merged
merged 1 commit into from
Jan 7, 2022

Conversation

sirosen
Copy link
Member

@sirosen sirosen commented Jan 7, 2022

Context-manager lock usage is safer because it releases the lock if an exception is raised.

The name of the lock is bad because it's used by task cancel and by result sending. task_finalization_lock seems like an appropriate rename.


This is meant to address outstanding comments from #606

Context-manager lock usage is safer because it releases the lock if an
exception is raised.

The name of the lock is bad because it's used by task cancel and by
result sending. `task_finalization_lock` seems like an appropriate
rename.
@sirosen sirosen requested review from benclifford and yadudoc January 7, 2022 16:48
Copy link
Collaborator

@yadudoc yadudoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. This makes #657 obsolete.

@yadudoc yadudoc merged commit 3f70145 into globus:main Jan 7, 2022
@sirosen sirosen deleted the cleanup-task-cancel-lock branch January 7, 2022 16:50
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