More system actors and more consistent system actor naming #1649
Conversation
|
|
| @@ -36,7 +37,7 @@ public async Task SetupAsync(Cluster cluster, string[] kinds, bool isClient) | |||
| await Storage.Init(); | |||
|
|
|||
| var workerProps = Props.FromProducer(() => new IdentityStorageWorker(this)); | |||
| _worker = _system.Root.Spawn(workerProps); | |||
| _worker = _system.Root.SpawnNamedSystem(workerProps, WorkerActorName); | |||
There was a problem hiding this comment.
I'm not sure about whether this should be a system actor. As far as I understand this worker lives alongside the IdentityStorageLookup and stops when that is stopped hence in that case it makes sense for me that this is spawn as a system actor.
| @@ -196,7 +196,7 @@ internal IEndpoint GetClientEndpoint(string systemId) | |||
| private void SpawnActivator() | |||
| { | |||
| var props = Props.FromProducer(() => new Activator(_remoteConfig, _system)); | |||
| ActivatorPid = _system.Root.SpawnNamedSystem(props, "activator"); | |||
| ActivatorPid = _system.Root.SpawnNamedSystem(props, "$activator"); | |||
There was a problem hiding this comment.
I see that this change breaks the remoting tests as other places still rely on the old name.
The name could be turned into a const as for the other named actors above, e.g. $server_seed and $client_seed
There was a problem hiding this comment.
I didn't get to run the tests on my machine as I don't have the 3.1 sdk 😃 I can fix that. Though it's not comforting knowing that there are such "couplings" in the system with dependencies on actor names. Have it been a discussion point to reduce that implicit coupling by either introducing an abstraction between or having something akin to a centralized "name registry" (don't think I like this 😀 ), or something completely different
There was a problem hiding this comment.
82187d7 This should resolve it
|
Very nice! |
Description
This PR intends to solve issues #1604 and #1603.
Purpose
This pull request is a:
Checklist