8000 Fix publish hang by pdeziel · Pull Request #64 · rotationalio/pyensign · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix publish hang #64

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

Merged
merged 2 commits into from
Sep 8, 2023
Merged

Fix publish hang #64

merged 2 commits into from
Sep 8, 2023

Conversation

pdeziel
Copy link
Collaborator
@pdeziel pdeziel commented Sep 8, 2023

This PR fixes an issue with the one-off publish scenario hanging the program. The issue is that we need to be treating the asyncio.CancelledError exception more seriously, if the user wants to cancel the task (e.g. closing the event loop) then we should honor that.

I have made the following changes:

  1. Moved task exception handling up to the worker level.
  2. Added regression test to catch the hang scenario

TODOs and questions

CHECKLIST

  • Is the commit message formatted correctly?
  • Do all of your functions and methods have docstrings?
  • Have you added/updated unit tests where appropriate?
  • Have you run the unit tests using pytest?
  • Is your code style correct (are you using PEP8, pyflakes)?

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #19678: PyEnsign hangs on exit.

@pdeziel pdeziel requested a review from bbengfort September 8, 2023 21:51
Copy link
Contributor
@bbengfort bbengfort 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 for helping debug and resolve this issue!

@bbengfort bbengfort merged commit bff7a8e into develop Sep 8, 2023
@bbengfort bbengfort deleted the sc-19678 branch September 8, 2023 22:12
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
0