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

Implement Parser-API (Intent-Driven) #401

Closed
Tracked by #481
smoya opened this issue Nov 3, 2021 · 19 comments
Closed
Tracked by #481

Implement Parser-API (Intent-Driven) #401

smoya opened this issue Nov 3, 2021 · 19 comments
Labels
enhancement New feature or request

Comments

@smoya
Copy link
Member

smoya commented Nov 3, 2021

Reason/Context

Some months ago we created https://github.com/asyncapi/parser-api, an API definition all AsyncAPI parsers should eventually implement.

An introduction can be found as well in this blog post: https://www.asyncapi.com/blog/intent-driven-api

Ideally, we should implement it for the next major AsyncAPI Spec v3.0.0.

@jonaslagoni and I scheduled a Contributor-first meeting on the 6th December 2021 for having a debate around pros/cons of implementing such a thing.

The video is available here.
The slides are available here.

The list of pros/cons we ended up collecting (attendees suggested) during the meeting was written here.

Here is the summary of all pros/cons we found out, including the ones mentioned above:

Introduce intent-driven API

Positive sides:

  • User intentions/use cases/desires are expressed intentionally in this API. Meaning users, without prior knowledge, will quickly find what they need. This is one of the most important points regarding UX.
    • IDE autocompletion also helps.
  • API methods, as being described as intentions, can avoid potential breaking changes.
  • Adds a layer of abstractions, so you dont need to learn 100% of the specification..
  • Can access the raw data that follows the specification.
  • Helps everyone (not just coders) better understand the tool.

Negative sides:

  • We add another layer of "abstraction" that users need to learn.
    • You have to know/understand how the intent API maps the spec within their intent functions.
  • There can be some different interpretations between the spec and the API which forces us to clarify in the documentation. For example, the fact Operations return a list of Channels in the API, but in the spec a single Operation can only be attached to one single channel. In this case, it is done in that way to avoid possible breaking changes in the future, where we could consider an Operation can be linked to several channels.
  • If you provide the raw data, more API's to maintain.

Introduce parser-api

Positive sides:

Negative sides:

  • New things we must align on, such as governance of new parsers and how compliant parsers are.
    • Also if someone wishes to donate parsers, should they be compliant with the parser-api first?

Keep normal API

We can still add support functions to ease use-cases such as allMessages() as the current parser-js support.

Positive sides:

  • Simple to understand for anyone who knows the spec, because learning the spec structure will be reflected in the code 1:1
    • More easy to learn

Negative sides:

  • You have to know/understand the spec for absolutely all the API operations you perform.
  • Always breaking changes (when the spec have a major version change, the API will also have)
  • No backward compatibility.
  • Usage is more complex for basic operative. I.e. get all messages the application uses receives.
  • Basically all positive sides of the intent-driven API section.

Description

Focus should go only into supporting v3.0.0. However, retro compatibility might be a thing.
More info at asyncapi/parser-api#26

As a side note, there is already a WIP implementation in Go here https://github.com/asyncapi/event-gateway/blob/master/asyncapi/v2/v2.go.

As per the data collected, it seems implementing this new Intent-Driven API as the Parser-API gives more benefits than cons, specially for the mid and long term. So the community can count me in for moving this forward if we decide we should do.

Now the question is... Shall we move forward? I need your help!

cc @fmvilas @magicmatatjahu @jonaslagoni @derberg as codeowners.
cc @KhudaDad414 @lbroudoux @rmelian @BOLT04 @iamdevelopergirl @aayushmau5 as randomly picked individual contributors.

@BOLT04
Copy link
Member

BOLT04 commented Dec 29, 2021

Hi @smoya 🙂, I don't have much context about what this is about, but through the links and bullet points, I got a better grasp of the purpose of an intent-driven API. Here are a few key topics I wanted to share.

I think having the parser-api is really helpful to ensure compatibility between languages 👍. parser-js is the most know currently, but in order to grow and support C#, Go, Python, Java, etc, it would be great to have docs on best practices, guidelines, and other content to make it easier for all parsers to implement the spec with the same considerations.
About all these docs and explanations for contributors, it would be amazing to have your input @alequetzalli 🙂. Perhaps structure a "Get Started" with a step-by-step tutorial, or a FAQ with common considerations like handling multiple schemas, dereferencing, I don't know 😅.

API methods, as being described as intentions, can avoid potential breaking changes.

Breaking changes to the spec propagating to tooling is indeed a tough problem. But I'm not sure I understand the solution to this. For example, if we are talking about the method clientPublishOperations on the current parser-api docs, if the spec changes publish to send, would the method still be called clientPublishOperations?
The parser implementation might need to use the new send concept, but I get that the interface doesn't change so there would be no breaking changes, which is great! But wouldn't we eventually want to update to clientSendOperations?
I guess my question here is if we want to introduce this abstraction layer on the parser-api, so that the client simply understands what publish means? Would having this difference in the words used in the interface and the spec confuse users and contributors?

I'd love any feedback on my questions, since I might be missing the solution to all of this 😅

@smoya
Copy link
Member Author

smoya commented Dec 29, 2021

About all these docs and explanations for contributors, it would be amazing to have your input @alequetzalli 🙂.

That would be awesome!🤞 I have to say that we had the pleasure to have @alequetzalli in the Contributor first meeting.

But wouldn't we eventually want to update to clientSendOperations?

Yeah, exactly. But we won't swap to the new method immediately but progresively, deprecating the old one and keeping support for the deprecated one for few versions.
Thats a key difference between the current implementation, which is mostly tied to the JSON representation of the doc.

@BOLT04
Copy link
Member

BOLT04 commented Dec 29, 2021

ahh got it, yes it makes sense to progressively update the methods. Thanks for helping 😃
I think this should go forward and start thinking about the v1 of intent-driven API and Parser API, supporting AsyncAPI spec v3 👍

@magicmatatjahu
Copy link
Member

I had a question about the send and receive operation, but David outdid me 😅 Speaking of breaking changes, I have a few comments about the implementation itself:

  • how long will it take to deprecate methods? If breaking change occurs, are we planning to keep old methods for one major version and then remove them? This approach is e.g. in Angular, if a new version comes out, they define what is deprecated and remove it in the next version. Maybe we should have the same, or extend the deprecation period for 2 versions and remove in 3?
  • I hope that the new API will force to rewrite the parser to TS :)

@derberg
Copy link
Member

derberg commented Jan 3, 2022

We add another layer of "abstraction" that users need to learn

It only affects people that know the spec in details...so not many folks

Introduce parser-api

yes yes yes, consistency across parsers would be awesome, would make many things much easier long term. I'm just not sure if it has to be super perfect now if we should not focus on implementing it first in JS Parser and then update Parser-API based on our learnings.

Introduce intent-driven API

I have nothing against the idea, I'm just concerned about asyncapi/parser-api#4

IDE autocompletion also helps

we should take advantage of it definitely.

in short: I do not want clientSubscribableMessages because my intent is to get messages that I want to subscribe to, and not get client subscribable message. Do you know what I mean?

for example in here https://github.com/asyncapi/nodejs-ws-template/pull/65/files

I still need to:

{%- for channel in asyncapi.channels() %}
{%- for operation in channel.operations() %}
{%- if operation.isApplicationSubscribing() %}
console.log('* {{ channel.path() }}');

To get channel path. But my intent is to get a channel path that an app can subscribe to:

{%- for channel in asyncapi.channels('subscribe') %}
console.log('* {{ channel.path() }}');

IDE is there to help me understand what options I have for channels function and what they mean and which one I should use

as a template developer, I would really prefer as this is what I'm actually looking for

{%- for path in asyncapi.channelsPaths('subscribe') %}
console.log('* {{ path }}');

now yes, subscribe is pretty spec specific, but operation.isApplicationSubscribing() too,
look at latest proposal when we have send and receive actions. I would also need to do some updates to my code. asyncapi.channelsPaths('subscribe') would require some refactor before subscribe gets deprecated.

It could also be just a config that you pass to the parser, like mapping of actions, what they mean 😆

I really think things like clientSubscribableMessages are super not intuitive, and another client/application confusion comes into play 😞

I hope that the new API will force to rewrite the parser to TS :)

there always need to be some TS fanatic in the group

@fmvilas
Copy link
Member

fmvilas commented Jan 3, 2022

subscribe is pretty spec specific, but operation.isApplicationSubscribing() too

@derberg I don't think isApplicationSubscribing() is specific to the spec and the "subscribe" meaning. I mean, anyone doing EDA would understand what this function does, without knowing AsyncAPI.

I really think things like clientSubscribableMessages are super not intuitive, and another client/application confusion comes into play 😞

Yes. clientSubscribableMessages is not an option IMHO. However, I don't have a better alternative, to be honest. I just think if we make it "hard to guess" for someone, it has to be for people using older versions, i.e., v2.x. That said, I think this API design should be designed along the lines of v3. Don't get me wrong, I'm not saying we should map it to v3. I just think it should play well with v3. In other words, DX should be great for v3 users and gracefully degrade for v2 users.

@smoya
Copy link
Member Author

smoya commented Jan 4, 2022

  • how long will it take to deprecate methods? If breaking change occurs, are we planning to keep old methods for one major version and then remove them? This approach is e.g. in Angular, if a new version comes out, they define what is deprecated and remove it in the next version. Maybe we should have the same, or extend the deprecation period for 2 versions and remove in 3?

Yeah, most of frameworks do it in that way. And this is what originally @jonaslagoni and I thought about. No clear strategy if deprecations should stay there for one or two versions as you mentions. Worth to discuss. We already have an issue for discussing about how to keep a compatibility matrix where this can also be in the same topic: asyncapi/parser-api#25. I would love to see more people involved!

  • I hope that the new API will force to rewrite the parser to TS :)

I hope that happens at some point, same for Generator as mentioned by asyncapi/community#212 :D

@smoya
Copy link
Member Author

smoya commented Jan 4, 2022

yes yes yes, consistency across parsers would be awesome, would make many things much easier long term. I'm just not sure if it has to be super perfect now if we should not focus on implementing it first in JS Parser and then update Parser-API based on our learnings.

I think finding a perfect balance between design (parser-api) and implementation (parser-js) can be achieved so we can design the API while we validate it.

IDE is there to help me understand what options I have for channels function and what they mean and which one I should use

Besides my answer on asyncapi/parser-api#4 (reply in thread), I want point out that not all the programming languages have native support for enums (subscribe, publish). It introduces an extra complexity we should document in parser-api (what are the possible values? should the method return an error and which one in case of invalid value, etc).
I'm not saying it does not introduce benefits because I would be lying. The number of methods to define and generate are much less (that half on those that need that "perspective" prefix) but here what it matters is the DX of the API the user will use, so I would only be concern on that side (aligned with your main concern, so 👍 ).

I think it's worth to move this discussion to asyncapi/parser-api#4 and reactivate it by involving more people.

@jonaslagoni
Copy link
Sponsor Member

jonaslagoni commented Jan 4, 2022

I think it is time we should start implementing it yes 👍 I think if we keep the implementation in the next branch that will enable us to do the same with the generator. That way we can start working towards figuring out the best namings conventions and further refurbish the intents, while we can actually use them in tooling.

What do you think about that approach?

Yes. clientSubscribableMessages is not an option IMHO. However, I don't have a better alternative, to be honest. I just think if we make it "hard to guess" for someone, it has to be for people using older versions, i.e., v2.x. That said, I think this API design should be designed along the lines of v3. Don't get me wrong, I'm not saying we should map it to v3. I just think it should play well with v3. In other words, DX should be great for v3 users and gracefully degrade for v2 users.

I agree, that it would be good to match it with v3, especially since the actions send and receive play better together with the intents.

I really think things like clientSubscribableMessages are super not intuitive, and another client/application confusion comes into play 😞

Bottom line is that naming of intents is hard to do as you point out @derberg, I think the specifics are out of scope for this issue. The current intents serve nothing but examples to give an idea about what they might be 🙂 As we start using them in implementations, we will start to learn what is missing and what not working and what is.

@derberg
Copy link
Member

derberg commented Jan 11, 2022

TBH my understanding was that we agreed to do Parser API few months ago and that now you just want to push forward the API proposed back then 😅

Not a single codeowner of Parser JS objects intents-driven API. Just go for it and let us argue on specific things on PR level.

I think if we keep the implementation in the next branch

I agree with @jonaslagoni that this is the way to go, it works in other repos, and we have release infrastructure ready 👍🏼

Besides my answer on asyncapi/parser-api#4 (reply in thread), I want point out that not all the programming languages have native support for enums (subscribe, publish)

might be it will disqualify idea and Parser API will in the end just be a set of guidelines. I don't think any of us imagines it is possible to satisfy all programming languages on API design level. First, most important is that JavaScript developers must be happy to work with the API the way they are used to in JS, and consistency with other languages is not going to be a good argument for them.

I mean, anyone doing EDA would understand what this function does, without knowing AsyncAPI.

if this would be true @fmvilas, you would not drop these in your proposal for v3 😄
no matter what words we choose, it will require vocabulary and some initial learning from library users

@fmvilas
Copy link
Member

fmvilas commented Jan 12, 2022

if this would be true @fmvilas, you would not drop these in your proposal for v3 😄
no matter what words we choose, it will require vocabulary and some initial learning from library users

@derberg agree it requires vocabulary and initial learning but I meant "isApplicationSubscribing" is not confusing. It has nothing to do with our meaning of publish and subscribe. It's common EDA vocabulary.

@felixjung
Copy link

I kind of agree with @derberg that tooling should not try to fix what's wrong with the spec. I know there are other proposals and issues regarding the topic of perspective on publish and subscribe operations. Thinking about this now, I feel the main problem is that the spec takes a perspective of the consumer or producer of messages to begin with.

If you look at OpenAPI, then you see that what you describe is the HTTP endpoints and the data. You don't describe who does what (yes, there is a list of servers in there, though). For example, a GET endpoint on the route /api/users lets a client receive a list of users and requires some server to provide that list. If I need to obtain user data, I need to implement a client to that API. I don't care which application is the server. All I care about is that I can obtain the data I need from an endpoint, but making a GET request.

Taking this to AsyncAPI, we can establish the same principles. There is a channel that carries certain messages. If there is a user channel and I want to obtain user data, I implement a consumer. I don't care which service produces the messages. One could try and argue that this becomes more difficult when you consider messages being used as commands. You have to say which application receives these commands, no? But I don't think so. There is a command that requires certain data and will be interpreted by a service. As an issuer of the command, I don't care which service interprets it, because the spec describes the effects of the command.

So in summary, I would maybe focus on channels and messages rather than who produces/consumes the messages to/from those channels. If there is a need to describe the roles of consumers and producers, then maybe a separate schema or subschema can describe these relations.

Sorry if all this has been discussed and considered somewhere else already.

@smoya
Copy link
Member Author

smoya commented Feb 18, 2022

@felixjung Your point is not only very valid and useful. It is also in the direction of some of the discussions people are having in asyncapi/spec#618 (comment). So I encourage you to join the issue, read the comments thread (starting from the one I linked here) and add your point in there.

You might be also interested in the following issues (even though are not 100% related to this topic):

Thanks for your help!!!

@felixjung
Copy link

Haha, yeah #618, that's the one I remember reading at some point when I complained I didn't understand what an operation was in a channel with multiple message schemas. 😂 Thanks, will do the reading and posting part.

@smoya
Copy link
Member Author

smoya commented Mar 18, 2022

For the reference, #453 it is considered now the implementation to follow of the Parser-API

@smoya
Copy link
Member Author

smoya commented Apr 1, 2022

This task is under development, and all PR's should target next-major branch, where also Migrate project to TypeScript is happening.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added stale and removed stale labels Jul 31, 2022
@smoya
Copy link
Member Author

smoya commented Aug 1, 2022

No way github-actions!! This is in development actually! #481

@magicmatatjahu
Copy link
Member

Issue is resolved in v2 ParserJS - now in https://github.com/asyncapi/parser-js/tree/next-major branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants