Skip to content
This repository has been archived by the owner on Oct 2, 2024. It is now read-only.

Request reply support fails #233

Closed
YoeriVD opened this issue Mar 17, 2022 · 7 comments
Closed

Request reply support fails #233

YoeriVD opened this issue Mar 17, 2022 · 7 comments
Labels
bug Something isn't working stale

Comments

@YoeriVD
Copy link
Contributor

YoeriVD commented Mar 17, 2022

Describe the bug

Request/Reply bindings are generated as normal async subscriptions and take a non existent request object as parameter

How to Reproduce

Add the following to any channel:

bindings:
      nats:
        is: requestReply
        requestReply:
          is: replier
        bindingVersion: 0.1.0

Expected behavior

Request reply behaviour is generated as methods that return the response.

@YoeriVD YoeriVD added the bug Something isn't working label Mar 17, 2022
@github-actions
Copy link

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@jonaslagoni
Copy link
Member

jonaslagoni commented Mar 17, 2022

Hey @YoeriVD, thanks for raising the issue.

The bindings for defining request/reply became so confusing for me, that I removed it from my proposal for the NATS bindings. So the behavior currently supported by ts-nats-template is on it's own.

That said I would like to support it. However, I feel like we should talk about how you should define it in the bindings to make it less complex to understand.

If we want to keep

bindings:
      nats:
        is: requestReply
        requestReply:
          is: replier

Which of the operations publish/subscribe should be for the reply and the request? Or how should we achieve it 🤔?

Keep in mind publish means others publish and your application subscribes to the subject. And subscribe means others subscribe and your application publishes to the subject.

@RubMertens
Copy link

What if the binding also describes the response schema? That would remove the confusing publish entry from the channel and it would more purely describe the nats specific request/reply functionality?

bindings:
    nats:
        is: requestReply
        reply:
            message:
                payload:
                     $ref: "#/components/schemas/GeneralReply"

Nats internally subscribes to a randomly generated inbox topic when you send a request. The request message captures that inbox address in it's meta data so you can send a message to that topic when replying. But I think it's probably better not to capture those internals in a asyncAPI document.

@jonaslagoni
Copy link
Member

jonaslagoni commented Mar 17, 2022

What if the binding also describes the response schema? That would remove the confusing publish entry from the channel and it would more purely describe the nats specific request/reply functionality?

I kinda like that yes 👍

  subscribe: 
    ...
  bindings:
    nats:
        is: requestReply
        reply:
            message:
                payload:
                     $ref: "#/components/schemas/GeneralReply"

Here I dont think reply make much sense, do you? 🤔 Do you see a combination of keywords that would make sense when used together? Or at least something we can use until it is officially supported?

@YoeriVD
Copy link
Contributor Author

YoeriVD commented Mar 17, 2022

@jonaslagoni
Perhaps response makes more sense as that seems to be the unwritten consensus in the other github issues.
Other than that, I wouldn't give it too much thought, especially as it is just something to use until we have (much needed) official support.

Thoughts?

PS: a good readme update explaining why this exists is implicit I would say.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Jul 16, 2022
@github-actions github-actions bot removed the stale label Jul 29, 2022
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Nov 26, 2022
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working stale
Projects
None yet
Development

No branches or pull requests

3 participants