-
Notifications
You must be signed in to change notification settings - Fork 45
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
Remove heartbeat | Add Deregister #884
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems this PR is unfinished. There are a few questions regarding the changes you made but more importantly there seem to be a few things that have not been tackled at all yet. We have an updating interval in the serviceupdater
struct defined despite us getting rid of the loop using that value.
I also wonder whether we still need the waitgroup
and the cancelfunc
given that we get rid of the update loop which we originally used it for.
…related codes and config | handle PR comments
Besides some naming conventions for functions, everything looks good. Everything else is working as expected. I tested by registering |
Fair point. We can call it |
Everything looks good besides the config, that I have mentioned above. I have tested |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks good.
Did you run
make format && make check
? YesFixes Skywire Service #447
Changes:
Delete
entry toDeregister
Deregister
on client side (skywire) stop app process and visor shutdown process.How to test this PR:
sd.skycoin.com/api/services?type=vpn
for related PK.