Skip to content

A more ergonomic macro #43

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

Open
patrickariel opened this issue Feb 17, 2025 · 4 comments
Open

A more ergonomic macro #43

patrickariel opened this issue Feb 17, 2025 · 4 comments
Labels
enhancement New feature or request

Comments

@patrickariel
Copy link

The Rust SDK currently forces you to write separate traits for all of your services, objects and workflows. I think it would be far more ergonomic to have the macro apply directly to the implementation body itself:

pub struct MyService;

#[restate_sdk::service]
impl MyService {
    async fn my_handler(&self, _ctx: Context<'_>, greeting: String) -> Result<String, HandlerError> {
        Ok(format!("{greeting}!"))
    }
}

pub struct MyVirtualObject;

#[restate_sdk::object]
impl MyVirtualObject {
    #[shared]
    async fn my_concurrent_handler(
        &self,
        ctx: SharedObjectContext<'_>,
        greeting: String,
    ) -> Result<String, HandlerError> {
        Ok(format!("Greetings {} {}", greeting, ctx.key()))
    }
}

pub struct MyWorkflow;

#[restate_sdk::workflow]
impl MyWorkflow {
    async fn run(&self, _ctx: WorkflowContext<'_>, _req: String) -> Result<String, HandlerError> {
        Ok(String::from("success"))
    }
}

This also eliminates the awkward and non-idiomatic FooBar <-> FooBarImpl naming convention.

What do you think of this approach?

@slinkydeveloper
Copy link
Collaborator

I think it's worth giving this a try, the macro could support both starting from the trait or starting from the struct impl.

IIRC one reason I didn't go down this road is that I had some issues with the Futures Send bound, that in the impl case had to be explicitly defined by the user. Would you be willing to take a stab at prototyping this? If that's the case, perhaps let's agree first on which code the macro should generate, and then we get on the macro.

@h7kanna
Copy link
Contributor

h7kanna commented Feb 18, 2025

I remember doing this in my Rust sdk prototype (Though not directly applicable here)

Macros: https://github.com/h7kanna/restate-sdk-rust/blob/main/crates/derive/src/lib.rs#L224
Exampe: https://github.com/h7kanna/restate-sdk-rust/blob/main/examples/src/example1.rs

@patrickariel
Copy link
Author

patrickariel commented Feb 18, 2025

I didn't go down this road is that I had some issues with the Futures Send bound, that in the impl case had to be explicitly defined by the user.

Was it due to this lint: async_fn_in_trait? If we go the inherent impl route, I think this shouldn't apply anymore. Also I think the lint is only relevant when you intend consumers to use the trait, but in this case it's basically a single-use interface.

perhaps let's agree first on which code the macro should generate, and then we get on the macro.

The macro above could expand to something like this, skipping the traits altogether:

pub struct MyService;

impl MyService {
    async fn my_handler(
        &self,
        context: Context<'_>,
        greeting: String,
    ) -> Result<String, HandlerError> {
        todo!()
    }

    #[doc = r" Returns a serving function to use with [::restate_sdk::endpoint::Builder::with_service]."]
    fn serve(self) -> ServeMyService {
        ServeMyService {
            service: Arc::new(self),
        }
    }
}

#[derive(Clone)]
pub struct ServeMyService {
    service: Arc<MyService>,
}

impl Discoverable for ServeMyService {
    /* */
}

impl Service for ServeMyService {
    /* */
}

Here, &self and context will have to be explicitly defined by the user instead of being implicitly generated, but I think the explicitness is actually nicer.

@slinkydeveloper
Copy link
Collaborator

slinkydeveloper commented Feb 19, 2025

Fair enough, I think it makes sense to give a shot at this.

The only rule though that I think it should apply is that when you annotate an impl block, to tell the macro which functions are handlers, the user should explicitly annotate them either with #[handler] or #[shared]. Otherwise this becomes a footgun, where you expose functions you didn't want to.

@slinkydeveloper slinkydeveloper added the enhancement New feature or request label Feb 20, 2025
BigFish2086 pushed a commit to BigFish2086/sdk-rust that referenced this issue Mar 19, 2025
Modify the procedural macros in ast.rs, gen.rs, and lib.rs to apply
directly to impl blocks instead of traits, per Issue restatedev#43. This eliminates
the need for separate trait definitions and simplifies the SDK's usage.

New macro syntax examples:
- #[restate_sdk::service] or #[restate_sdk::service(vis = "pub", name = "my_service")]
- #[restate_sdk::object] or #[restate_sdk::object(vis = "pub")]
- #[restate_sdk::workflow(name = "my_workflow")]
- Method attributes: #[handler], #[handler(shared)], or #[handler(name = "my_handler")]
BigFish2086 pushed a commit to BigFish2086/sdk-rust that referenced this issue Mar 19, 2025
BigFish2086 pushed a commit to BigFish2086/sdk-rust that referenced this issue Mar 19, 2025
BigFish2086 pushed a commit to BigFish2086/sdk-rust that referenced this issue Mar 19, 2025
BigFish2086 pushed a commit to BigFish2086/sdk-rust that referenced this issue Mar 19, 2025
BigFish2086 pushed a commit to BigFish2086/sdk-rust that referenced this issue Mar 19, 2025
BigFish2086 pushed a commit to BigFish2086/sdk-rust that referenced this issue Mar 19, 2025
BigFish2086 pushed a commit to BigFish2086/sdk-rust that referenced this issue Mar 19, 2025
BigFish2086 pushed a commit to BigFish2086/sdk-rust that referenced this issue Mar 19, 2025
BigFish2086 pushed a commit to BigFish2086/sdk-rust that referenced this issue Mar 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants