Refactoring of the project, give more coherence to the Registration struct #36
Conversation
Soulou
force-pushed
the
fix/35/refactoring
branch
from
5419807 to
1205609
Compare
| @@ -25,15 +19,15 @@ const ( | |||
| // This service will launch two go routines. The first one will maintain the | |||
| // registration every 5 seconds and the second one will check if the service | |||
| // credentials don't change and notify otherwise | |||
| func Register(ctx context.Context, service string, host Host) *Registration { | |||
| func Register(serviceName string, host Host) (*Registration, error) { | |||
There was a problem hiding this comment.
Why don't you keep the context here to forward it to the NewRegistration function?
There was a problem hiding this comment.
There is just no need of context in here, as I wanted to use the context to stop the registration, but it's not possible.
When a context is canceled, there is no way to way for the registration to remove itself cleanly from etcd synchronously, the entity which canceled the context will keep its execution, like shutting down the application.
Now the Registration can be Stop(), which is synchronous and do it in a clean manner, it's a better way to handle the lifecycle of the Registration. The context cancellation not being used anymore, there was no more requirement of the context.
| cancel context.CancelFunc | ||
| shutdownErr error | ||
|
|
||
| credsWatcher chan Credentials |
There was a problem hiding this comment.
Maybe a few comments to explain the purpose of these channels?
| time.Sleep(time.Second) | ||
| } | ||
|
|
||
| for err != nil { |
There was a problem hiding this comment.
after the previous loop, err equals nil. Will it ever enter this loop? Not sure.
| User: "Moi", | ||
| Password: "Lui", | ||
| } | ||
| // Convey("After a service registration", t, func() { |
|
@Soulou you may just have forgotten to push your changes here? |
|
I actually have not done them yet.. |
|
Closing, we won't work on etcd-discovery anymore, it will be replaced by something else |
Fixes #35