-
-
Notifications
You must be signed in to change notification settings - Fork 290
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat: add server and channel traits objects #858
feat: add server and channel traits objects #858
Conversation
"protocolVersion": "1.0.0", | ||
"traits": [ | ||
{ "$ref": "#/components/serverTraits/kafka" } | ||
] |
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.
Would it make sense to create another example for traits? I find it confusing here.
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.
TBH I don't understand. What you wanna add here? Similar examples of using traits we have in message and operation traits with $ref
.
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.
Yeah, I know, it's just that by looking at this example I simply would not understand what a server trait is if it were my first time with the spec.
spec/asyncapi.md
Outdated
@@ -524,6 +532,42 @@ servers: | |||
``` | |||
|
|||
|
|||
#### <a name="serverTraitObject"></a>Server Trait Object | |||
|
|||
Describes a trait that MAY be applied to a [Server Object](#serverObject). This object MAY contain any property from the [Server Object](#serverObject), except `url`, `protocol` and `protocolVersion`. |
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.
Why not protocolVersion
? Also, you're missing the traits
property. Otherwise, traits could define traits that could define traits and so on.
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.
Yeah, I missed traits
🤦🏼♂️ Thanks for catch! However, not specifying protocolVersion
is deliberate, because it has a correlation with protocol
. You shouldn't define version like 0.1.0
and then in the server object define protocol: kafka
, because you don't know if you kafka
(example) has 0.1.0
version.
We could add protocolVersion
to the Server Trait Object
, but together with protocol
. However protocol
is mandatory field in the server so we will end up with this same situation as we had with Operation Object
with action
and channel
fields.
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.
I think that's completely fine. People usually use one protocol only (two when they use http/ws for frontend too). I think it's up to them to take care that protocolVersion
still makes sense after applying the trait 🤷
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.
Ok, if someone will start to combine with versioning and protocols it will be his/her problem 😄
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.
The parser should validate it's a legal combination anyway.
spec/asyncapi.md
Outdated
##### Server Trait Object Example | ||
|
||
```json | ||
{ | ||
"tags": [ | ||
{ | ||
"name": "env:production", | ||
"description": "This environment is the live environment available for final users" | ||
} | ||
] | ||
} | ||
``` | ||
|
||
```yaml | ||
tags: | ||
- name: "env:production" | ||
description: "This environment is the live environment available for final users" | ||
``` |
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.
Can you come up with a more complete example? For instance, security
and variables
would be super useful in many cases.
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.
I made such simple examples of what operation and message traits have I will extend it.
}, | ||
"traits": [ | ||
{ "$ref": "#/components/channelTraits/kafka" } | ||
] |
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.
Same as with servers. Would be cool to have another example for traits.
|
||
#### <a name="channelTraitObject"></a>Channel Trait Object | ||
|
||
Describes a trait that MAY be applied to an [Channel Object](#channelObject). This object MAY contain any property from the [Channel Object](#channelObject), except the `messages` and `traits` ones. |
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.
Why not messages
? What if I want to define another channel with the same messages but a different address?
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.
This is the same problem as with payload
in the message. You can't define payload in message traits, and likewise you shouldn't be able to add messages to the channel trait - look at my example in the PR description. The current main problem in defining channels is that we define messages on channel level (this is not a problem at all) but when we want to add a "filter" on the operation to inform user on which messages given operation operates, we need to define another channel with subset of messages - maybe this will fix that problem in more elegant way #870
Why not messages? What if I want to define another channel with the same messages but a different address?
This is some use case, but when we give the ability to define the messages at the level of the channel trait then someone can do something like this:
channels:
someChannel:
address: ...
messages:
someMessage: ...
traits:
- messages:
someMessage: ...
The ability to override information on the message from the level of the channel trait is a bad idea, that's why I want to avoid it.
Co-authored-by: Fran Méndez <fmvilas@gmail.com>
Kudos, SonarCloud Quality Gate passed! |
Would it be possible to limit the scope of this PR to |
I can, however I don't know if that PR is worth to focus for |
Closing - see the previous comment. |
title: "Add server and channel traits objects"
Related issue(s):
#618
That PR adds
Server Trait Object
andChannel Trait Object
to be able to create generic channels and servers (e.g. for registries) and extend them. The idea ofChannel Trait
was conceived while writing the #618 (comment). In the current state of spec v3, we have problems related to the creation of similar channels (with the same data), but differing in the list of available messages.Example: We can have application where we have one channel, but operations perform logic only on particular messages, so (in current spec v3) we need to define multiple channels with this same data, but with different
messages
list. When channel has onlyaddress
andmessages
fields copying is easy, but however with situation when channel has definedtags
,externalDocs
,bindings
,parameters
etc, we can have situation like:so as we see we have duplicated data like
parameters
,bindings
etc. Of course we can use$ref
but it doesn't solve problem at all, because we still needs to write a lot of code.channels[x].traits
help us with that:As we can see we remove a lot of duplicated code.
Similar situation with
Server Trait Object
s - they are not as necessary asChannel Trait Objects
, but due to the fact that we have traits for messages, operations and channels then servers should have them too.Another solution to handle above problem with redundant channel data - feat: add messages field in the Operation Object.