Repository navigation
ActorKind Refactor - #20
Open
TylerBloom wants to merge 11 commits into
Open
TylerBloom wants to merge 11 commits into
TylerBloom wants to merge 11 commits into
Conversation
… state thus allowing ActorState and ActorBuilder to be slimmed and more ergonomic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently,
ActorStaterequires four associated types. One of which,Output, is not needed in many/most cases. Another,Permanence, is a bit strange, adds confusion, and is also not important most of the time. These strange, confusing bits thread themselves all the way through the crate's model. This PR removes these pieces, making the crate easier to pick up and use while unlocking additional features in the future.Removing
OutputfromActorStateis a complete win forSinkactors as it is a completely unnecessary type and means that theSchedulerno longer contains additional fields that aren't used. Moreover, it means there aren't no-op methods in the scheduler's API in some cases. Replacing this is theActorKindassociated type (formerlyActorType). It now has a trait bound,ActorKind(ya, naming things is difficult). Originally,ActorKindsoftly implied a type of client. This change now directly encodes that. EveryActorKindhas an associatedClienttype and will construct the initial client upon the actor's launch. Additionally, theActorKindcan contain state, rather than being a simple marker type. This state becomes embedded in the scheduler and will be accessible to the actor's state. Embedding theActorKindlike this removes the blanketbroadcastfield that the scheduler had before. Since that state is directly available, theSchedulercan be extended for a variety of future kinds of actors without needing to extend the scheduler itself.These changes unlock a new dimension of extensibility. For example, there are use cases for sink and stream clients that have built-in back pressure. This feature will simply require adding a new
ActorKindtype along with its client. Similarly, one might need a SPSC actor. This too would just require a newActorKind.The
Permanenceassociate type was also removed fromActorStateas it was only really needed for some parts of theSinkClient's API. Should that type of functionality be needed in the future, it can be modeled with different types ofActorKind's (or by makingSinkActorgeneric).Lastly, the "edge map" feature of the scheduler and
ActorBuilderwas removed. This can be added back if the need arises. As it stood, this was premature and clunky.