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 messaging to support request-reply #21

Closed

Conversation

thomastaylor312
Copy link
Collaborator

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

@thomastaylor312
Copy link
Collaborator Author

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 <[email protected]>
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.

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 {
Copy link
Collaborator

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)

Copy link
Collaborator Author

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.
Copy link
Collaborator

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)

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 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

@thomastaylor312
Copy link
Collaborator Author

Closing in favor of #23 which also incorporates some of the changes from here

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

2 participants