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

docs: initial NATS bindings #6

Merged
merged 13 commits into from
Aug 6, 2021

Conversation

jonaslagoni
Copy link
Member

@jonaslagoni jonaslagoni commented Nov 5, 2019

This PR introduces the initial minimalistic NATS bindings.

Fixes #5

@jonaslagoni
Copy link
Member Author

(question for my self or if any of you can help out!) I am not quite sure whether sequences can be used with queues or if there is a usecase for this and how to define it.

Added queue and request reply properties.
Added operation unsub after n message option.
Copy link
Member

@fmvilas fmvilas 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 not sure about sequences neither. I haven't tried NATS yet myself.

nats/README.md Outdated Show resolved Hide resolved
@jonaslagoni
Copy link
Member Author

While trying to create the template for typescript, I realized that there was missing a property explaining if the request reply is of type replier or requester. Given these 2 opposite channel specifications.
Client 1:

channels:
  smartylighting.streetlights.1.0.action.{streetlightId}.dim:
    description: Get notified when a streetlight should be dimmed. This should only be handled by 1 subscriber but ensured by request reply.
    parameters:
      streetlightId:
        $ref: './components/parameters.yml#/streetlightId'
    publish:
      operationId: dimLightResponse
      description: The response returned by us.
      message:
        $ref: './components/messages.yml#/dimLightResponse'
    subscribe:
      operationId: dimLight
      description: The dim light request received from the frontend.
      message:
        $ref: './components/messages.yml#/dimLight'
    bindings:
      $ref: './components/channel-bindings.yml#/smartylightingDim'

Client 2:

channels:
  smartylighting.streetlights.1.0.action.{streetlightId}.dim:
    parameters:
      streetlightId:
        $ref: './components/parameters.yml#/streetlightId'
    subscribe:
      operationId: dimLightResponse
      description: The response returned by the light controller.
      message:
        $ref: './components/messages.yml#/dimLightResponse'
    publish:
      operationId: dimLight
      description: The dim light request send to the light controller.
      message:
        $ref: './components/messages.yml#/dimLight'
    bindings:
      $ref: './components/channel-bindings.yml#/smartylightingDim'

Where client 1 is the replier and client 2 is the requester (opposite publish and subscribe message)

My question: Can you think of any other way to figure out if it is the replier and requester for the request reply message? I dont want to add an "unnecessary" variable if it can be avoided.

@fmvilas
Copy link
Member

fmvilas commented Nov 13, 2019

It's a client-server model so I'd not create 2 files but just the server one and make the client use it.

For instance, this would be your server definition:

channels:
  smartylighting.streetlights.1.0.action.{streetlightId}.dim:
    description: Get notified when a streetlight should be dimmed. This should only be handled by 1 subscriber but ensured by request reply.
    parameters:
      streetlightId:
        $ref: './components/parameters.yml#/streetlightId'
    publish:
      operationId: dimLight
      description: The dim light request received from the frontend.
      message:
        $ref: './components/messages.yml#/dimLight'
    subscribe:
      operationId: dimLightResponse
      description: The response returned by the server.
      message:
        $ref: './components/messages.yml#/dimLightResponse'
    bindings:
      $ref: './components/channel-bindings.yml#/smartylightingDim'

This way you only have one file describing what the server allows clients to do, i.e. publish or subscribe.

@jonaslagoni
Copy link
Member Author

jonaslagoni commented Nov 13, 2019

hmm, I see your point, I just dont think you can divide it like such, what if both clients must act as a server and client i.e (this is a use case I face):
Client 1:

channels:
  smartylighting.streetlights.1.0.action.{streetlightId}.off:
    parameters:
      streetlightId:
        $ref: './components/parameters.yml#/streetlightId'
    subscribe:
      operationId: turnOffLightResponse
      message:
        $ref: './components/messages.yml#/turnOffLightResponse'
    publish:
      operationId: turnOffLightRequest
      message:
        $ref: './components/messages.yml#/turnOffLight'
  smartylighting.streetlights.1.0.action.{streetlightId}.dim:
    parameters:
      streetlightId:
        $ref: './components/parameters.yml#/streetlightId'
    publish:
      operationId: dimLightResponse
      message:
        $ref: './components/messages.yml#/dimLightResponse'
    subscribe:
      operationId: dimLight
      message:
        $ref: './components/messages.yml#/dimLight'

Client 2:

channels:
  smartylighting.streetlights.1.0.action.{streetlightId}.off:
    parameters:
      streetlightId:
        $ref: './components/parameters.yml#/streetlightId'
    publish:
      operationId: turnOffLightResponse
      message:
        $ref: './components/messages.yml#/turnOffLightResponse'
    subscribe:
      operationId: turnOffLightRequest
      message:
        $ref: './components/messages.yml#/turnOffLight'
  smartylighting.streetlights.1.0.action.{streetlightId}.dim:
    parameters:
      streetlightId:
        $ref: './components/parameters.yml#/streetlightId'
    subscribe:
      operationId: dimLightResponse
      message:
        $ref: './components/messages.yml#/dimLightResponse'
    publish:
      operationId: dimLight
      message:
        $ref: './components/messages.yml#/dimLight'

Is it because I define the async api definition the wrong way and this could never be the use case?

@fmvilas
Copy link
Member

fmvilas commented Nov 13, 2019

If you do publish and subscribe on the same channel, then your publishers are going to receive the messages they publish. Also, you'd be putting 2 different types of messages on the same channel. I'm curious what's the real use case behind this because with the streetlights example it doesn't seem to make sense to me.

@jonaslagoni
Copy link
Member Author

Full use case: A backend service (lets call it backend1) which processes information from input service (lets call it inputService1). Backend1 produces some data which another service (backend2) consumes. Which means that backend1 would need to be a client and a server at the same time. Here I would love to not to generate 1 client and 1 server code for backend1 but only 1 😄 For me to do this, I would need in the request reply channel to define if it should act as the server or the client.

@fmvilas
Copy link
Member

fmvilas commented Nov 13, 2019

Ok, I see where you're going. In my opinion, it's a problem of the code generator, which should support handling many AsyncAPI files probably. If we add support for this in the NATS binding, it would be possible when using NATS but not when using other protocols, so that makes me think that this should be solved in either the spec or the code generator.

@jonaslagoni
Copy link
Member Author

Hmmm, yea, I guess the question comes down to (not NATS specifically, since other protocols use them as well) whether request/reply should be supported the same way pubsub is in the spec.

@fmvilas
Copy link
Member

fmvilas commented Nov 13, 2019

Yes, that's another issue: asyncapi/spec#94.

@jonaslagoni
Copy link
Member Author

I guess I will use the requestReply.is type for now, until this gets resolved in the spec 😄 For the templates I will use a filter for getting this information so it can easily be applied to a new version of the spec.

@fmvilas
Copy link
Member

fmvilas commented Nov 13, 2019

Agree. Let's be pragmatic here.

nats/README.md Outdated Show resolved Hide resolved
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity 😴
It will be closed in 30 days if no further activity occurs. To unstale this pull request, add a comment with detailed explanation.
Thank you for your contributions ❤️

@jonaslagoni jonaslagoni marked this pull request as ready for review July 27, 2020 10:26
@jonaslagoni jonaslagoni requested a review from fmvilas July 27, 2020 11:03
@jonaslagoni jonaslagoni requested a review from derberg July 27, 2020 11:03
@fmvilas fmvilas removed their request for review January 5, 2021 09:58
@jonaslagoni jonaslagoni changed the title NATS bindings docs: NATS bindings Mar 8, 2021
@jonaslagoni jonaslagoni changed the title docs: NATS bindings docs: initial NATS bindings Jul 30, 2021
@jonaslagoni
Copy link
Member Author

Thought it was time to finish this PR and making it as minimal as possible, so use-cases will push the changes.

  • Removed any mentioning of request/reply as it should be handled natively.
  • Removed unnecessary properties that are more application configurations than design properties.

Copy link
Member

@fmvilas fmvilas left a comment

Choose a reason for hiding this comment

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

Yeah, I think it's a good decision to keep it simple and let use cases drive it.

@jonaslagoni jonaslagoni merged commit f380502 into asyncapi:master Aug 6, 2021
@jonaslagoni jonaslagoni deleted the feature/nats_binding branch August 6, 2021 08:14
@derberg
Copy link
Member

derberg commented Aug 6, 2021

@jonaslagoni don't forget you need to add it to the spec and json schema if you want it to be available in 2.2 September release.

Feature branches 2021-09-release are created.

@jonaslagoni
Copy link
Member Author

@jonaslagoni don't forget you need to add it to the spec and json schema if you want it to be available in 2.2 September release.

Feature branches 2021-09-release are created.

Will do 🙂

@derberg
Copy link
Member

derberg commented Sep 7, 2021

@jonaslagoni small hint, we have September already 😆

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.

[FEATURE REQUEST] Specifying NATS bindings
4 participants