serviceId and serviceName inconsistencies

[sjoerdmulder]

The following example has multiple ServiceDefinition with the same name but different tags, currently this scenario doesnt work and the ServiceBrokerActor at line 70 throws a NoSuchElement exception since the lookup is by name and not by id. In most cases this works because id == name but if name and id differ it was broken; PR #19 fixes this

ServiceDefinition("http.service1", "service1", Set("http"))
ServiceDefinition("gprc.service1", "service1", Set("grpc"))

Could you maybe make a new release after this :question:

[sjoerdmulder]

@dlouwers Could you please review my PR? We are kinda waiting on this, and rather not create a forked release my self :smiling_imp:

[dlouwers]

Are you telling me that querying by name + tags is not working?

Hi, yeah can have a look tonight. One question though. Doesn't ID need to be unique for a node and name refer to a service? E.g. can, for instance, two HTTP service instances share the same ID? Seems like you would do:

id = service1.http.1
name = service1.http
tag = http

id = service1.http.2
name = service1.http
tag = http

& 

id = service1.grpc.1
name = service1.grpc
tag = grpc

id = service1.grpc.2
name = service1.grpc
tag = grpc

...when you have two nodes of this service, no? Because ID needs to be unique per node. So query by ID wouldn't make sense since that fetches a specific node and would only ever return at most one node.

[sjoerdmulder]

Ahhh, yes you are correct but that's another serviceID, i do see that the code currently will not work, wil rename the serviceId in ServiceDefinition to key so it becomes more clear and fix the PR

[dlouwers]

Ok, merged and described as fix allowing multiple connection strategies per named service.

[dlouwers]

Snapshot published 0.3.1-SNAPSHOT. Please let me know if this works for you.

[sjoerdmulder]

@dlouwers Yes it works. Tested it with a service that exposes the following 4 services (1 service running with 2 ports in HA mode) in consul:

ServiceID = "service1:192.168.1.1:10001"
ServiceName = "service1"
ServiceTags = [ "grpc" ]

ServiceID = "service1:192.168.1.1:10002"
ServiceName = "service1"
ServiceTags = [ "grpc" ]

ServiceID = "service1:192.168.1.1:10003"
ServiceName = "service1"
ServiceTags = [ "http" ]

ServiceID = "service1:192.168.1.1:10004"
ServiceName = "service1"
ServiceTags = [ "http" ]

And querying with the following:

ServiceDefinition("grpc.service1", "service1", Set("grpc"))
ServiceDefinition("http.service1", "service1", Set("http"))

Delivers round-robin for the grpc.service1 the addresses of

192.168.1.1:10001
192.168.1.1:10002
192.168.1.1:10001
192.168.1.1:10002

[dlouwers]

Ok good to hear. Will create a release. Will be 1.3.0 instead of 1.3.1. Thanks for the PR. Btw are you using the library in production? Would love to hear from your experience if you do and perhaps include a logo or something like that.

[dlouwers]

Release done as 1.3.0

[sjoerdmulder]

Not yet in production but it might be in the near future, will let you know then :+1: