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

Add support for messageId #458

Closed
Tracked by #735
WaleedAshraf opened this issue Nov 3, 2020 · 30 comments
Closed
Tracked by #735

Add support for messageId #458

WaleedAshraf opened this issue Nov 3, 2020 · 30 comments
Labels
🏁 Accepted (RFC 3) RFC Stage 3 (See CONTRIBUTING.md) 👻 Needs Champion RFC Needs a champion to progress (See CONTRIBUTING.md) released on @2022-04-release

Comments

@WaleedAshraf
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Currently, there is no support for a unique identifier for messages. My use case is to use messageId for validation.

Describe the solution you'd like
We have operationId. messageId would be the exact same field for messages.

Describe alternatives you've considered
For now, I'm relying on the name field and adding extra checks on it. But it's not required to be unique by specs.

Additional context
Related slack conv: https://asyncapi.slack.com/archives/C34F2JV0U/p1604399108296600
Another previous conv about adding messageId: #94 (comment)
Related request from tooling: WaleedAshraf/asyncapi-validator#25

@derberg
Copy link
Member

derberg commented Nov 4, 2020

@WaleedAshraf better copy-paste the slack conversation as we are on slack free plan and it will soon be lost

@WaleedAshraf
Copy link
Contributor Author

Yup, I also noticed that, when trying to search for an old conv.

image

@github-actions
Copy link

github-actions bot commented Jan 4, 2021

This issue has been automatically marked as stale because it has not had recent activity 😴
It will be closed in 60 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 Jan 4, 2021
@derberg derberg removed the stale label Jan 4, 2021
@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
@derberg
Copy link
Member

derberg commented May 14, 2021

@fmvilas I think we can add label that indicates this issue reached Stage 1. What do you think. @WaleedAshraf just has to open a new PR as the other one was closed some time ago, but was already approved.

@WaleedAshraf keep in mind we no longer have schema in spec repo but now it is only in https://github.com/asyncapi/asyncapi-node

to reach Stage 2 we are missing a PR on the JS Parser that enables the feature. Not everything can be done with JSON Schema. To assure uniqueness of IDs, in my opinion there needs to be a custom validation provided as we did with operationId.

@fmvilas fmvilas added the 💡 Proposal (RFC 1) RFC Stage 1 (See CONTRIBUTING.md) label May 14, 2021
@fmvilas
Copy link
Member

fmvilas commented May 14, 2021

Yeah, agree this one is on Stage 1. I've added the proper label.

@WaleedAshraf
Copy link
Contributor Author

WaleedAshraf commented May 15, 2021

@derberg in asyncapi-node I already have a pr asyncapi/spec-json-schemas#24
Do we still need a PR here? I'm still confused about how schema changes take place.

I think we need PR against the "2021-06-release" branch.

Also, do we need to update js-parser right away? It doesn't need to be? Otherwise, we can end up with the same problem that other libs/tooling blocking newer versions of the schema.

Well, I think these things will clear out with time on how/when we rollout new version.

@derberg
Copy link
Member

derberg commented May 18, 2021

@WaleedAshraf

I think we need PR against the "2021-06-release" branch.

every PR for 2.1 release must be against 2021-06-release

Also, do we need to update js-parser right away? It doesn't need to be? Otherwise, we can end up with the same problem that other libs/tooling blocking newer versions of the schema.

To get a change in the spec it must reach Stage 3 and it requires a change (approved PR) against the release branch. Have a look at the guide

We want to have at least schema and parser supporting the latest spec as the most important tools. The others can follow up independently before or after the release.

@WaleedAshraf
Copy link
Contributor Author

Got it. I don't think I'll have time any soon. So, I'll just leave it here for someone to pick and I can help out if needed.

@github-actions
Copy link

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

@joerg-walter-de
Copy link

I am coming from me looking for a message-specific operationId.

I am used to generate client code for OpenAPI specs, that is for example callable JS functions where the function names match the operationId s and the functions provide datagram / parameter validation. And obviously it would be nice for a client to be able to call sendPing(), subscribeToData() etc. that is, to have a send-function for each possible message. Having a operationId for each message would help achieving that, I guess.

Currently operationId is bound to a channel. But since it may very well be reasonable to send multiple types of messages through the same channel and since it would be good to have a message specific send-function and a message-specific handler function for incoming messages a message-specific ID but probably even a message-specific operation ID may make sense.

With OpenAPI each path/method combination must have its own operation. There is an inbalance in the hierarchy in AsyncAPI: There is no equivalent for an operation let's say for create entry.

@fmvilas fmvilas added this to the 3.0.0 Release milestone Sep 14, 2021
@smoya smoya added the 👻 Needs Champion RFC Needs a champion to progress (See CONTRIBUTING.md) label Nov 26, 2021
@smoya
Copy link
Member

smoya commented Nov 26, 2021

I'm moving back this to RFC 0 as there is no PR with spec edits. Also added Needs Champion label.

@smoya
Copy link
Member

smoya commented Nov 26, 2021

Hi @WaleedAshraf! since this RFC has been here for some time, I'm wondering if you would like to retake it and be the champion, hopefully, you have some time now.

I'm more than happy to help you moving it forward :)

@WaleedAshraf
Copy link
Contributor Author

@smoya As I remember from the last discussion, this will require changes at multiple places and now I don't have the full context. I think you can help with defining each task as a separate issue, which can be picked up by anyone. I might be able to pick those.

@derberg
Copy link
Member

derberg commented Nov 27, 2021

change needs to be done in repo where JSON Schema is and proper helper function added to JS Parser. Should not be much work to do IMHO, we can help coordinate for sure.

@smoya
Copy link
Member

smoya commented Nov 29, 2021

@WaleedAshraf As you requested, here is the list of issues so you can track progress:

@dalelane
Copy link
Collaborator

dalelane commented Jan 4, 2022

@WaleedAshraf Would you like this one included in the 2.3 release? I know it's been quiet for a little while, but it looks like it could be do-able - I'd be happy to help if you'd like.

@WaleedAshraf
Copy link
Contributor Author

Hi @dalelane
I don't think I'll have time anytime soon. :(

@smoya
Copy link
Member

smoya commented Mar 30, 2022

@WaleedAshraf We plan to release 2.4 release soon (April 2022). Do you think you will have some time for materializing this RFC?
I can help with it.

@smoya smoya mentioned this issue Mar 30, 2022
55 tasks
@WaleedAshraf
Copy link
Contributor Author

@smoya Yes, I'll have some time. Let's sync on this on Slack.

@fmvilas
Copy link
Member

fmvilas commented Mar 31, 2022

Is Waleed back? 😱 🚀 🎉

@WaleedAshraf
Copy link
Contributor Author

@smoya PR is there 🎉 . Who can review it?

@derberg
Copy link
Member

derberg commented Apr 5, 2022

@WaleedAshraf we also need an update to JSON Schema, you had a PR in the past asyncapi/spec-json-schemas#24 but you just need to create new against proper file and branch

@WaleedAshraf
Copy link
Contributor Author

@derberg Yes. I will do that once the PR for parser PR is approved. As there can be more changes required and it gets delayed, etc.

@WaleedAshraf
Copy link
Contributor Author

@derberg
Copy link
Member

derberg commented Apr 11, 2022

I think we can move this one to Stage 2: Draft

and also solution itself makes a lot of sense and can't really be done differently.
@fmvilas @dalelane

cc @smoya

@smoya
Copy link
Member

smoya commented Apr 11, 2022

I guess the next step would be working on the spec repository. Are you @WaleedAshraf happy to move forward with it? Happy to help with it if needed.

@fmvilas
Copy link
Member

fmvilas commented Apr 13, 2022

Yeah, let's move it to Stage 2 👍

@smoya
Copy link
Member

smoya commented Apr 21, 2022

@fmvilas @derberg @dalelane All PR's are aproved. Can you check and add the RFC Stage 3 label? I can only proceed with merging after that.

@derberg derberg added 🏁 Accepted (RFC 3) RFC Stage 3 (See CONTRIBUTING.md) and removed 💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md) labels Apr 21, 2022
@asyncapi-bot
Copy link
Contributor

🎉 This issue has been resolved in version 2.4.0-2022-04-release.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@smoya
Copy link
Member

smoya commented Apr 27, 2022

This issue can be now closed as it got merged and released. @WaleedAshraf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏁 Accepted (RFC 3) RFC Stage 3 (See CONTRIBUTING.md) 👻 Needs Champion RFC Needs a champion to progress (See CONTRIBUTING.md) released on @2022-04-release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants