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

feat: handle os signals #126

Merged
merged 2 commits into from
Apr 2, 2024
Merged

feat: handle os signals #126

merged 2 commits into from
Apr 2, 2024

Conversation

lvlcn-t
Copy link
Collaborator

@lvlcn-t lvlcn-t commented Mar 16, 2024

Motivation

To address this conversation #125 (comment)

Changes

We're now handling the os shutdown signals so the sparrow still shuts down, even in case of a controlled shutdown. This also addresses the bug that sparrow instances stay registered on the remote repository even though the sparrow instance doesn't run anymore.

Additionally to that I've adjusted the README to the changes made in #122 and removed the error handling of the (*Sparrow).Run method because it's always non nil.

  • feat: add os signal handling for graceful shutdown
  • chore: housekeeping

For additional information look at the commits.

Tests done

  • Unit tests succeeded
  • E2E tests succeeded - we have none for graceful shutdown

TODO

  • I've assigned this PR to myself
  • I've labeled this PR correctly

@lvlcn-t lvlcn-t added bug Something isn't working docs Improvements or additions to documentation labels Mar 16, 2024
@lvlcn-t lvlcn-t self-assigned this Mar 16, 2024
Copy link
Collaborator

@puffitos puffitos left a comment

Choose a reason for hiding this comment

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

LGTM

@lvlcn-t lvlcn-t merged commit cf97baa into main Apr 2, 2024
11 checks passed
@lvlcn-t lvlcn-t deleted the feat/handle-os-signals branch April 2, 2024 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working docs Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants