Provide a new means to update a Eureka application record periodically

[seh]

Define the ScheduleAppUpdates and NewAppSource methods on EurekaConnection. The former supplies a channel of periodic updates to an application, while the latter holds the latest successfully acquired update of an application and makes it available for reading safely from any goroutine.

This addresses #36, but for now I left EurekaConnection's UpdateApp method in place, for fear of breaking existing callers. I considered documenting it as "deprecated", but without a commitment to remove it by a given date it felt disingenuous to wag a finger like that.

[seh]

Also, while I did include a set of example calls to these new methods, I did not include tests. I admit that I am unsure how best to test these methods, because I can't rely on having a particular Eureka server available.

I considered writing a fake EurekaConnection, but since it's not an interface, it's hard to substitute such a fake in a unit test. If you have suggestions as to how to write such a test (in this case confirming that we can receive periodic updates to a given application record), I'm willing to do so.

And now that I think about it more, if these new methods were free functions instead of being methods on EurekaConnection, they could instead accept a parameter like the following:

type ApplicationSupplier interface {
    getApp(string) (*Application, error)
}

The struct EurekaConnection already implements that interface.

Let me know what you think.

[tysonstewart]

@seh Thank you so much for your contribution! We'll look over it and try to get it merged as soon as possible. Just wanted to make sure you knew we're listening. :-)

[itsrainy]

And now that I think about it more, if these new methods were free functions instead of being methods on EurekaConnection, they could instead accept a parameter like the following:

type ApplicationSupplier interface {
    getApp(string) (*Application, error)
}

The struct EurekaConnection already implements that interface.

I think that's a good way to make these more testable. Although instead of making them free functions, you could make them methods on the ApplicationSupplier interface. That might make it cleaner from the caller's side.

[seh]

Although instead of making them free functions, you could make them methods on the ApplicationSupplier interface. That might make it cleaner from the caller's side.

Well, I noticed when trying to introduce this interface is that ScheduleAppUpdates requires access to EurekaConnection's PollInterval field. That means that having it rely solely on a GetApp method is insufficient. I could have ScheduleAppUpdates demand a polling interval parameter, but that dilutes the utility of EurekaConnection's PollInterval field. As far as I can tell, this is the whole point of including that field to begin with.

I am now less certain of the utility of introducing the proposed ApplicationSupplier interface.

// ApplicationSupplier is the interface that wraps the GetApp method, allowing
// retrieval of registered Eureka applications.
//
// GetApp returns the Eureka application with the given name. If no such
// application exists, it returns AppNotFoundError.
type ApplicationSupplier interface {
    GetApp(name string) (*Application, error)
}

Instead, the best I can see doing is hoisting ScheduleAppUpdates up to an interface, and having NewAppSource rely on that interface.

type ApplicationUpdater interface {
    ScheduleAppUpdates(name string, prime bool, done <-chan struct{}) <-chan AppUpdate {
}

Then:

func NewAppSource(u ApplicationUpdater, name string, prime bool) *AppSource {
// ...

That makes NewAppSource testable with a mock ApplicationUpdater, but we still can't test ScheduleAppUpdates with a mock of the GetApp method, which was my real goal.

[seh]

I'd appreciate any further critique you can offer here.

[itsrainy]

Sorry for letting this sit so long @seh. I missed your most recent commit. Other than tests, I think you've addressed all of my concerns. It'd be nice to be able to write tests, but I think mocking out the EurekaSupplier interface is outside of the scope of what you're trying to accomplish here.

I'm good with getting this merged in. @tysonstewart or @ryansb any additional thoughts?

[ryansb]

Looks good to me, just the minor nit on the prime parameter.