[go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

fix: async shutdown handler not getting called #965

Merged
merged 14 commits into from
Oct 15, 2024
Merged

fix: async shutdown handler not getting called #965

merged 14 commits into from
Oct 15, 2024

Conversation

VishnuSanal
Copy link
Contributor

Description

This PR fixes #964

Summary

This PR does fix async shutdown handler not getting called

PR Checklist

Please ensure that:

  • The PR contains a descriptive title
  • The PR contains a descriptive summary of the changes
  • You build and test your changes before submitting a PR.
  • You have added relevant documentation
  • You have added relevant tests. We prefer integration tests wherever possible

Pre-Commit Instructions:

@vercel Vercel
Copy link
vercel bot commented Sep 20, 2024

@VishnuSanal is attempting to deploy a commit to the sparckles Team on Vercel.

A member of the Team first needs to authorize it.

@codspeed-hq CodSpeed HQ
Copy link
codspeed-hq bot commented Sep 20, 2024 •
edited
Loading

CodSpeed Performance Report

Merging #965 will not alter performance

Comparing VishnuSanal:fix-shutdown-handler (86f4700) with main (74cdabf)

Summary

✅ 116 untouched benchmarks

@sansyrox sansyrox marked this pull request as ready for review September 23, 2024 15:03
sansyrox
sansyrox reviewed Sep 23, 2024
View reviewed changes
robyn/cli.py Outdated Show resolved Hide resolved
sansyrox
sansyrox reviewed Sep 23, 2024
View reviewed changes
src/server.rs Outdated Show resolved Hide resolved
@VishnuSanal
Copy link
Contributor Author

note to self: Sanskar taking this over after discussion in the meeting. :)

@VishnuSanal
Copy link
Contributor Author
VishnuSanal commented Sep 28, 2024 •
edited
Loading
  • fix crash event loop already runnign

@VishnuSanal VishnuSanal marked this pull request as draft September 28, 2024 07:55
@VishnuSanal
Copy link
Contributor Author
VishnuSanal commented Sep 30, 2024 •
edited
Loading

current issues:

  • have to press ctrl + c twice for an app with async shutdown handler to quit when running as cli
  • dev server doesn't get terminated properly -- the post is occupied the next time the app runs (when using with async shutdown handler)

@VishnuSanal VishnuSanal marked this pull request as ready for review October 2, 2024 18:47
@sansyrox
Copy link
Member
sansyrox commented Oct 6, 2024

Hey @VishnuSanal ,

There are some conflicts. Can you have a look again?

@VishnuSanal
Copy link
Contributor Author

@sansyrox PTAL

@vercel Vercel
Copy link
vercel bot commented Oct 8, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
robyn ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 15, 2024 7:32pm

sansyrox
sansyrox reviewed Oct 8, 2024
View reviewed changes
robyn/cli.py Outdated Show resolved Hide resolved
sansyrox
sansyrox reviewed Oct 8, 2024
View reviewed changes
src/server.rs Show resolved Hide resolved
sansyrox
sansyrox reviewed Oct 8, 2024
View reviewed changes
src/server.rs Show resolved Hide resolved
sansyrox
sansyrox reviewed Oct 13, 2024
View reviewed changes
src/server.rs Outdated Show resolved Hide resolved
sansyrox
sansyrox approved these changes Oct 15, 2024
View reviewed changes
Copy link
Member
@sansyrox sansyrox left a comment

Choose a reason for hiding this comment

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

LGTM! 👍 🚀

@sansyrox sansyrox merged commit 06c487d into sparckles:main Oct 15, 2024
60 checks passed
@VishnuSanal VishnuSanal deleted the fix-shutdown-handler branch October 16, 2024 00:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sansyrox sansyrox sansyrox approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Shutdown handler not working
2 participants
@VishnuSanal @sansyrox