akka / akka-edge-rs

Akka Edge support in Rust
https://doc.akka.io/docs/akka-edge/current/
Other
9 stars 1 forks source link

Sample code / effect systems complexity #79

Closed olofwalker closed 1 year ago

olofwalker commented 1 year ago

I assume that we aim this against Scala developers already familiar with Akka and the effect system, so this may be okay and might only need to be cleaned up a bit so it's easier on the eyes.

https://doc.akka.io/docs/akka-edge-preview/snapshot/guide-rs/3-temperature-entity.html

But I wonder how many Rust developers intuitively get the and(then without us having a section on it?

              emit_event(Event::TemperatureRead { temperature })
                    .and(then(
                        move |behavior: &Self,
                              _new_state: Option<&State>,
                              prev_result: effect::Result| {
                            behavior.send_broadcast_event(
                                prev_result,
                                broadcast_entity_id,
                                broadcast_temperature,
                            )
                        },
                    ))
                    .boxed()
olofwalker commented 1 year ago

I would like a bit more words about the effects in the documentation for Then, Reply etc. image

huntc commented 1 year ago

I assume that we aim this against Scala developers already familiar with Akka and the effect system, so this may be okay and might only need to be cleaned up a bit so it's easier on the eyes.

https://doc.akka.io/docs/akka-edge-preview/snapshot/guide-rs/3-temperature-entity.html

But I wonder how many Rust developers intuitively get the and(then without us having a section on it?

              emit_event(Event::TemperatureRead { temperature })
                    .and(then(
                        move |behavior: &Self,
                              _new_state: Option<&State>,
                              prev_result: effect::Result| {
                            behavior.send_broadcast_event(
                                prev_result,
                                broadcast_entity_id,
                                broadcast_temperature,
                            )
                        },
                    ))
                    .boxed()

The scope has been to empower existing developers of Akka at the edge. I think we're ok here.

With that particular bit of code, there's another task that may change this area. The task concerns itself with harmonising events published to the browser by using gRPC-web. Meanwhile, I'm happy to accept suggestions on how to make this existing code clearer.

huntc commented 1 year ago

I would like a bit more words about the effects in the documentation for Then, Reply etc.

I've now annotated those effect types to point at the functions that return them, which is where the effects are described. See https://github.com/lightbend/akka-edge-rs/pull/76

olofwalker commented 1 year ago

If we aim only for a subset of Scala/Akka developers that have dealt with effect systems before, we probably are ok.

But the documentation still assumes that you know what effects systems are and how they are supposed to be used, I can't find that a then is supposed to execute whether the future failed or not. Something to think about.

patriknw commented 1 year ago

One detail regarding current syntax is that I find it difficult to discover what is possible after and(. In Scala we have .thenReply and .thenRun.

The bigger question is if we need effects in Rust or if it would work with await?

if (let Ok(new_state) = emit_event(Event::TemperatureRead { temperature })).await {
  reply(reply_to, new_state.clone())
}
huntc commented 1 year ago

The entity manager must pass in behaviour, context and modified state to the effects… We’d be compromising a bit to drop the effect approach at this stage.

huntc commented 1 year ago

Also, the approach here with effects is influenced by the JVM code. We can depart from that, but a goal has always been to adopt a similar approach to the JVM code. Moving away from effects here might be very reasonable, but I think it may question why the JVM has effects also.

I can run a quick experiment to see how this code looks without the effect approach. The good thing now is that we’ve got a good body of code in place to test out this idea.

I’ll separately include and_then as a combinator as a convenience for and(then(…. That will solve an issue in terms of discoverability at least.

huntc commented 1 year ago

I've now provided and_then as a convenience. Aside from experimenting with async, will https://github.com/lightbend/akka-edge-rs/pull/87 resolve the main issue here?

patriknw commented 1 year ago

Yeah, I'm biased by the Scala/Java API and that comes natural to me. Just wanted to raise the question so that we are not following old habits if there are no good reasons.

but I think it may question why the JVM has effects also

There we are running in an actor and don't have a nice way for the user to wait for persist to complete before running the side effects. We don't wan't to mix actor code with Future composition.

Also, we shouldn't speak about this as an advanced effect system, it's really just a small dsl to express instructions of what to do after persisting events.

olofwalker commented 1 year ago

Also, we shouldn't speak about this as an advanced effect system, it's really just a small dsl to express instructions of what > to do after persisting events.

Is there a reason then to keep it? DSLs in general are hard to motivate IMO unless they come with considerable savings on some axis.

huntc commented 1 year ago

Is there a reason then to keep it? DSLs in general are hard to motivate IMO unless they come with considerable savings on some axis.

I think it’d be worth seeing how this looks using futures, but let’s not forget what I mentioned earlier. Various parameters would have to be passed to the future-yielding functions eg context, updated state, entity operations implementation… the effect trait as we have it hides all of that.

huntc commented 1 year ago

I'm going to close this. The original issue is now gone given https://github.com/lightbend/akka-projection-temp/pull/14 and friends.