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 a script and CI job to validate spec examples #957

Closed
jonaslagoni opened this issue Aug 2, 2023 · 34 comments · Fixed by #1059
Closed

Add a script and CI job to validate spec examples #957

jonaslagoni opened this issue Aug 2, 2023 · 34 comments · Fixed by #1059
Assignees
Labels
bounty AsyncAPI Bounty keep-open Prevents stale bot from closing this issue or PR

Comments

@jonaslagoni
Copy link
Member

This feature request was discussed here: #947 (comment)

We need to make sure all the examples are complete and valid according to our spec. This should of course be checked for each PR that change the examples.

@smoya
Copy link
Member

smoya commented Sep 17, 2023

We should include the examples embeded into the spec itself, not only the files under the examples dir. Related to this: #971

In order to do that, those examples would need to be fully working AsyncAPI files instead of partials. Otherwise validation won't work. If we want to avoid having full AsyncAPI documents as embedded examples, we could maybe do some trick in CI and add all the minimum required fields to each example before validating.

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 the stale label Jan 16, 2024
@smoya smoya added keep-open Prevents stale bot from closing this issue or PR and removed stale labels Feb 19, 2024
@smoya
Copy link
Member

smoya commented Feb 19, 2024

I consider this as a potential issue for next Bounty program. Not sure if there is a way of keeping track potential issues for next rounds somewhere. Perhaps labels 🤔

cc @aeworxet

@aeworxet
Copy link

I consider this as a potential issue for next Bounty program.

Sure, anyone from CODEOWNERS of this repository can submit it.

Not sure if there is a way of keeping track potential issues for next rounds somewhere. Perhaps labels 🤔

Up until now, it was simply something the maintainer just remembered.
If there is a huge flow of potential Bounty Issues in the future, I will of course introduce a way to mark those for coming calendar rounds with something like bounty/potential.
Adding GH labels to issues by a user is troublesome because, in order to SIMPLY add a label, a user needs to have full write permission to the repository, which, of course, is not likable by anyone. So I am eyeing one of the existing GH Actions bots, but no ready decision has been made yet.

@derberg
Copy link
Member

derberg commented Feb 22, 2024

yeah, would be good to have a kinda "reminder" like bounty/candidate or something

@smoya
Copy link
Member

smoya commented Feb 22, 2024

The work could be split in two phases:

  1. Validation of all AsyncAPI documents under /examples dir in the Spec repo. https://github.com/asyncapi/spec/tree/master/examples. I believe we could use For the record, I believe we could use https://github.com/asyncapi/github-action-for-cli
  2. Validation of all examples embedded in the spec itself. Such as the following https://github.com/asyncapi/spec/blob/master/spec/asyncapi.md#multi-format-schema-object-examples

However, I believe both should be part of the same bounty program issue.

@AnimeshKumar923
Copy link
Contributor

AnimeshKumar923 commented Mar 6, 2024

Hello there 👋 @jonaslagoni @smoya

If it's fine with you, I would be glad to work on this issue under the Bounty Program, 2024-Q2 as this is a potential issue for the Program.

Thanks! 👍


cc: @aeworxet

@aeworxet
Copy link

aeworxet commented Mar 7, 2024

@AnimeshKumar923
Submitted.

@AnimeshKumar923
Copy link
Contributor

Thank you! @aeworxet @smoya

@derberg
Copy link
Member

derberg commented Mar 7, 2024

ah yes, let's have it on the bounty, definitely 🍺 thanks @AnimeshKumar923 for reminder

@Shurtu-gal
Copy link
Contributor

Hope I am not late 😔. Would love to help out in any way possible 😉.

@asyncapi-bot asyncapi-bot added the bounty AsyncAPI Bounty label Mar 18, 2024
@aeworxet
Copy link

Bounty Issue's service comment

Text labels: bounty/2024-Q2, bounty/medium, bounty/coding
First assignment to third-party contributors: 2024-03-22 00:00:00 UTC+12:00
End Of Life: 2024-08-31 23:59:59 UTC-12:00

@asyncapi/bounty_team

@smoya
Copy link
Member

smoya commented Mar 19, 2024

Considering @Shurtu-gal is already assigned to two Bounty issues already (having reached the limit), it is automatically discarded from this one in favour of @AnimeshKumar923, who additionally applied earlier, and also worked on similar issues in the past.

So @AnimeshKumar923 this is all yours.

@AnimeshKumar923
Copy link
Contributor

Considering @Shurtu-gal is already assigned to two Bounty issues already (having reached the limit), it is automatically discarded from this one in favour of @AnimeshKumar923, who additionally applied earlier, and also worked on similar issues in the past.

So @AnimeshKumar923 this is all yours.

Thank you for this opportunity! 🙏 @smoya
Grateful to be participating again and work with you... 🙇


cc: @aeworxet assignment has been made. Please update the timeline for this issue. Thanks 👍

@aeworxet
Copy link

Bounty Issue's Timeline

Complexity Level Assignment date (by GitHub) Start date (by BP rules) End date (by BP rules) Draft PR submission Final PR submission Final PR merge
Medium 2024-03-19 2024-04-01 2024-05-10 2024-04-12 2024-04-26 2024-05-10
Please note that the dates given represent deadlines, not specific dates, so if the goal is reached sooner, it's better.

@aeworxet
Copy link

#1046 (comment)

@aeworxet
Copy link

Upon request of the Bounty Program Participant (@AnimeshKumar923), all remaining target dates of the Bounty Issue's Timeline are extended by six calendar weeks.

Bounty Issue's Timeline Extended

Complexity Level Assignment date (by GitHub) Start date (by BP rules) End date (by BP rules) Draft PR submission Final PR submission Final PR merge
Medium 2024-03-19 2024-04-01 2024-06-21 2024-04-12 2024-06-07 2024-06-21
Please note that the dates given represent deadlines, not specific dates, so if the goal is reached sooner, it's better.
Keep in mind the responsibility for violations of the Timeline.

@AnimeshKumar923
Copy link
Contributor

From May 19th to May 28th, I had prolonged break-downs of telecommunications in my area (which included internet shutdown). This situation is subject to Article 3(f) of the ICC Force Majeure and Hardship Clauses. Hence, I request an additional extension by two weeks to the one I already have.

cc: @aeworxet @smoya

@AnimeshKumar923
Copy link
Contributor

Progress Update

@aeworxet
Copy link

aeworxet commented Jun 1, 2024

Upon request of the Bounty Program Participant (@AnimeshKumar923), all remaining target dates of the Bounty Issue's Timeline are extended by eight calendar weeks.

Bounty Issue's Timeline Extended

Complexity Level Assignment date (by GitHub) Start date (by BP rules) End date (by BP rules) Draft PR submission Final PR submission Final PR merge
Medium 2024-03-19 2024-04-01 2024-07-05 2024-04-12 2024-06-21 2024-07-05
Please note that the dates given represent deadlines, not specific dates, so if the goal is reached sooner, it's better.
Keep in mind the responsibility for violations of the Timeline.

@smoya
Copy link
Member

smoya commented Jun 5, 2024

As embeded examples (asyncapi.md file) are partials of an AsyncAPI doc, meaning the examples are not full AsyncAPI documents but just parts of it, we need somehow to annotate on each example which part of the spec (to which object) the example maps to.

I suggested @AnimeshKumar923 to add an HTML comment like the following.

Given the following embedded example for Operations Object:

onUserSignUp:
  title: User sign up
  summary: Action to sign a user up.
  description: A longer description
  channel:
    $ref: '#/channels/userSignup'
  action: send
  tags:
    - name: user
    - name: signup
    - name: register
  bindings:
    amqp:
      ack: false
  traits:
    - $ref: '#/components/operationTraits/kafka'

To add the following line before it:
<!-- asyncapi-example-tester:{test:'Operations Object',json_path:'$.operations'} -->

By using JSON Path, we can set up a base AsyncAPI document that includes everything is needed (not just Info object, etc) but also all components referred in all examples. In that way, it will be just a matter of replacing that base object with the example on those indicated paths and validate the whole doc.

You can see the current whole spec with such HTML comments added in my following gist: https://gist.github.com/smoya/1eafa264cee7ba6d9be7419f8ec33859 (see raw)

I believe @AnimeshKumar923 is working on providing such base AsyncAPI document: https://asyncapi.slack.com/archives/C34F2JV0U/p1717573165200079

I'm asking the rest of Code Owners to comment and share what is your feeling about adding such comments or to suggest an alternative cc @fmvilas @derberg @dalelane @char0n @GreenRover

@AnimeshKumar923
Copy link
Contributor

AnimeshKumar923 commented Jun 5, 2024

Thank you so much for your suggestion @smoya Really appreciate it!.
Also, can the maintainers please provide their feedback upon this method (or maybe suggest an alternative approach?)
Will adding the comments break anything? Or some other problems that we're unaware of?
Thanks! 👍

@GreenRover
Copy link
Collaborator

I dont know how the markdown parser will handle those comments, but this should be easy to test.
I would like to see a poc.
But expect it to work

@fmvilas
Copy link
Member

fmvilas commented Jun 6, 2024

I like it. Markdown parsers will handle it just fine because HTML is supported in Markdown.

An alternative approach could be to generate these examples (and actually the whole Markdown file) from the JSON Schema definition but that's a totally different thing 😅

@dalelane
Copy link
Collaborator

dalelane commented Jun 7, 2024

I really like this - I think having a way to catch problems in examples will be hugely valuable.

Perhaps like @fmvilas, I started considering an alternate approach - require that examples be written in full and stored externally, and then embed an excerpt from it in markdown docs. My reason for thinking this is that it would catch issues that can only be observed in the context of a whole doc, such as references to other sections.

But I've talked myself out of this as I think it's too onerous a requirement for docs to demand fully examples every time we want to illustrate something, and would likely put people off adding examples at all.

I think your approach is a better compromise - gets us to improve the checking we do on examples, with only minimal incremental cost for each example.

AnimeshKumar923 added a commit to AnimeshKumar923/asyncapi-spec that referenced this issue Jun 8, 2024
Changes:

- added comments in the file as suggested by Sergio here: asyncapi#957 (comment)
- more comments to be adjusted according to the json format
@AnimeshKumar923
Copy link
Contributor

Thank you for your thoughts upon this! @GreenRover @fmvilas @dalelane 👍

Although it's still a WIP, I'm currently trying to implement the approach that @smoya suggested via #1059

My reason for thinking this is that it would catch issues that can only be observed in the context of a whole doc, such as references to other sections.

This is the thing that I've been pondering upon recently. Sometime the examples would contain references that would be new, whose references might not be already present there in the base document, ultimately resulting in failing of the validation in the CI.
I'm thinking that there should be an acceptable range that 'okay, now this can handle majority of the examples'. Maybe we can add more examples and fields as we receive them in the future? Don't know, just putting it out there...

But I've talked myself out of this as I think it's too onerous a requirement for docs to demand fully examples every time we want to illustrate something, and would likely put people off adding examples at all.

That I agree with. I'm thinking of going right now with the approach suggested by @smoya and in the future, if needed, we can open a new issue and try to implement this way of validating the embedded, or maybe seek out to find alternatives... 👀

@AnimeshKumar923
Copy link
Contributor

AnimeshKumar923 commented Jun 8, 2024

Also, since both JSON and YAML excerpts are given for a particular example, I'm thinking of taking only the JSON excerpts from the asyncapi.md file because ultimately that would be used for the JSONPath and document validation.

@smoya
Copy link
Member

smoya commented Jun 10, 2024

Perhaps like @fmvilas, I started considering an alternate approach - require that examples be written in full and stored externally, and then embed an excerpt from it in markdown docs. My reason for thinking this is that it would catch issues that can only be observed in the context of a whole doc, such as references to other sections.

I thought about this and Indeed I see it as the right way to go. However, it escapes from my knowledge. Not sure if we use this for rendering code blocks written in markdown files. If so, we could implement a trim function or similar so we can set the lines we want to extract from the example file and use it as final example.

Definitely a good issue to work in the future. Totally worth opening a new issue in website repo.

@smoya
Copy link
Member

smoya commented Jul 5, 2024

This task is completed now.
This is a Bounty program issue assigned to @AnimeshKumar923 and it has been merged in time 💯

Cc @aeworxet

@aeworxet
Copy link

aeworxet commented Jul 6, 2024

Bounty Issue Completed 🎉

@AnimeshKumar923, please go to the AsyncAPI page on Open Collective and submit an invoice for USD 200.00 with the expense title Bounty spec#957, tag bounty, and full URL of this Bounty Issue in the description.

@AnimeshKumar923
Copy link
Contributor

Bounty Issue Completed 🎉

@AnimeshKumar923, please go to the AsyncAPI page on Open Collective and submit an invoice for USD 200.00 with the expense title Bounty spec#957, tag bounty, and full URL of this Bounty Issue in the description.

Submitted.

@AnimeshKumar923
Copy link
Contributor

Thank you so much @smoya for this opportunity. I learned a lot during the resolution of this issue. Looking forward to more collaborations in the future. 🙏 Cheers! 🙌

Thank you @GreenRover @fmvilas @dalelane as well for your valuable input. 🙏

@smoya
Copy link
Member

smoya commented Jul 15, 2024

@AnimeshKumar923 It would be so cool if you can write a blog post on AsyncAPI website about how you managed to set up validation of the embedded examples. Just suggesting!

@AnimeshKumar923
Copy link
Contributor

@AnimeshKumar923 It would be so cool if you can write a blog post on AsyncAPI website about how you managed to set up validation of the embedded examples. Just suggesting!

@smoya That sounds good. I'll look at it. I'll be a bit busy for 3-4 weeks due to new college semester starting and need to figure out and setup some stuff. I'll keep working on it bit-by-bit as I get time in between.
Thanks for the suggestion. Appreciate it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty AsyncAPI Bounty keep-open Prevents stale bot from closing this issue or PR
Projects
Status: Completed
Development

Successfully merging a pull request may close this issue.

10 participants