Consul support as coordination service #12
Conversation
| @@ -70,8 +70,8 @@ abstract class MultiNodeConstructrSpec extends MultiNodeSpec(ConstructrMultiNode | |||
|
|
|||
| "Constructr should manage an Akka cluster" in { | |||
| runOn(nodes.head) { | |||
| "docker rm -f constructr-etcd".!(ProcessLogger(_ => ())) | |||
| s"""docker run --name constructr-etcd -d -p 2379:2379 quay.io/coreos/etcd:v2.2.1 -advertise-client-urls http://$host:2379 -listen-client-urls http://0.0.0.0:2379""".! | |||
| "docker-machine ssh default docker rm -f constructr-etcd".!(ProcessLogger(_ => ())) | |||
There was a problem hiding this comment.
What's the purpose of this change?
There was a problem hiding this comment.
We'll remove this change as we've seen that is not necessary.
| case Event(Coordination.SelfAdded(context), _) => | ||
| goto(State.RefreshScheduled).using(stateData.copy( | ||
| coordinationRetriesLeft = coordinationRetries, | ||
| context = context.asInstanceOf[B#Context] |
There was a problem hiding this comment.
This is ugly but we haven't found a cleaner version.
There was a problem hiding this comment.
;-)
I think you should simply use a typed pattern in the pattern match three lines above:
case Event(Coordination.SelfAdded(context: B#Context), _) => ...
|
Great stuff! I have a couple of minor style related changes. I assume the test passes? ;-) |
|
May I also ask you to squash the various commits into one with a short and meaningful name, e.g. "Add support for Consul"? |
gerson24
force-pushed
the
consul-support-delta
branch
from
44af85f
to
2a1f6ce
Compare
| def toNodes(s: String) = { | ||
| import rapture.json._ | ||
| import rapture.json.jsonBackends.spray._ | ||
| def jsonToNode(json: Json) = { |
There was a problem hiding this comment.
More nitpicks ;-)
These curly braces aren't needed. Move the next line up, just after the equals sign.
|
Looks really good. Just some more nitpicks (sorry, I'm German, it's in my blood). |
|
LOL No problem!. I'll fix this minor issues right away. Thanks!. |
| @@ -147,7 +147,11 @@ final class ConstructrMachine[A: Coordination.AddressSerialization, B <: Coordin | |||
| } | |||
|
|
|||
| when(State.AddingSelf, coordinationTimeout) { | |||
| case Event(Coordination.SelfAdded(_), _) => goto(State.RefreshScheduled).using(stateData.copy(coordinationRetriesLeft = coordinationRetries)) | |||
| case Event(Coordination.SelfAdded(context: B#Context), _) => | |||
There was a problem hiding this comment.
@hseeberger, this is an unchecked type pattern. We're facing a compile-time warning because of that:
Warning:(150, 50) abstract type pattern B#Context is unchecked since it is eliminated by erasure
case Event(Coordination.SelfAdded(context: B#Context), _) =>
How do you prefer to deal with it?
There was a problem hiding this comment.
Add an @unchecked annotation, like above in line 113 case Event(nodes: List[A] @unchecked, _) => ....
juanjovazquez
force-pushed
the
consul-support-delta
branch
from
2a1f6ce
to
53b930a
Compare
|
@hseeberger, new installment trying to be more descriptive. Eliminated some duplications also. |
|
This is awesome work. Thanks! |
Consul support as coordination service
Proposal to incorporate Consul as other possible coordination service in addition to etcd.