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

Added Watchdog #52

Merged
merged 1 commit into from
Feb 20, 2018
Merged

Added Watchdog #52

merged 1 commit into from
Feb 20, 2018

Conversation

theimo1221
Copy link
Contributor

This should fix #16

This should fix pmaji#16
@theimo1221 theimo1221 mentioned this pull request Feb 20, 2018
@CrackLord
Copy link
Contributor

CrackLord commented Feb 20, 2018

Ah good find. Wouldn't it be better if we try to catch and handle the exception from the GDAX endpoint? Maybe wrapping the GDAX function in a try catch would solve the issue?

I guess what we want to stop is an exception causing the thread to crash, so if we catch and handle exceptions then it should also solves the issue. To me that would be a cleaner fix to the issue than creating a watchdog.

Since you can reproduce also you could test to be sure that it fixes it.

@theimo1221
Copy link
Contributor Author

For proper error handling I totally agree
But at this state we won't be able to see all possible exceptions.
It runs like 2 hours without any problems
So having a watchdog restarting it whenever it crashes is no problem
Especially when this will be a permanently running service a watchdog is always nice

Copy link
Contributor

@CrackLord CrackLord left a comment

Choose a reason for hiding this comment

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

Makes sense to me to have a watchdog either way to ensure that the threads are always running I guess.

@pmaji pmaji merged commit 381d9f7 into pmaji:master Feb 20, 2018
@CrackLord
Copy link
Contributor

Nice, I've pushed the update to my server now.

@theimo1221 theimo1221 deleted the patch-2 branch February 27, 2018 02:26
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.

Hosting
3 participants