Add Stop() to Instancer interface

[sidh]

Stop() func is not part of Instancer interface which makes it impossible to stop instancers in a generic way.

[yurishkuro]

I think it's because there is no generic definition of "stopping" the instancer, it' implementation specific and sometimes meaningless, e.g. for a static/fixed instancer.

In practice this should not be a problem because when you instantiate an instancer as one of the implementations, you actually get back a concrete type, so if it has Stop() you can call it.

[sidh]

I just checked and all instancer implementations do have Stop that does various things including stopping background goroutine which all of them do.

As for concrete type, yes you get it. But if you want to make an abstraction in your code and hide details behind gokit's sd.Instancer, you have to type cast to Stop it which is very annoying.

[yurishkuro]

I just checked and all instancer implementations do have Stop that does various things including stopping background goroutine which all of them do.

Well, this guy doesn't: FixedInstancer

But if you want to make an abstraction in your code and hide details behind gokit's sd.Instancer, you have to type cast to Stop it which is very annoying.

You can still hide the abstraction. There is no generic factory that gives you an Instancer, so your code always has to call explicit constructor, e.g.

var instancer sd.Instancer
var stopper func()
if discoveryType == "etcd" {
    if etcd, err := etcd.NewInstancer(...); err == nil {
      instancer = etc
      stopper = func() { etcd.Stop() } // make your generic stopper
    }
} else if discoveryType == "eureka" {
    // ...
}

[sidh]

Well, this guy doesn't: FixedInstancer

Not hard to add an empty func.

You can still hide the abstraction.

Yes, I never said its impossible. If you hide construction in some func and want an ability to stop it, you have to return a stopper func yourself as another argument. Or construct an object that holds both instancer and stopper and provide your own interfaces. In any case its more work compared to just calling instancer.Stop() on the same object and be done with it.

All I am saying is right now its inconvenient and could be made easier/better.

[basvanbeek]

Not hard to add empty func but definitely code smell.

If you don't like the proposed explicit example you could also use a consumer side interface.
Something like this:

type Stopper interface {
  Stop()
}

...
var instancer sd.Instancer
...

// check if we can call Stop
if instancer, ok := instancer.(Stopper); ok {
  instancer.Stop()
}