Skip to content

Conversation

@gvdongen
Copy link
Collaborator

We want to show Durable Execution in the quickstart. This allows us to make the quickstart touch upon the main features of Restate.

pub fn send_notification(greeting_id: &str, name: &str) {
if random::<f32>() < 0.5 {
println!("👻 Failed to send notification: {} - {}", greeting_id, name);
panic!("Failed to send notification: {} - {}", greeting_id, name);
Copy link
Contributor

Choose a reason for hiding this comment

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

not a good idea, convert this to error. I'm not even sure we catch panics actually.

Copy link
Contributor

Choose a reason for hiding this comment

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

one thing you can do to make this code better overall is the following:

Change the signature of this function to

pub async fn send_notification(greeting_id: &str, name: &str) -> anyhow::Result<()>

And then the context run should be as easy as

ctx.run(|| send_notification(&greeting_id, &name)).await?

@@ -0,0 +1,17 @@
use rand::random;

pub fn send_notification(greeting_id: &str, name: &str) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comments above

Copy link
Contributor

@slinkydeveloper slinkydeveloper left a comment

Choose a reason for hiding this comment

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

one minor thing and lgtm

Comment on lines +7 to +12
class GreetingRequest(BaseModel):
name: str


class Greeting(BaseModel):
message: str
Copy link
Contributor

Choose a reason for hiding this comment

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

I would honestly have them in example.py, makes the whole thing way more readable, and this is super small...

@gvdongen gvdongen merged commit d5bb855 into main Nov 22, 2024
6 checks passed
@gvdongen gvdongen deleted the new_quickstart branch November 22, 2024 10:20
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.

3 participants