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

Remove unnecessary todo from DurableStateStoreRegistry #32207

Conversation

Captain1653
Copy link
Contributor

A little suggestion :) .

I was checking todo items in the project and found this.

This code was created in this PR. This PR is related to #30277 . This issue was released 2 years ago.

Should this todo be removed?

@Captain1653 Captain1653 force-pushed the remove-unnecessary-todo-durable-state-store-registry branch from b8fe017 to d175fd8 Compare October 23, 2023 18:35
@Captain1653
Copy link
Contributor Author

@johanandren Can it be reviewed? Please )

@johanandren
Copy link
Member

@patriknw was this just a left over or something to actually deal with?

Copy link
Member

@patriknw patriknw left a comment

Choose a reason for hiding this comment

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

LGTM, yes we can remove the fixme

@@ -85,9 +85,7 @@ class DurableStateStoreRegistry(system: ExtendedActorSystem)
* Java API: Returns the [[akka.persistence.state.javadsl.DurableStateStore]] specified by the given
* configuration entry.
*/
final def getDurableStateStoreFor[T <: javadsl.DurableStateStore[_]](
@unused clazz: Class[T], // FIXME generic Class could be problematic in Java
Copy link
Member

Choose a reason for hiding this comment

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

I think it was related to that in Java you can't easily create/get a Class with a generic parameter. E.g. this would not work:

Class<DurableStateStoreQuery<Record>> durableStateStoreQueryClass = DurableStateStoreQuery.class;

We get away by using this in the doc sampel:

DurableStateStoreQuery<Record> durableStateStoreQuery =
        DurableStateStoreRegistry.get(system)
            .getDurableStateStoreFor(DurableStateStoreQuery.class, pluginId);

That is an untyped warning, but probably fine.

@johanandren johanandren merged commit cae221b into akka:main Nov 10, 2023
5 checks passed
@Captain1653 Captain1653 deleted the remove-unnecessary-todo-durable-state-store-registry branch November 11, 2023 06:50
He-Pin pushed a commit to He-Pin/akka that referenced this pull request Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants