Skip to content
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

Add API for UntypedActorWithStash types #6327

Merged
merged 2 commits into from
Jan 19, 2023

Conversation

ismaelhamed
Copy link
Member

@ismaelhamed ismaelhamed commented Jan 5, 2023

Added utility abstract classes for UntypedActors that support stashing:

  • UntypedActorWithStash
  • UntypedActorWithUnboundedStash
  • UntypedActorWithUnrestrictedStash

Depends on #6325

Checklist

@ismaelhamed ismaelhamed force-pushed the untypedactorwithstash branch 3 times, most recently from d1acd40 to d689dbe Compare January 5, 2023 09:18
@Aaronontheweb
Copy link
Member

@ismaelhamed #6325 has been merged. I can start work on backporting it probably on Friday unless you want to do it (via git cherry-pick)

@ismaelhamed
Copy link
Member Author

ismaelhamed commented Jan 19, 2023

@ismaelhamed #6325 has been merged. I can start work on backporting it probably on Friday unless you want to do it (via git cherry-pick)

I think I can find the time to do it myself. I will be backporting #6323, #6325 and #6327 if you're fine with it.

@ismaelhamed ismaelhamed marked this pull request as ready for review January 19, 2023 08:39
@Aaronontheweb
Copy link
Member

@ismaelhamed #6325 has been merged. I can start work on backporting it probably on Friday unless you want to do it (via git cherry-pick)

I think I can find the time to do it myself. I will be backporting #6323, #6325 and #6327 if you're fine with it.

Yes, that would delight me

Copy link
Member

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the general goal behind this API change to move away from the actor creation pipeline, so the stash is provided automatically as part of the base class infrastructure itself rather than injected in from the outside?

@Aaronontheweb Aaronontheweb enabled auto-merge (squash) January 19, 2023 20:13
Copy link
Member

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Aaronontheweb Aaronontheweb merged commit 303ad3c into akkadotnet:dev Jan 19, 2023
@ismaelhamed
Copy link
Member Author

ismaelhamed commented Jan 20, 2023

Is the general goal behind this API change to move away from the actor creation pipeline, so the stash is provided automatically as part of the base class infrastructure itself rather than injected in from the outside?

@Aaronontheweb If you think about it, using stashing is currently a little weird since users must implement an interface (including declaring the Stash property that, BTW, they are not instantiating themselves). That's because Scala traits can have behavior, which can't be done with interfaces in JAVA and CSharp (well, we have default interfaces now, but you get the point).

So, it is natural that these utility classes exist in the JAVA API, allowing the user to just derive from a class (like they would with normal actors) and get everything needed for stashing already set up. They can still implement the interfaces themselves if they choose to, but this is a bit of an improvement on the usability side.

@Aaronontheweb
Copy link
Member

They can still implement the interfaces themselves if they choose to, but this is a bit of an improvement on the usability side.

Time will tell I guess, but I see your point.

ismaelhamed added a commit to ismaelhamed/akka.net that referenced this pull request Jan 21, 2023
Co-authored-by: Aaron Stannard <aaron@petabridge.com>
Aaronontheweb added a commit that referenced this pull request Jan 24, 2023
)

* Read stash capacity from actor's mailbox or dispatcher configuration (#6323)

* Added support for `UnrestrictedStash` (#6325)

* Add API for UntypedActorWithStash types (#6327)

Co-authored-by: Aaron Stannard <aaron@petabridge.com>

Co-authored-by: Aaron Stannard <aaron@petabridge.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants