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

feat(framework) Allow clients to exit gracefully #3090

Merged
merged 54 commits into from
Jun 10, 2024
Merged

feat(framework) Allow clients to exit gracefully #3090

merged 54 commits into from
Jun 10, 2024

Conversation

charlesbvll
Copy link
Member

@charlesbvll charlesbvll commented Mar 10, 2024 •
edited
Loading

Issue

Description

  • Currently we have to press Ctrl+C 2 times to exit the flower-client-app in some cases
  • The flower-client-app doesn't exit gracefully

Related issues/PRs

N/A

Proposal

Explanation

Catch the interrupt exception and stop the program gracefully.

Checklist

  • Implement proposed change
  • Update the changelog entry below
  • Make CI checks pass
  • Ping maintainers on Slack (channel #contributions)

Changelog entry

Any other comments?

N/A

@charlesbvll charlesbvll marked this pull request as ready for review March 10, 2024 19:52
charlesbvll and others added 20 commits March 11, 2024 07:14
@charlesbvll
Merge branch 'main' of https://github.com/adap/flower into better-cli…
dc2bed0 
…ent-exit
@charlesbvll
Remove nested try-except
90ed4f0
@charlesbvll
Merge branch 'main' into better-client-exit
7c48d02
@charlesbvll
Revert log changes
240c31f
@charlesbvll
Merge branch 'main' into better-client-exit
d902a5b
@charlesbvll
Merge branch 'main' into better-client-exit
ec7b57c
@charlesbvll
Merge branch 'main' into better-client-exit
7534e01
@charlesbvll
Merge branch 'main' of https://github.com/adap/flower into better-cli…
6a55c4f 
…ent-exit
@charlesbvll
Use signals
48910a2
@charlesbvll
Merge branch 'main' of https://github.com/adap/flower into better-cli…
e931d20 
…ent-exit
@charlesbvll
Merge branch 'main' of https://github.com/adap/flower into better-cli…
d968b6c 
…ent-exit
@charlesbvll
Better exception
5b17f2e
@charlesbvll
Add type hints
53a6d6f
@charlesbvll
Add disables
38aed6a
@charlesbvll
Simplify implementation
73d68e2
@charlesbvll
Add docstring
aa76470
@charlesbvll
Merge branch 'main' into better-client-exit
549df99
@charlesbvll
Move signal handler registration before try/except
27ad48c
@charlesbvll
Merge branch 'main' into better-client-exit
251b299
@charlesbvll
Merge branch 'main' into better-client-exit
7f35989
jafermarq
jafermarq previously approved these changes May 28, 2024
View reviewed changes
Copy link
Contributor

@jafermarq jafermarq left a comment

Choose a reason for hiding this comment

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

works great! let's have a followup PR that calls delete_node() when processing a ctrl+c event?

@charlesbvll charlesbvll mentioned this pull request May 29, 2024
Merged
4 tasks
@charlesbvll charlesbvll mentioned this pull request May 30, 2024
Merged
4 tasks
jafermarq
jafermarq previously approved these changes May 30, 2024
View reviewed changes
@jafermarq jafermarq mentioned this pull request Jun 8, 2024
Merged
@charlesbvll charlesbvll mentioned this pull request Jun 9, 2024
Merged
3 tasks
@danieljanes danieljanes enabled auto-merge (squash) June 10, 2024 09:33
@danieljanes danieljanes disabled auto-merge June 10, 2024 09:33
@danieljanes danieljanes enabled auto-merge (squash) June 10, 2024 09:34
@danieljanes danieljanes merged commit e182983 into main Jun 10, 2024
29 checks passed
@danieljanes danieljanes deleted the better-client-exit branch June 10, 2024 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@panh99 panh99 panh99 left review comments

@danieljanes danieljanes danieljanes approved these changes

@jafermarq jafermarq jafermarq left review comments

@tanertopal tanertopal Awaiting requested review from tanertopal tanertopal is a code owner

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

Successfully merging this pull request may close these issues.
4 participants
@charlesbvll @danieljanes @jafermarq @panh99