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

feat(*)!: Updates wasi-messaging interface with feedback #23

Closed

Conversation

thomastaylor312
Copy link
Collaborator

This PR incorporates various points of feedback from discussion amongst the interface champions, users of Wasm projects, and discussion in the CNCF wasm WG. This smooths over some of the rough edges and adds support for request/reply paradigms. Some of the bigger changes are:

  • Addition of request/reply as an optional interface included in a separate world
  • Removal of the custom format type in favor of an optional content type string
  • Addition of the topic field to the message type
  • Concrete errors in form of a variant

This makes several updates to the messaging interface. Initially the
README said that this wasn't going to support request/reply, but based
on my reading of the Kafka, NATS, MQTT, and SQS APIs, this is a fairly
common pattern. Another piece of evidence here is what I've seen as a
wasmCloud maintainer from our users. Request/reply is one of the more
common things we see with a messaging service. Please note that this
doesn't _require_ the use of a reply-to topic, just exposes it for use.

I also did a few other changes here. First is that I added the topic to
the message. This was common across all systems and is often used by code
to select the appropriate logic to perform. I also removed the format
field as this didn't seem to be a common parameter across various services.
We could definitely add a content-type member to this record in the future
if needed, but I think much of that can be passed via the metadata field.

There are other things I might suggest some changes to, but I want to think
on them some more and open some issues to discuss them first

Signed-off-by: Taylor Thomas <taylor@cosmonic.com>
This PR integrates various changes from talking to current users of
messaging in the community as well as conversations among the champions

Signed-off-by: Taylor Thomas <taylor@cosmonic.com>
wit/consumer.wit Show resolved Hide resolved
wit/guest.wit Show resolved Hide resolved
wit/types.wit Show resolved Hide resolved
wit/request-reply.wit Outdated Show resolved Hide resolved
wit/consumer.wit Outdated Show resolved Hide resolved
wit/request-reply.wit Show resolved Hide resolved
Copy link
Member

@lukewagner lukewagner left a comment

Choose a reason for hiding this comment

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

I'm only starting to understand the conceptual model of this proposal, so I just had some possibly-naive questions to start with:

wit/types.wit Outdated Show resolved Hide resolved
wit/types.wit Outdated Show resolved Hide resolved
wit/guest.wit Outdated Show resolved Hide resolved
wit/types.wit Outdated Show resolved Hide resolved
@thomastaylor312
Copy link
Collaborator Author

Thanks for all the great feedback! Gimme until tomorrow and I should be able to make all the changes

I also deleted the examples.md for now until we settle on the interface.
It will be easier to add back in once we have some real world examples
to point at

Signed-off-by: Taylor Thomas <taylor@cosmonic.com>
@thomastaylor312
Copy link
Collaborator Author

Ok I've pushed up some additional changes as discussed

Copy link
Collaborator

@danbugs danbugs left a comment

Choose a reason for hiding this comment

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

These changes are great. Thanks for your work, @thomastaylor312 !

Mostly LGTM, just one last note - I'd just probably not delete the examples.md document. I think it can be a useful recource for people to see how an interface like this will be used "in the real world" at a quick glance. Do you think we could bring it back or achieve that goal in some other way?

@thomastaylor312
Copy link
Collaborator Author

My plan was to actually implement something with this interface and then copy over. I deleted purely because it was completely out of date. I am also fine to put something back together once people are ok with the changes. I'll do that before we merge

Copy link

@brooksmtownsend brooksmtownsend left a comment

Choose a reason for hiding this comment

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

Overall looks nice. The new request-reply interface looks really great! Left some comments for discussion

wit/types.wit Show resolved Hide resolved
wit/types.wit Outdated Show resolved Hide resolved
Copy link
Collaborator

@devigned devigned left a comment

Choose a reason for hiding this comment

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

I like where this is going. I have a handful of comments and questions.

imports-request-reply.md Show resolved Hide resolved
examples.md Show resolved Hide resolved
wit/consumer.wit Show resolved Hide resolved
wit/types.wit Outdated Show resolved Hide resolved
wit/types.wit Show resolved Hide resolved
Also removes extensions as a guest configuration option (for now)

Signed-off-by: Taylor Thomas <taylor@cosmonic.com>
In many of the interfaces out there right now, we've moved more towards
just calling these things config

Signed-off-by: Taylor Thomas <taylor@cosmonic.com>
wit/producer.wit Outdated Show resolved Hide resolved
wit/producer.wit Outdated
send: func(c: client, ch: channel, m: list<message>) -> result<_, error>;
/// Sends a message to the given channel/topic. If the channel/topic is not empty, it will
/// override the channel/topic in the message.
send: func(c: client, ch: channel, m: message) -> result<_, error>;
Copy link

Choose a reason for hiding this comment

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

Suggested change
send: func(c: client, ch: channel, m: message) -> result<_, error>;
send: func(c: client, ch: channel, m: message, opts: option<send-options>) -> result<_, error>;
record send-options {
  timeout-ms: option<u32>
}

I expect the timeout to be configurable. How are you envisioning the situation around it?

Are you envisioning to add the value to the message itself?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See this thread above for more information around ttl/timeout. Right now this only matters if you're in a request/reply

Copy link

Choose a reason for hiding this comment

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

I wasn't thinking of the ttl message itself, but the network timeout, if you will, something around the lines of "I give you 2 seconds to tell me you have received (not process) the message" per request-based

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think we'll want to add that kind of guarantee to the interface right now. That is much more of a Kafka Stream or NATS JetStream type functionality. The guarantees we offer here are just that the host responds with an error if unable to send a message out, not if it was received. Like I mentioned elsewhere, I think that will be an extension interface on top of this

wit/request-reply.wit Outdated Show resolved Hide resolved
wit/types.wit Outdated Show resolved Hide resolved
wit/request-reply.wit Outdated Show resolved Hide resolved
wit/types.wit Outdated
metadata: option<list<tuple<string, string>>>
/// A message with a binary payload and additional information
resource message {
constructor(topic: topic, data: list<u8>, content-type: option<string>, metadata: option<list<tuple<string, string>>>);
Copy link

@yordis yordis Jun 13, 2024

Choose a reason for hiding this comment

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

What are the expectations around the Message ID? Such metadata is so critical that it would be good to have one way to figure this out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is a message ID actually important to a guest here? Most of the time those are used in acks or other advanced operations. Each implementation can take the wit type message and convert it to/assign it an ID based on implementation details IMO.

Copy link

Choose a reason for hiding this comment

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

From the guest's perspective, I think it is critical for the guest to control the identity of the message, the guest is most likely the component aware of the domain, while the host is at the platform level.

What the host would do with it shouldn't be part of the spec. That is a different story. Still, guest could find a component that provides the guarantees required around the dedupe and whatnot.

It's worth saying that the ID doesn't mean unique; it means an identifier.

Copy link

Choose a reason for hiding this comment

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

For whatever is worth, "messaging.message_id" OpenTelemetry Semantic Conventions Trace exists. Just to point to the important of such information

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I'd prefer to hold off and add this once we have a concrete use case for it

wit/types.wit Outdated Show resolved Hide resolved
wit/types.wit Outdated Show resolved Hide resolved
Copy link
Member

@lukewagner lukewagner left a comment

Choose a reason for hiding this comment

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

Changes are looking good! Just a few more ideas/suggestions:

wit/consumer.wit Outdated Show resolved Hide resolved
wit/guest.wit Outdated Show resolved Hide resolved
wit/types.wit Outdated Show resolved Hide resolved
wit/request-reply.wit Outdated Show resolved Hide resolved
wit/request-reply.wit Outdated Show resolved Hide resolved
/// Replies to the given message with the given response message. The details of which channel
/// the message is sent to is up to the implementation. This allows for reply to details to be
/// handled in the best way possible for the underlying messaging system.
reply: func(reply-to: borrow<message>, reply: message) -> result<_, error>;
Copy link
Member

Choose a reason for hiding this comment

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

What if instead of reply being an imported interface, it was instead an exported interface:

interface incoming-request-reply-handler {
  handle: func(msg: message) -> result<message, error>;
}

Here, the reply is just the success case of the result. This can avoid some of the weird spec interaction questions like I was pointing out in my other comment on complete. Thus, there could be a request-reply world that looks like:

world request-reply-messaging {
  include messaging;
  import outgoing-request-reply-handler;
  export incoming-request-reply-handler;
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So I actually think this would make things too complicated. This would make it so a component would have implement a different handle function if it is responding to requests. The reply function here is for a handler to use to reply to a message it has received, no matter the source. The host could throw it away or decide not to send it (for example, in the code I've written for NATS, if reply wasn't set, we just don't send anything back). That could still work here, but I don't like the idea of having another interface the guest has to export rather than just sending the reply and letting the implementation decide what to do with it (which feels more compatible)

Copy link
Member

@lukewagner lukewagner Jul 17, 2024

Choose a reason for hiding this comment

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

Maybe you could help me understand how these request-reply messaging APIs are used in practice because it feels like there's something I'm missing. I guess a basic question is: is calling the reply method on a message passed as the parameter to handle able to express some use cases that returning the reply as a return value cannot? If so: what are they? If not, how is ye olde parameters+results not the simpler interface?

If there is a good reason for having reply be a function, I think the spec text needs to go into more detail about what happens in various cases such as:

  • What is the interaction between the return value of handle (the result) and reply: if I reply but then return an error (signalling that this message should not be considered "processed", per the current spec text), does that mean handle might get called again for the same message and, in that case, will the host care of making sure there is only one reply or does that guest have to worry about that?
  • If I destroy a message without calling reply, is some default reply sent (that would be received by a blocking request caller) and if so, what is it?
  • What happens if I call reply multiple times for the same message?
  • If I reply to a message that was sent by send (not request) what happens? Is there any way to detect whether a message is "replyable"?
  • If I forward a message (to any of send, request or reply) and that recipient calls reply, who does the reply go to?

I think all these questions go away if replies are return values, because there are just less moving parts (which suggests that it is indeed the simpler interface).

Copy link
Collaborator

Choose a reason for hiding this comment

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

In reply to the parallels between handle's result and reply + your two first bullet points: The way I see it, wasi:messaging's handle is similar to wasi:https incoming-handler's handle. So, maybe, just like it, we shouldn't have a return type here. Instead, the guest should either do nothing or explicitly reply to a given message. Otherwise, unavoidably, I think we will need to have an implicit reply and we should move way from anything implicit like that.

What happens if I call reply multiple times for the same message?

First, I think we have to better define who the reply is to. That is, I don't think this should be up to the implementation like it currently says - Considering request-reply is an imported interface (and, hence, implemented by a host), why does the, say, NATS implementor decide who I am sending messages to? Instead, I think we should be clear that it goes to the topic specified in the reply message. Note, it should not go to the topic in the reply-to message because, if it did, the message would be handled by the same handler we are calling reply from and we'd get infinite recursion.

Second, when replying, the consumer is essentially acting as a producer, so, whatever happens when we reply multiple times should be decided by the handler of whatever topic that message was sent to.

If I reply to a message that was sent by send (not request) what happens? Is there any way to detect whether a message is "replyable"?

There should be no difference. Every message should be replyable. Maybe we should clarify this, if you think this could be a point of confusion.

If I forward a message (to any of send, request or reply) and that recipient calls reply, who does the reply go to?

I touched on this point in my previous reply. Currently, the interface states that this should be decided by the implementor of reply (i.e., a host), but I don't think that should be the case. Instead, we should always reply the topic specified in the reply message, so it can be handled by some other handler.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

FWIW, I originally had the idea of an optional reply-to field than then the handler can just call the producer.send function to send a message back, but got push back on it initially. To me there seems to be 3 options:

  1. Have this be in the metadata of the message and entirely external to the interface
  2. Optional reply to field that a handler can use with producer.send
  3. A reply-request interface

My opinions on the 3:

  1. Bad idea because every implementor may or may not have it and the naming is not standardized
  2. Nicer because it is named, but if you want to depend on it, the implementation might not provide it
  3. Best because an implementation can indicate if it supports replying. This is why the details of how to reply are left up to said implementation rather than being a field on the message

wit/types.wit Outdated Show resolved Hide resolved
wit/types.wit Outdated Show resolved Hide resolved
Also removes the channel parameter I forgot to remove in a previous
commit

Signed-off-by: Taylor Thomas <taylor@cosmonic.com>
@thomastaylor312
Copy link
Collaborator Author

Hey all! Sorry for the delay in responding, got pulled away to some other work. I've implemented all suggested changes and I think we're ready for a final review @danbugs @devigned @lukewagner. Once we've approved this, I'll add back an examples file that matches the new interfaces before merging

metadata: option<list<tuple<string, string>>>
/// A message with a binary payload and additional information
resource message {
constructor(topic: string, data: list<u8>);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please note that I shrunk this down to avoid any breaking changes. I can't think of any scenario where you wouldn't need to provide at least a topic and a body (though sometimes that body would be empty, so I could get behind an argument we don't need that parameter, but I still think it is useful to have). You can still set the other data members using the getters and the setters

…ions

One of the uses of request-multi is to support a scatter/gather operation.
In these cases, you might not know how many requests you are going to
receive, so you can't set expected replies. Generally these wait until
timeout and then return the results. This commit adds the ability to
support all the different use cases for request-multi

Signed-off-by: Taylor Thomas <taylor@cosmonic.com>
@thomastaylor312
Copy link
Collaborator Author

Please note that I just pushed one more commit (so we can pop it off if people hate it) to change how request-multi works. One of the uses of request-multi is to support a scatter/gather operation. In these cases, you might not know how many requests you are going to receive, so you can't set expected replies. Generally these wait until timeout and then return the results. This commit adds the ability to support all the different use cases for request-multi by adding expected replies to the request options. We don't have to support this, but when I was reviewing different use cases one more time, I was reminded of the scatter/gather operation. Just let me know if people hate it or if it isn't documented clearly

/// to resources that are not intended to be accessible to the guest. This means implementations
/// should validate that the configured topics are valid topics the guest should have access to or
/// enforce it via the credentials used to connect to the service.
set-subscriptions: func(topics: list<string>) -> result<_, error>;
Copy link

Choose a reason for hiding this comment

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

Suggested change
set-subscriptions: func(topics: list<string>) -> result<_, error>;
subscribe: func(topics: list<string>) -> result<_, error>;

Maybe? Calling it a "setter" feels weird

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1 for subscribe over set-subscriptions.

/// timeout and should instead return all of the replies received up to that point. This is to
/// faciliate use in scatter/gather operations where the number of expected replies is not
/// known.
request-multi: func(c: borrow<client>, msg: message, opts: option<request-options>) -> result<list<message>, error>;
Copy link

Choose a reason for hiding this comment

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

Suggested change
request-multi: func(c: borrow<client>, msg: message, opts: option<request-options>) -> result<list<message>, error>;
request-multi: func(c: borrow<client>, msg: message, opts: option<request-multi-options>) -> result<list<message>, error>;

I would love different data structures here

Copy link
Collaborator

Choose a reason for hiding this comment

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

Currently, request-options only has two functions:

  • set-timeout-ms, and
  • set-expected-replies.

I understand set-expected-replies isn't used by request and only request-multi, but I feel like adding multiple structures could just add some confusion.

Do you feel like there are fields missing that would warrant the separation of request-options and request-multi-options? Otherwise, I'd opt to keeping them the same 👍

Copy link

Choose a reason for hiding this comment

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

My concern is not "now" but "tomorrow." The options have two different use cases, and a copy-and-paste today could avoid a major breaking change in the future when configurations collide.

Cohesion around the use-case will avoid problematic global configuration options that the user needs to learn how to configure properly per use case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

At the same time, copy-and-paste here would mean that a host would have to copy-and-paste their implementation for, say, set-timeout-ms. This could lead to divergence in implementations that should be the same and hurt maintainability. That said, I understand what you're saying - maybe there's a middleground somewhere here...

Copy link

@yordis yordis Jul 19, 2024

Choose a reason for hiding this comment

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

Yeah, there's no question about your concern here.

Overall, let the implementor decide how much code-reusing they want to do and how the code path execution may look.

I am purely focusing on the specification detail.

@thomastaylor312
Copy link
Collaborator Author

@lukewagner and I had a good convo about the reply stuff today as well as what the portability criteria should be. I am going to noodle on that a bit and also probably get some feedback from the Wasm WG in CNCF (since they've been interested in this) on the possible options. So I might wait to review at least the request/reply stuff until we can have some conversations

handler: func(ms: list<message>) -> result<_, error>;
/// Whenever this guest receives a message in one of the subscribed channels, the message is
/// sent to this handler. The guest is responsible for matching on the channel and handling the
/// message accordingly. An Ok result indicates that the message was handled successfully and
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// message accordingly. An Ok result indicates that the message was handled successfully and
/// message accordingly. An `Ok` result indicates that the message was handled successfully and

~nit

@@ -0,0 +1,43 @@
/// The request-reply interface allows a guest to send a message and await a response. This
/// interface is considered optional as not all message services support the concept of
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// interface is considered optional as not all message services support the concept of
/// interface is considered optional as not all messaging services support the concept of

~nit

/// Replies to the given message with the given response message. The details of which channel
/// the message is sent to is up to the implementation. This allows for reply to details to be
/// handled in the best way possible for the underlying messaging system.
reply: func(reply-to: borrow<message>, reply: message) -> result<_, error>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

In reply to the parallels between handle's result and reply + your two first bullet points: The way I see it, wasi:messaging's handle is similar to wasi:https incoming-handler's handle. So, maybe, just like it, we shouldn't have a return type here. Instead, the guest should either do nothing or explicitly reply to a given message. Otherwise, unavoidably, I think we will need to have an implicit reply and we should move way from anything implicit like that.

What happens if I call reply multiple times for the same message?

First, I think we have to better define who the reply is to. That is, I don't think this should be up to the implementation like it currently says - Considering request-reply is an imported interface (and, hence, implemented by a host), why does the, say, NATS implementor decide who I am sending messages to? Instead, I think we should be clear that it goes to the topic specified in the reply message. Note, it should not go to the topic in the reply-to message because, if it did, the message would be handled by the same handler we are calling reply from and we'd get infinite recursion.

Second, when replying, the consumer is essentially acting as a producer, so, whatever happens when we reply multiple times should be decided by the handler of whatever topic that message was sent to.

If I reply to a message that was sent by send (not request) what happens? Is there any way to detect whether a message is "replyable"?

There should be no difference. Every message should be replyable. Maybe we should clarify this, if you think this could be a point of confusion.

If I forward a message (to any of send, request or reply) and that recipient calls reply, who does the reply go to?

I touched on this point in my previous reply. Currently, the interface states that this should be decided by the implementor of reply (i.e., a host), but I don't think that should be the case. Instead, we should always reply the topic specified in the reply message, so it can be handled by some other handler.

Comment on lines +39 to +40
/// Replies to the given message with the given response message. The details of which channel
/// the message is sent to is up to the implementation. This allows for reply to details to be
Copy link
Collaborator

Choose a reason for hiding this comment

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

I touched on this in my reply to Luke in another comment, but I don't think the details of which channel the message is sent to should be up to the implementation. Considering request-reply is an imported interface, it will be implemented by a host - say, NATS. Why should NATS decide where my messages go? Instead, I think we should be clear the message should go to the topic in the reply message.

/// message accordingly. An Ok result indicates that the message was handled successfully and
/// should be considered processed. If explicitly abandoning the message, the guest should return
/// error::abandoned with a message indicating why.
handle: func(ms: message) -> result<_, error>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I talked about this in my reply to Luke in another comment, but I believe the handle function should maybe no longer have a return type. Instead, implementors (i.e., guests) should either: (1) do nothing with a received message, or (2) explicitly reply.

Otherwise, I think we can't get away from implicit replies - right? 'Cause who else (other than a host) could be looking at the return type of handle?

However, note that, with request-reply being an optional interface, the handler may have to instead use send over reply if request-reply is unimplemented.

Comment on lines +8 to +12
world imports-request-reply {
include imports;
import request-reply;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we maybe also want a world where we import-request-reply and export incoming-handler?

Suggested change
world imports-request-reply {
include imports;
import request-reply;
}
world imports-request-reply {
include imports;
import request-reply;
}
world messaging-with-request-reply {
include imports-request-reply;
export incoming-handler;
}

Comment on lines +9 to +11
/// The requested option is not authorized. This could be a topic it doesn't have
/// permission to subscribe to, or a permission it doesn't have to perform a specific
/// action. This error is mainly used when calling `set-subscriptions` on a guest.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// The requested option is not authorized. This could be a topic it doesn't have
/// permission to subscribe to, or a permission it doesn't have to perform a specific
/// action. This error is mainly used when calling `set-subscriptions` on a guest.
/// The requested option is not authorized.
/// This could be because a topic was not permissive to being
/// subscribed to, or because permissions were lacking to
/// perform some other specific action. This error is mainly used
/// when calling `set-subscriptions` on a guest

~nit

/// permission to subscribe to, or a permission it doesn't have to perform a specific
/// action. This error is mainly used when calling `set-subscriptions` on a guest.
unauthorized,
/// The request or operation timed out.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// The request or operation timed out.
/// The request or operation timed out

~nit

/// to resources that are not intended to be accessible to the guest. This means implementations
/// should validate that the configured topics are valid topics the guest should have access to or
/// enforce it via the credentials used to connect to the service.
set-subscriptions: func(topics: list<string>) -> result<_, error>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

+1 for subscribe over set-subscriptions.

/// timeout and should instead return all of the replies received up to that point. This is to
/// faciliate use in scatter/gather operations where the number of expected replies is not
/// known.
request-multi: func(c: borrow<client>, msg: message, opts: option<request-options>) -> result<list<message>, error>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Currently, request-options only has two functions:

  • set-timeout-ms, and
  • set-expected-replies.

I understand set-expected-replies isn't used by request and only request-multi, but I feel like adding multiple structures could just add some confusion.

Do you feel like there are fields missing that would warrant the separation of request-options and request-multi-options? Otherwise, I'd opt to keeping them the same 👍

@danbugs
Copy link
Collaborator

danbugs commented Jul 19, 2024

Please note that I just pushed one more commit (so we can pop it off if people hate it) to change how request-multi works. One of the uses of request-multi is to support a scatter/gather operation. In these cases, you might not know how many requests you are going to receive, so you can't set expected replies. Generally these wait until timeout and then return the results. This commit adds the ability to support all the different use cases for request-multi by adding expected replies to the request options. We don't have to support this, but when I was reviewing different use cases one more time, I was reminded of the scatter/gather operation. Just let me know if people hate it or if it isn't documented clearly

Btw - I am pretty sure that some implementors (like, maybe, Apache Kafka) do not inherently support a singular request, and, instead, often operate w/ returning a batch of messages as they are designed for high throughput. I think it's possible that you can still hack together a singular request implementation, but it's worth keeping in mind that that is a anti-pattern for some implementors.

@thomastaylor312
Copy link
Collaborator Author

@danbugs So the more I look at this, the more I think having request and request-multi is what causes the problem here. As it currently stands a request-multi with expected replies set to 1 is the exact same as request, so why not just simplify it to one thing. It also avoids any problems with breaking interfaces in minor versions and any issues with implementations like Kafka

@danbugs
Copy link
Collaborator

danbugs commented Jul 29, 2024

@danbugs So the more I look at this, the more I think having request and request-multi is what causes the problem here. As it currently stands a request-multi with expected replies set to 1 is the exact same as request, so why not just simplify it to one thing. It also avoids any problems with breaking interfaces in minor versions and any issues with implementations like Kafka

I am good with unifying request-multi and request 👍

@yordis
Copy link

yordis commented Jul 29, 2024

I prefer if the spec works for n most of the time instead of 1 or n

resource message {
constructor(topic: string, data: list<u8>);
/// The topic/subject/channel this message was received or should be sent on
topic: func() -> string;
Copy link

Choose a reason for hiding this comment

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

We use CloudEvents for our IIoT machine data. A main objective is to decouple the event data from the publishing layer. Therefore, the topic in our model is in the pub/sub layer, while the data and the content-type is part of the event. Usually, the event/message is created at a different time and place than the publishing step. Also, an event can be published to multiple topics after its instanciation.

Copy link

Choose a reason for hiding this comment

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

I needed to clarify this, but I never said anything; I self-reply. Could it be an ambiguous language problem?

@danbugs
Copy link
Collaborator

danbugs commented Oct 4, 2024

I am closing this PR in favour of #24 which intends to simplify the wasi-messaging interface and hopefully remove some points of contention.

cc: @thomastaylor312 , @lukewagner , @brooksmtownsend , @devigned , @yordis , @jocrau

@danbugs danbugs closed this Oct 4, 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

7 participants