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

Reword log message when function raises a cancellation error #2070

Merged
merged 2 commits into from
Aug 5, 2024

Conversation

mwaskom
Copy link
Contributor

@mwaskom mwaskom commented Aug 5, 2024

The previous log message was a bit misleading as we can get to this line for reasons other than the user explicitly cancelling a function call — for example if the function that spawned the function call hits its retry limit, or potentially anything else that causes an asyncio.CancelledError to bubble up.

Related to MOD-3132. Not sure if it closes it — we may be able to do more here — but wanted to do a quick fix to make the message less actively confusing.

Copy link

@modal-pr-review-automation modal-pr-review-automation bot left a comment

Choose a reason for hiding this comment

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

Auto-approved 👍. This diff qualified for automatic approval and doesn't need follow up review.

@mwaskom mwaskom force-pushed the michael/improve-cancellation-error branch from 6dbc5ae to ebb04e6 Compare August 5, 2024 12:50
@mwaskom mwaskom force-pushed the michael/improve-cancellation-error branch from ebb04e6 to 3bbbb4b Compare August 5, 2024 13:05
modal/_container_io_manager.py Outdated Show resolved Hide resolved
@mwaskom mwaskom requested a review from erikbern August 5, 2024 14:40
Copy link
Contributor

@gongy gongy left a comment

Choose a reason for hiding this comment

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

Thank you!

@mwaskom mwaskom merged commit 34b31cd into main Aug 5, 2024
23 checks passed
@mwaskom mwaskom deleted the michael/improve-cancellation-error branch August 5, 2024 15:13
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