-
Notifications
You must be signed in to change notification settings - Fork 11
feat(*): Updates messaging to support request-reply #21
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
Conversation
Also, I still have no idea what tool it is we use that updates all of the autogenerated HTML docs, so if someone could direct me with that, it would be great! |
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>
fdf96df
to
6231f8a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As pointed out by the comments below, some of these suggestions were previously rejected by our stakeholder review.
That said, I wouldn't mind re-evaluating depending on how folks feel - I've always been more of the opinion that our interfaces should better reflect usability.
/// Format specification for messages | ||
/// - more info: https://github.com/clemensv/spec/blob/registry-extensions/registry/spec.md#message-formats | ||
/// - message metadata can further decorate w/ things like format version, and so on. | ||
enum format-spec { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
format-spec
wasn't part of the original interface, but was added during our stakeholder review. Here's some relevant comments you may want to take a look at:
(1) #9 (comment)
(2) #9 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm...I think agree with that second comment that this should be a either a bare string or at least called content-type
as the link in the comment was about registry stuff and nothing to do with content-type. The thread in (2) suggests this but I didn't seem to understand the argument that format-spec should be the thing we use
@@ -45,7 +45,7 @@ Overall, the messaging interfaces aim to make it easier to build complex and sca | |||
### Non-goals | |||
|
|||
- The messaging service interfaces do not aim to provide advanced features of message brokers, such as broker clustering, message persistence, or guaranteed message delivery. These are implementation-specific details that are not addressed by the interfaces. | |||
- The messaging service interfaces do not aim to provide support for every possible messaging pattern or use case. Instead, they focus on the common use cases of pub-sub and push-based message delivery. Other messaging patterns, such as request-reply or publish-confirm-subscribe, are outside the scope of the interfaces. | |||
- The messaging service interfaces do not aim to provide support for every possible messaging pattern or use case. Instead, they focus on the common use cases of pub-sub, push-based message delivery, and request-reply. Other messaging patterns, such as publish-confirm-subscribe, are outside the scope of the interfaces. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding support for request-reply
was rejected during our stakeholder review - here's a relevant comment explaining why: #8 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I missed that when I was digging around. However, I think I need to do some more thinking on the reasoning there. I understand the motivation as described there for not adding it, but based on everything I've seen in real usage of the wasmcloud messaging interface and in my review of most of the common messaging systems, request/reply showed up everywhere. Let me do some more reading and come back to this, because, like you mentioned, this should reflect real world usage
Closing in favor of #23 which also incorporates some of the changes from here |
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