Skip to content

Implementation of UnloadStateAsync#1750

Draft
olitomlinson wants to merge 2 commits intodapr:masterfrom
olitomlinson:unload-async
Draft

Implementation of UnloadStateAsync#1750
olitomlinson wants to merge 2 commits intodapr:masterfrom
olitomlinson:unload-async

Conversation

@olitomlinson
Copy link
Copy Markdown

Description

Adds an UnloadStateAsync() method to the Actor StateManager, so that state can be removed from the cache, without being removed from disk

Issue reference

#1728

Please reference the issue this PR will close: #[1728]

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

Implementation of UnloadStateAsyc (dapr#1728)

Signed-off-by: Oliver Tomlinson <oliverjamestomlinson@gmail.com>
Copy link
Copy Markdown
Contributor

@WhitWaldo WhitWaldo left a comment

Choose a reason for hiding this comment

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

At first pass, this looks great - I'll keep an eye out for when you pull it out of draft.

@olitomlinson olitomlinson changed the title Implementation of UnloadStateAsyc Implementation of UnloadStateAsync Mar 28, 2026
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <returns>A task that represents the asynchronous unload operation.</returns>
/// <exception cref="InvalidOperationException">Thrown if the state is modified and not yet persisted, unless allowed by options.</exception>
Task UnloadStateAsync(string stateName, UnloadStateOptions options = null, CancellationToken cancellationToken = default);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

consider adding as a default interface method with a NotImplementedException / virtual behavior.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

why?

Copy link
Copy Markdown
Contributor

@WhitWaldo WhitWaldo Apr 9, 2026

Choose a reason for hiding this comment

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

In this scenario, adding a default interface method to throw NotSupportedException isn't a meaningful inclusion because we already provide the concrete type implementing this interface ourselves, and this will ship with same-day support.

Especially as this project doesn't support dependency injection, I would not expect that developers have implemented their own types on this interface, modified the library to use them instead, update the package and see this new method on the interface and call it in their applications just to see a TypeLoadException because they didn't implement the method themselves - I just don't think that's likely. I wouldn't have expected them to wrap every actor invocation in a try/catch block either, so this would just shift throwing a TypeLoadException to an NotSupportedException - not a meaningful change here.

@@ -0,0 +1,14 @@
// Options for UnloadStateAsync operation
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

missing license header

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.

4 participants