dlouwers / reactive-consul

Consul client for Scala
MIT License
18 stars 7 forks source link

ServiceBroker should be created with unique actor name #24

Closed nonsleepr closed 6 years ago

nonsleepr commented 6 years ago

Hardcoded name prevents the creation of multiple instances of ServiceBroker.

dlouwers commented 6 years ago

I wouldn't have thought this to be a potential issue. What scenario do you need multiple ServiceBrokers in? Simplest might be to add the option to pass a name that defaults to "default".

nonsleepr commented 6 years ago

Can't recall exact case but I guess I had brokers for different services.

dlouwers commented 6 years ago

Ok, well all my constructors are convenience methods. You can easily take control like so:

def apply(systemName: String, consulAddress: URL, services: Set[ConnectionStrategy]): ServiceBroker = {
    implicit val rootActor = ActorSystem(systemName)
    val httpClient = new AkkaHttpConsulClient(consulAddress)
    ServiceBroker(rootActor, httpClient, services)
  }

Thank you for your feedback.

nonsleepr commented 6 years ago

That's what I did in the end.

dlouwers commented 6 years ago

Good. Do you feel that is fine or that is such a common use case there should be a special constructor for it?

nonsleepr commented 6 years ago

It probably is. The thing is, I followed the example to create service provider (I needed to have two providers for different services) and stumbled upon that problem.