-
-
Notifications
You must be signed in to change notification settings - Fork 285
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: add tags field to Server Object #809
Conversation
spec/asyncapi.md
Outdated
@@ -361,6 +361,7 @@ Field Name | Type | Description | |||
<a name="serverObjectDescription"></a>description | `string` | An optional string describing the host designated by the URL. [CommonMark syntax](https://spec.commonmark.org/) MAY be used for rich text representation. | |||
<a name="serverObjectVariables"></a>variables | Map[`string`, [Server Variable Object](#serverVariableObject)] | A map between a variable name and its value. The value is used for substitution in the server's URL template. | |||
<a name="serverObjectSecurity"></a>security | [[Security Requirement Object](#securityRequirementObject)] | A declaration of which security mechanisms can be used with this server. The list of values includes alternative security requirement objects that can be used. Only one of the security requirement objects need to be satisfied to authorize a connection or operation. | |||
<a name="serverObjectTags"></a>tags | [Tags Object](#tagsObject) | A list of tags for API documentation control. Tags can be used for logical grouping of servers. |
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.
Let's avoid using API and documentation. These are just specific use cases IMHO. What about this:
<a name="serverObjectTags"></a>tags | [Tags Object](#tagsObject) | A list of tags for API documentation control. Tags can be used for logical grouping of servers. | |
<a name="serverObjectTags"></a>tags | [Tags Object](#tagsObject) | A list of tags for logical grouping and categorization of servers. |
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.
Shall we apply this change to Operation
, OperationTrait
, Message
, and MessageTrait
as well? I.e. https://github.com/asyncapi/spec/blob/master/spec/asyncapi.md?plain=1#L712
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.
That would be awesome yes 👍
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.
Okey changes applied.
However, cc @alequetzalli in case you want to take this opportunity to add, remove, fix, and or rephrase this description.
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 would be good to extend some of the official examples, like streetlights for kafka with this feature, as this use case is most common, that Kafka has multiple production servers. Could make sense to also add example description
in Tags
object.
separate question:
I think we need to mention at least in this PR that we are aware there is a feature that allows you too specify that some channel is available only on a given server (CHANNEL_NAME.servers
).
I somehow feel that people will immediately assume, they can refer not only to a server by server name but the tag too 😄
Kudos, SonarCloud Quality Gate passed! |
I don't fully get what you mean by
Good suggestion! I've pushed a new commit with the requested change.
TBH I don't see the relation between |
so with Kafka you can have a cluster of brokers. Related discussion #465 I'm pretty sure people will use
Am I really the only one (with Kafka cluster use case in mind) that thinks that people will like to do |
This is a really good point 🤔 Maybe something to consider for 3.0.0? |
is this PR resolving #654? I think yes, so please update description do we agree that this PR also solves #465? 👀 @dalelane that wanted to champion the other proposal @smoya 👇🏼 was not yet addressed
and regarding 👇🏼
I think we have an agreement that for 3.0 we want to go |
9f1ef50
to
24deced
Compare
c91ecbe
to
0570ee3
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
@dalelane can you have a final look please, ref -> #809 (comment) |
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 looks really good - I like the examples, as they are super helpful prompts for the ways that tags can be used, instead of just mechanically showing how to add a tag 👍
@smoya can you update the description accordingly so the other issue is also resolved after merge, by default? and then merge 💪🏼 |
@magicmatatjahu something we might want to support in react component out of the box after release 😉 |
Done at the same time you posted this comment ⚡ 🏃 |
@derberg I will remember :) |
Looks all good here and ready for 2.5.0, going for a merge. Thanks @smoya! |
/rtm |
🎉 This PR is included in version 2.5.0-next-spec.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Description
This PR adds support for defining Tags at Server Object. They can be used for providing metadata, including the environment such as
production
,development
, etc.Related issue(s)
Fixes #654
Fixes #465