New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename PersistentBehavior #25721

Closed
patriknw opened this Issue Oct 2, 2018 · 15 comments

Comments

Projects
7 participants
@patriknw
Copy link
Member

patriknw commented Oct 2, 2018

@viktorklang suggested that we should rename PersistentBehavior because it's not the Behavior that is stored.

I think the background of the name is from PersistentActor and PersistentEntity.

One suggestion is PersistingBehavior.

@patriknw

This comment has been minimized.

Copy link
Member

patriknw commented Oct 2, 2018

BehaviorWithPersistence?

@viktorklang

This comment has been minimized.

Copy link
Member

viktorklang commented Oct 2, 2018

PersistanceBehavior?
EntityBehavior?

@johanandren

This comment has been minimized.

Copy link
Member

johanandren commented Oct 3, 2018

I think we should keep the current name to make it easy to find for existing users of PersistentActor, even if there could potentially be more correct names.

@TimMoore

This comment has been minimized.

Copy link
Contributor

TimMoore commented Oct 3, 2018

How about EventSourcedBehavior? It describes that the behavior is dependent on the event stream. It would also open up an opportunity for other kinds of persistence to be added in the future.

I take your point @johanandren but there are enough differences between the way that PersistentActor and PersistentBehavior are used that people are going to need to read the migration docs regardless.

@johanandren

This comment has been minimized.

Copy link
Member

johanandren commented Oct 3, 2018

Fair enough. Then I like EventSourced/ing best since Persisting could be done in more ways than eventsourcing.

@viktorklang

This comment has been minimized.

Copy link
Member

viktorklang commented Oct 3, 2018

I think EventSourcedBehavior makes sense (if we decide not to unify the CRUD and CRUD+EL in the same thing).

@chbatey

This comment has been minimized.

Copy link
Member

chbatey commented Oct 3, 2018

I like EventSourcedBehavior, and we could rename akka-persistence-typed to akka-event-sourcing to prevent the misinterpretation that it is the only way to do persistence with akka.

@WadeWaldron

This comment has been minimized.

Copy link
Contributor

WadeWaldron commented Oct 3, 2018

I think EventSourcedBehavior makes sense (if we decide not to unify the CRUD and CRUD+EL in the same thing).

Perhaps we need to figure out our plans for CRUD/CRUD+EL before we make the decision then. What we do there might dictate what we do here.

I actually think that EventSourcedBehaviour isn't really any better than PersistentBehaviour. It's not really the Behaviour that is Event Sourced. It's the state. Or perhaps more accurately it's actually the state and behaviour.

This is actually one of my biggest concerns with the Typed API in general. By eliminating the concept of an Actor and/or Entity we have created a gap in the terminology. Actors encapsulate state and behaviour so the term Actor would work well here, but we don't have that term to draw on.

@patriknw

This comment has been minimized.

Copy link
Member

patriknw commented Oct 3, 2018

CRUD and CRUD+EL

I think it would be enough if that API is (source) similar and not necessarily the same. The implementation would anyway be very different and doesn't make much sense to have an instance that is both event sourced and CRUD.

@viktorklang

This comment has been minimized.

Copy link
Member

viktorklang commented Oct 3, 2018

@patriknw I'm not sure I agree. Being able to programmatically (Effect) store the state as a part of the command processing makes a ton of sense, for instance when you have made an expensive calculation and want to make sure that it doesn't need to get replayed. It would be my personal preference to see if it is possible to fit it under the same API.

One possible way of dealing with the EventSourcing part is to have that be a demarcation on the State type. Since, it is as @WadeWaldron says: the state which is event sourced. Then the State could have an applyEvent method.

@TimMoore

This comment has been minimized.

Copy link
Contributor

TimMoore commented Oct 5, 2018

Even if there is some unification of the event-sourced, CRUD, and event-log APIs, I would expect that there would still be some differences, too. For one thing, they wouldn't all have event handlers. So to me it makes sense to start with what we know we have (EventSourcedBehavior) and then extract out any commonalities once we start to introduce new ones.

@TimMoore

This comment has been minimized.

Copy link
Contributor

TimMoore commented Oct 5, 2018

@WadeWaldron my understanding is that in Akka Typed, the behavior is considered to encapsulate state. In the case of immutable behavior, it closes over it and returns a new behavior to transition state. It seems to me like the behavior is event-sourced, as the command handler that is in effect depends on the state (i.e., the result of replaying events from the last snapshot).

I suppose this is a new concept for people already used to Akka Untyped, but OTOH, it's a different enough model that reusing the same terminology could be more confusing. For new users, I'm not sure it will make much difference.

@patriknw

This comment has been minimized.

Copy link
Member

patriknw commented Oct 5, 2018

I like EventSourcedBehavior

@patriknw patriknw added this to Backlog in Akka Typed Nov 9, 2018

@helena helena self-assigned this Nov 30, 2018

@helena helena moved this from Backlog to In Progress in Akka Typed Nov 30, 2018

@helena

This comment has been minimized.

Copy link
Member

helena commented Dec 3, 2018

Currently in akka-persistence-typed there exists object EventsourcedBehavior

@patriknw

This comment has been minimized.

Copy link
Member

patriknw commented Dec 3, 2018

That is internal, and can be renamed to EventSourcedBehaviorImp, or whatever

helena added a commit to helena/akka that referenced this issue Dec 4, 2018

helena added a commit to helena/akka that referenced this issue Dec 4, 2018

@helena helena added this to the 2.5.19 milestone Dec 4, 2018

patriknw added a commit that referenced this issue Dec 5, 2018

Rename PersistentBehavior (#25721)
Migrated InternalProtocol with least refactor changes, in the end.

patriknw added a commit that referenced this issue Dec 5, 2018

Merge pull request #26048 from akka/wip-26036
Rename PersistentBehavior (#25721) (for validation)

@patriknw patriknw closed this Dec 5, 2018

Akka Typed automation moved this from In Progress to Done Dec 5, 2018

@patriknw patriknw referenced this issue Dec 7, 2018

Merged

Akka 2.5.19 #534

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment