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

fix: remove conditional autostart handling #56

Merged
merged 4 commits into from
Sep 12, 2022

Conversation

theborakompanioni
Copy link
Collaborator

@theborakompanioni theborakompanioni commented Aug 21, 2022

Main change: Remove the conditional autostart feature.
Services will all be started by default.

Also contains a short delay (5s) when starting jmwalletd and ob-watcher to wait for tor port bindings.
Otherwise there might be a race condition and jm might try to start its own daemon (which will fail) before trying again (subsequently successful). Both services will be considered healthy only if they are running for at least 10 seconds (effectively 5 seconds considering the delay). This is a workaround as supervisord has no concept of dependencies between services. Currently looking at dinit as a replacement which might be suitable in the near future.

@theborakompanioni theborakompanioni self-assigned this Aug 21, 2022
@theborakompanioni theborakompanioni changed the title fix: remove autostart handling fix: remove conditional autostart handling Aug 21, 2022
@theborakompanioni theborakompanioni marked this pull request as ready for review August 29, 2022 13:57
Copy link
Contributor

@dergigi dergigi left a comment

Choose a reason for hiding this comment

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

Not the biggest fan of sleep, but looks good to me in general ✅

@theborakompanioni
Copy link
Collaborator Author

theborakompanioni commented Sep 9, 2022

Not the biggest fan of sleep, but looks good to me in general white_check_mark

Same here, this approach is anything but bulletproof.. Searched for alternatives but did not want to do major changes before the next release. Thinking about trying dinit, and if it fits the purpose, use it in an upcoming version.

Will not merge for another day or two.. maybe a better fix comes to mind. If not, I'll nonetheless merge it later.
Thanks for reviewing 🧡

@theborakompanioni theborakompanioni merged commit ac30470 into master Sep 12, 2022
@theborakompanioni theborakompanioni deleted the remove-autostart-concept branch September 12, 2022 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants