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

[WIP] Actor proof of concept #63

Closed
wants to merge 5 commits into from

Conversation

thomas-tribus
Copy link

I made a proof of concept of a possible implementation of the Actor API.

This is my first ever open source contribution and I'm also fairly new to rust. I'm really open to any suggestion.

Also not 100% sold on my implementation, but eager to hear some thoughts. Problem is that best as I can read the docs, Dapr only uses HTTP for the Actor API, which is unfortunate given the GPRC implementation of the Rust SDK. So really going forward would this would probably mean adding the rest of the API to the HTTP client.

It is also a bit more opiniated than the rest of the SDK, I couldn't really figure out a good way to offer the services required without shipping a HTTP server. I chose Axum given its closeness to the rest of the stack (based on tower / hyper).

Apart from being incomplete, the implementation is also far less ergonomic than its C# counter-parts. I'm sure there is a way to improve, but that is beyond (my time availability / me ) at this point ;-).

If there is some agreement that this is a generally right solution directory I can probably slowly contribute and add some missing features. If not, not hard feelings. Maybe this will inspire someone else to do something better.

TODO

  • Add example
  • Move behind feature flag
  • Proper IntoResult implementation that returns appropriate HTTP error codes
  • Is it OK to ship with Axum?
  • Add unit tests
  • Add type checking and return Serialize and Deserialize Errors when appropriate
  • Use POST body as parameters to invoke method
  • Add state API
  • Add timer API
  • Add delete API
  • Add reminder API
  • Add deactivation API
  • Expose non-actor APIs over HTTP
  • Probably more

@thomas-tribus
Copy link
Author

I tried to add the do-not-merge label, but I think I can't edit labels?

@thomas-tribus thomas-tribus mentioned this pull request Nov 22, 2021
@yaron2
Copy link
Member

yaron2 commented Mar 29, 2022

@thomas-tribus first I want to extend apologies on behalf of all Dapr maintainers - we have overlooked the Rust SDK for quite sometime due to a lack of maintainers, and your amazing contribution went under the radar.

It would be amazing to add Rust actors to Dapr and I think the direction you're taking here is correct. What do you need from us in order to proceed?

@thomas-tribus
Copy link
Author

@yaron2 good to get some response. No worries about the late reply. I made this PR while learning about dapr, and it was a fun exercise.

What I needed was some general feedback, about this being the right direction in general. What I needed first was some feedback on the desirability and implementation direction of my work. So thanks for that :-)

Problem is that while I had some time last November, I don't really have it now. This is something I'd be happy to work on, but I have a bit too much other stuff going on right now. I might have some time in a few months.

Not really sure what the best thing is now. I just reread my own PR, and a lot of things need a lot of work. It is a proof of concept, but many important things are not implemented yet, or not in a durable manner.

We can leave this open for now, and I'll try and make some progress when I have some time. But that might not happen for a good while.

@yaron2
Copy link
Member

yaron2 commented Mar 31, 2022

We can leave this open for now, and I'll try and make some progress when I have some time. But that might not happen for a good while.

Sounds good. We'll keep this open!

@NickLarsenNZ
Copy link
Contributor

This relates to #99

@cscetbon
Copy link
Contributor

Does it still make sense to keep this open ?

@thomas-tribus
Copy link
Author

thomas-tribus commented Jan 15, 2024 via email

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

4 participants