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

[2.0.0 REVIEW] Reusing channel definitions across files is hard #415

Closed
fmvilas opened this issue Jul 21, 2020 · 11 comments
Closed

[2.0.0 REVIEW] Reusing channel definitions across files is hard #415

fmvilas opened this issue Jul 21, 2020 · 11 comments
Labels
keep-open Prevents stale bot from closing this issue or PR

Comments

@fmvilas
Copy link
Member

fmvilas commented Jul 21, 2020

It's currently very hard to reuse channel definitions across multiple AsyncAPI files. The reason is that they contain the operation definition too. An example:

channels:
  example:
    subscribe:
      message:
        payload: ...

The document above describes an example channel with the subscribe operation. If this AsyncAPI file was describing an internal broker-centric architecture, it would probably have many applications consuming messages from this channel and therefore the definition of these subscribers would be:

channels:
  example:
    publish:
      message:
        payload: ...

As you can see, the only difference is the publish and subscribe verbs and possibly some other information related to the operation, like protocol bindings, description, summary, etc. And the most important part, message should belong to the channel, not the operation.

This is a spec flaw because it's intermixing channels-related and application-related information, making it hard to extract into a separate file for reusability.

Proposal

Find a new way to define channels and operations. Possibly syntactically separated so we can leverage the $ref mechanism to improve reusability.

@cappelaere
Copy link

In that particular case, I was under the impression that we had to use the $ref that would refer to a message in a different file ("library")? A view property would seem to confuse the original intent of the application yml file to describe the application's publish/subscribe operations.

@jonaslagoni
Copy link
Member

I am just gonna spark the discussion with a suggestion to how we can solve this problem.

I suggest we change the layout of channels to be an array rather then an object and add a operation keyword to keep the channel and operation on the same object. Also I would remove the publish and subscribe operation keywords and merge the operation item object into the channel item object. This however does comes at the cost of readability, however i think this is something that should be sacrificed for usability. If we find any other keywords which are unique depending on the operation this could easily be added to the same object.

With the new setup you would define channels as such:

channels:
	  - example/: # keep the current setup
	    operation: publish/subscribe

And enables referencing for channels as such:

channels:
	  - $ref: Example.yaml # In this example nothing changes regardless of operation.
	    operation: publish/subscribe

Example.yaml

example/: # keep the current setup
	parameters: # Channel parameters
	message: # Message for the channel regardless of operation

@fmvilas
Copy link
Member Author

fmvilas commented Sep 16, 2020

That's a great suggestion, @jonaslagoni. I foresee a couple of issues at a glance:

  1. If channels is an array, it would be easy to have inconsistencies. For instance, you can have the example channel defined many different times each of them with different details. For sure, we can avoid this by restricting the presence of a channel to just one item of the array but I don't completely like it.
  2. In case we use $ref (like in - $ref: Example.yaml), there would be an inconsistency if the Example.yaml file had more than one root key, e.g., example and example2. If we point to a specific one —- $ref: Example.yaml#/example—, then there's no way to know the name of the channel once the $ref has been resolved and dereferenced. I mean, the example part would then be gone.

I've been working on a proposal some time ago but still have to find time to properly frame it. Will share as soon as possible.

@jonaslagoni
Copy link
Member

1. If channels is an array, it would be easy to have inconsistencies. For instance, you can have the `example` channel defined many different times each of them with different details. For sure, we can avoid this by restricting the presence of a channel to just one item of the array but I don't completely like it.

Yea I agree I dont like the array as well... But if we wanna stick to an Object we could maybe use some sort of key instead of channel name 🤔 ? and still move the channel name inside the object like in the example.

2. In case we use `$ref` (like in `- $ref: Example.yaml`), there would be an inconsistency if the Example.yaml file had more than one root key, e.g., `example` and `example2`. If we point to a specific one —`- $ref: Example.yaml#/example`—, then there's no way to know the name of the channel once the $ref has been resolved and dereferenced. I mean, the `example` part would then be gone.

I dont understand? Cant it be achieved as the following: Example.yaml contains:

example:
    example/: # channel, and keep the current object.
	parameters: # Channel parameters
	message: # Message for the channel regardless of operation

Which can then be referenced as - $ref: Example.yaml#/example?

@fmvilas
Copy link
Member Author

fmvilas commented Sep 21, 2020

Yes but nothing prevents you for doing something like:

example:
  example/: # channel, and keep the current object.
    parameters: # Channel parameters
    message: # Message for the channel regardless of operation
  example2/:
    parameters: # Channel parameters
    message: # Message for the channel regardless of operation

In this case, $ref: Example.yaml#/example would be referencing two different channels. See what I mean?

@jonaslagoni
Copy link
Member

But with the proposed setup this would not be a valid file though 🤔 ?

@fmvilas
Copy link
Member Author

fmvilas commented Sep 21, 2020

Sure. Just arguing that it's leaving a lot of room for failure. We should aim to make it less error prone.

@jonaslagoni
Copy link
Member

Ahh! Yes! Then I agree with you 😄

@github-actions
Copy link

This issue 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 issue, add a comment with detailed explanation.
Thank you for your contributions ❤️

@github-actions github-actions bot added the stale label Nov 21, 2020
@jonaslagoni jonaslagoni added keep-open Prevents stale bot from closing this issue or PR and removed stale labels Nov 21, 2020
@fmvilas fmvilas added this to the AsyncAPI specification 2.1.0 milestone Jan 31, 2021
@fmvilas fmvilas removed this from the Next specification version milestone May 12, 2021
@fmvilas fmvilas added this to the 3.0.0 Release milestone Sep 14, 2021
@smoya
Copy link
Member

smoya commented Nov 15, 2021

In the case #618 moves forward, would this issue be considered as fixed? cc @fmvilas

@fmvilas
Copy link
Member Author

fmvilas commented Nov 15, 2021

I'd say so, yes.

@smoya smoya closed this as completed Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep-open Prevents stale bot from closing this issue or PR
Projects
None yet
Development

No branches or pull requests

4 participants