-
-
Notifications
You must be signed in to change notification settings - Fork 8
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
Define Parser(s) API #90
Comments
Hey! You've labeled this issue as a Scope. Remember you can use the following command to inform about its progress:
or
Example:
🏋️♀️ See the progress on the Shape Up Dashboard. |
Well, this is quite a hard nut to crack... Since this is the end of the workday I am just posting my initial thoughts on the task and potential solutions. Different kind of signatures that we could implement, this should be followed throughout the API to ensure consistency:
i.e.
i.e. We also know that the intent will most likely always be present. Thoughts:
|
I was not aware of it. What is the use case for having no messages for an operation?
I'm not fully convinced of this approach. I know it was mentioned by @fmvilas in #82, However, I think it will carry on with the same issues the current API has. asyncapi.message('operationID') // returns all messages for a given operation
asyncapi.message('operationID', 0) // returns the first message for a given operation
asyncapi.message('operationID', 0, 3) // returns the first and the third message for a given operation
Variadic functions (spread syntax in >=ES6) would not cause that issue if combined with rest parameters |
Because publish:
message:
oneOf: []
Hehe, I am not fully convinced about this approach either, I just wrote down my thoughts from yesterday, and I agree that we should use the minimum args as possible for all functions throughout the API. The only reason why I like the second approach more in general, if the intent is to get the If your intent is to get the payload of the message, getting it by using That being said, we do have intents which wants to get the message object itself (#84 (comment)) therefore we they must be able to go through that as well. It is just more prune to being obsolete. And this I think will probably be one the hardest tasks to figure out where that boundary is with intents 😄 Since some of them are really, really close to the actual setup of the AsyncAPI spec.
What I mean is that if we have a function which takes multiple arguments of same type it can be a problem distinguish them from one another. But maybe we wont have that problem, lets see 😄 |
From Slack thread, it seems there are some valid use cases for having zero messages. |
We do not aim for having the silver bullet API but rather something simpler and more resilient than what we have. |
I have a kind of different approach which is somewhat the same structure as we have now. I don't think we can make the API without agreeing on the underlying structure of the calls. For example We need to make assumptions, assumptions that never change, if they do, a new major version of a parser is required. These assumptions are that regardless of how the specification declares the structure, the parser API will contain the same underlying structure. I tried to keep the API structure as close to the current API (parser-js) as possible and are actually able to do so. I did this since the current structure makes sense from a tooling perspective and can be altered to support changes to the specification. Descriptions for the API structure:
Suggested API structure
I have not included all intent functions in this suggestion, many can be thought of such as I am not 100% sure all changes to the underlying specification will be possible to "safe guard" here. But that is yet to be determined. But Jonas, that is the same structure as we have now, what changed? Yes it is (somewhat) the same structure that we have now, the only difference is that you have to change the way you think about it. The API structure is now completely detached from the underlying specification structure, and remains the same regardless of changes to the specification. Any thoughts or doubts about this underlying structure? Does it make sense? 🤔 |
Just to mention, there is a request to add messageId to spec. asyncapi/spec#458 |
Thanks, @jonaslagoni for your great work on that suggestion. You made a really great point:
If we stick with that as our main goal, we could reach an API that will be (quite) resilient to breaking changes. I'm gonna try to throw some ideas about what I think the approach can be (based partially on what Fran mentioned). We have to first find who are the potential users of the parser.
As you can see, those intents could be transformed into actual API methods and they will be valid (almost) forever. From the application point of view: asyncapi.messagesTheApplicationPublishes() From the client point of view: asyncapi.messagesAClientCanSubscribe() The interface is gonna stay unbreakable between versions, unless a really big refactor happens on the spec, such as a completely redefinition or renaming. I can't ellaborate a more detailed suggestion right now as I'm reaching my EOD here. Will keep iterating next Monday. |
/progress 10 We have been looking for different API approaches and added a couple of suggestions. We are looking for setting the API foundation based on intents. |
Thanks for the feedback @smoya!
How come 🤔? For the intents This is what I mean by the underlying structure of the API, we still need structure behind the scenes regardless of what intents we have. Therefore I suggest that we decouple the parser API from the spec in a way that makes sense and is resilient to changes (i.e. we iteratively use your suggestive changes to the spec as validating it wont break the API, if it does we then have to revise it and see if we can make it more resilient). |
Yes, we would need to keep the getters for each field. For example, Does it make sense? WDYT? |
I think we are beating around the bush and talking about the same thing 😆 When we talk implementation it means that we need a file for The link from the root object Also This still means that we can have intents such as Does this make sense or? 🤔 |
this thread is getting long 😄 and I have to be honest I didn't read all. My question to proposal based on example: why do I have to do also small comment to proposal that it would be useful to see in what arguments of the function are optional |
@derberg yea, sorry 😅
You won't "have to", what I proposed is the basic structure that needs to be in place for you to iterate messages, servers, channels, etc. And then what getter methods you will have access to from those objects. Intents are as I said a pathway to how you come from A to B. In order for We can after the basics are down add better intent functions such as those you describe there to better access the underlying properties. |
That's not true. You are not implying the structure behind but just the intention. This is a clear example of thinking out of the box. We need to focus on the real user intention which is: "I need that Kafka binding". The user should not case about the data structure behind, it does not matter if the binding lives under server or whatever. |
You are basically declaring your intent in there. This will totally fit within the suggestion I made. 👍 |
I'm gonna try to come up with a set of clear examples on the approach I suggested. |
I'm not implying anything here 😛 I just want to know what are bindings for my server. I simplified it as of course I can have more than 1 server so I should be able to specify the server name, etc. It would not change my point though API just implies that I have access to 3 different bindings This is my understanding of intend. Like here https://github.com/asyncapi/nodejs-ws-template/blob/master/template/src/api/services/%24%24channel%24%24.js#L31, I just want the payload type, |
@magicmatatjahu @jonaslagoni and @smoya (me) had an open meeting where we discussed the following topics:
The recordings of the meetings can be downloaded here: |
Next steps on this would be:
|
/progress 25 We have been sharing different approaches and it seems we have one that can be used as base: However, several good points were made on #90 (comment) and we are waiting for another pass by @jonaslagoni based on the intents we recap in #84. This will be useful for validating the suggestion. |
I still need some time to look at it closely but for no I found a duplication here:
and because of it, you are missing |
@derberg it is not a duplicate, one uses |
This one is not going to age well. If we end up adding support for Message Id (and most probably we will) we'll also want to have a function like
What about
What about
Watch out! If we change the meaning of "publish" and "subscribe", we're screwed here. Try to capture a bit more the intent. For instance, something like:
These four methods will: a. Express the intent of knowing if my application is subscribing to a channel. Our implementation will look for
Same as above. This signature will break if the meaning of publish/subscribe changes. I'm assuming
Same as above.
Don't assume channels have operations. It's a mistake we did in the past. This issue will most likely decouple channels and operations.
This method most likely belongs to Operation instead. Conceptually (forget about AsyncAPI here), a channel does not have any operation associated. Instead, an operation has a channel associated to it. In general, I think you also should avoid explicit mentions to
Typo. It's SecurityScheme.
I think this one is missing the operation id. |
Couldn't read all of the above. But I'm thinking there should also be something: All of them will be like, |
Just FYI I am taking all of your thoughts into consideration and gonna write another suggestion based on those instead of addressing them one by one. |
How are you gonna differentiate the argument from one function signature to the other if they have the same type? |
You can do a couple of things behind the scene to check whether it is an operation type/operation id, but based on @fmvilas reply it seems like we need to remove |
I don't see now the benefit of moving the property name you wanna get to the arguments. |
On one side, I would like to suggest we remove the I also wanna make a formal suggestion based on some of the methods I added in https://gist.github.com/smoya/1d81a9556b8756c883c44ce873b645f1 :
So we can break the confusion barrier of asyncapi/spec#520 and reduce the magnitude of the Operation object, becoming less important for the user. Considering those intents could also apply in the context of each collection (e.g. Channels array), I'm thinking if it would make more sense, from a pure technical point of view, to avoid using arrays and instead use iterable objects, so each array would become an object of a class with itents as methods. class Channels {
constructor(channels) {
this.channelsArray = channels;
}
[Symbol.iterator]() { return this.channelsArray.values() } // Not sure if this is exactly how you do an iterable
channelsApplicationPublishesTo() {
}
// ...
} |
I'm missing functions that would be used in templates when rendering multiple files (with file templates for example). Basically what would, for example,
why not just |
Would you do this for all the fields within the objects? |
I would say yes, just looking at the current structure of the operation, we anyway would like to have dedicated functions for getting a binding basing on operationId or getting a message the same way, so why not having for other fields current template code (warning: some Nunjucks stuff ahead):
what about:
in templates, we iterate a lot over arrays, just to pick up something by an id, then why not having helpers that just return only Ids collections? JS for a reason exposes API like
|
In the case of Operations, I think getting operations from the root level (by root level I mean the methods on the I was thinking about segregating intents and methods across models rather than add all of them at the same root level. I think @jonaslagoni will add more light on this soon as we have been trying to elaborate an assumption based on that, just to make things simpler, especially for newcomers. |
As a side comment, I think it will be easier to move this conversation to a PR. Easier to leave comments on lines and have discussions there. Plus, once you make changes, we all will see what has changed. |
@smoya and I have added a PR to the parser with an updated suggestion for the API - asyncapi/parser-js#263 We are keeping it in the PR as you suggested @fmvilas to enable better feedback. |
/progress 40 |
/progress 60 we have defined the mock intent API and started integrating it with tools to validate the API holds up. |
/progress 70 New API validated against markdown-template. Diff of changes to adapt it to the new Parser API can be found here |
Also new methods has been suggested as addition to the API definition that were required by the markdown-template: jonaslagoni/parser-js#1 |
Those are gonna be the next steps:
|
I have been investigating a solution for defining, with a kinda metalanguage, our API definition so it can be given to developers of new Parser implementations (Go, Rust, whatever). An incomplete example of using PlantUML dsl for generating a UML for our API can be found here As alternative, We could keep a document similar to https://github.com/asyncapi/parser-js/pull/263/files#diff-b57590968a12cee85a37c1b91d8cc7092cd8b68e50b242c39a178121db82a797 as Development guidelines for new Parser implementations, and instead, use the generated TS types for specific JS users. Any ideas are welcome anyway. |
/progress 75 We have been added new intents to the API documentation so we cover all the use cases we consider validated. |
As far as I see, next steps are:
|
/progress 90 We updated the documentation of the new Intent-driven API and added all missing intents. |
/progress 95 We have created a new repository that will contain the new Parser(s) API. There is a PR on it with the new intent-driven API specification and some design documents: asyncapi/parser-api#1 |
/progress 100 Parser API v1.0.0-alpha got documented: https://github.com/asyncapi/parser-api 🎉 🎉 🎉 🎉 |
We want to start defining the new Intent-Driven API for the parser(s).
Based on the current API Intents, define an API style having in mind the following (at least) requirements:
1. Intent-driven
Focus always on the final user intention rather than on a particular need on the code.
Examples:
publish
). This could look likeasyncapi.getChannelsByOperation('PUBLISH')
asyncapi.getMessageFor('mychannel', 'subscribe').payload()
2 Versioned
Introducing API versioning lets users stick to a version and ensuring their code will keep working.
3. Resilient to breaking changes
API breaking changes are gonna happen. However, we want to let users embrace those smoothly.
API methods should be written using signatures that can ensure compatibility within 2 versions or more.
Example:
Accessing the payload of a message in a
subscribe
channel looks like the following today:Let's imagine we want to rename the operations
publish/subscribe
topub/sub
.As this is tied to the nature of the data structure behind it. It is completely coupled to the structure of the AsyncAPI document.
Any change on such structure (changing deep, order...) will introduce a breaking change on the API.
In case we want to support the old method for a while (as deprecated), will mean we would maintain two methods on our API:
subscribe()
(deprecated)sub()
Resilient version
Will turn into
This introduces two pros:
asyncapi.getMessageFor('mychannel', 'subscribe').payload()
could still work for a couple of versions.4. User-friendly means simple
Examples:
As a variadic function, this one would support:
asyncapi.servers()
asyncapi.servers(server1name)
asyncapi.servers(server1name, server2name)
[More examples to come]
The text was updated successfully, but these errors were encountered: