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

How to solve tooling requirements for giving support to multiple parser-api versions #848

Closed
smoya opened this issue Aug 17, 2023 · 7 comments
Labels

Comments

@smoya
Copy link
Member

smoya commented Aug 17, 2023

I believe most of the use cases of this library are projects that need to parse their own AsyncAPI documents and extract info from them. The purpose doesn't matter for the topic we discuss here. Those, will use a particular version (if not latest) of the Parser-JS and, thanks to the [Parser-API](https://github.com/asyncapi/parser-api/), the API will be very stable between major versions of the spec.

However, there is a limited set of use cases where there is the need of parsing an AsyncAPI document and get a parsed object that contains a particular set of models and methods based on a given parser-api version. Examples are the [Generator](https://github.com/asyncapi/generator), and [Modelina](https://github.com/asyncapi/modelina).
In the case of the Generator, it requires an specific version of the parsed AsyncAPI object based on the parser-api version compatible with each template (specified in each template), otherwise the template could crash using methods that does not exist in that object.

That introduces the need for implementing a strategy pattern that, based on the found parser-api config on the template, to parse the document with one version of the Parser-JS or another (depending on which version supports that particular parser-api, which is defined in the compatibility matrix found in the README.md). That means the generator needs to install and manage several versions of the @asyncapi/parser package, among other stuff, to make that happen.

This problem was raised to me few days ago by @jonas Lagoni in a private and informal chat and today we discussed about it in a more formal way so we could reach an idea of what are the alternatives or better approaches for handling that. We have come with the following strategies/alternatives so we can pick up one (or find a new one):

  1. All parser-api versions (or few) are supported in Parser-JS. All in the same code, means just one package that supports all those parser-api versions.
    • PROS:
      • Tools depend only in one @asyncapi/parser package.
      • Easy to add fixes in Parser-JS code that is used in every of those implementations compared to propagate the fix to several branches/tags.
    • CONS:
      • Code base increases in size (lines of code mostly duplicated per parser-api version). Harder to maintain.
      • Need to introduce If/else(s) based on parser-api versions in shared code, etc, polluting the code.
      • For how long do we keep implementations for each parser-api major version?
  2. Same as the previous point, but releasing one @asyncapi/parser package per major parser-api version supported + one with support for all of them (like in previous point).
    • PROS:
      • Tools can depend in only one @asyncapi/parser package or several.
      • Same as in the previous point.
    • CONS:
      • Building and CI can be hard. Support for multiple packages inside the same repository should be added.
      • Same as in the previous point.
  3. Standard release cycles by keeping one branch/tag per each supported parser-api version, only backporting critical bugs to those if necessary. Means releasing one @asyncapi/parser package per major parser-api version supported as normal release cycle.
    • PROS:
      • More aligned with standard release cycles. As expected, every major change in parser-api will translate into a major release in Parser-JS.
      • All code in a particular branch is just focused on that version, meaning no need for to support anything else.
      • Smaller code base, easier to maintain.
      • No need to introduce any change in CI/CD.
    • CONS:
      • Backporting critical bugfixes need to create multiple PR’s, one for each supported version. Introduces the need of listing which parser-api major versions we support (compatibility matrix is also meant for that).
      • Those tools will need to import each package for each parser-api version. They will need to run some logic (almost the same) for, based on a given JSON document + parser-api version, to produce the right parsed asyncapi object.
  4. Same as previous but adding a new package (in a new repository) that contains that helper logic for using one or another parser version based on a given JSON doc + parser-api version as input.
    • PROS:
      • Those tools won’t need to code any special logic for deciding which version of the parser they need to use in order to produce a particular version of a parsed asyncapi object (based on parser-api version).
      • Those tools will only depend in this new package, replacing any other direct dependency on @asyncapi/parser
      • Same as previous.
    • CONS:
      • We maintain a new repository/package. However, minor updates of Parser-JS versions will be automatically updated by the bot.
      • Same as previous.

cc @jonaslagoni @fmvilas @magicmatatjahu @derberg

@smoya
Copy link
Member Author

smoya commented Aug 17, 2023

I see solution 4 is the most simplest and less invasive. In my opinion, solving that particular case of those tools should not affect the normal developer experience in the Parser-JS repository, nor producing side-effects like increasing the size of the installed package, etc.

By having that isolated new package, those tools plus any other that could came up from other users will be able to easily instantiate a parser that produces a compatible version of the parsed AsyncAPI object. I expect those cases to be a minority or close to none.

Regarding backporting critical bugfixes to previous versions, I suggest we use the compatibility matrix we have in the README.md file and either we add another column mentioning if that version is fully supported in terms of critical bufixes or not. In that way, we can help/force users to eventually update if they want to keep their tools to be up-to-date.

@smoya
Copy link
Member Author

smoya commented Aug 18, 2023

I created a first version of that library I mentioned https://github.com/smoya/multi-parser-js

@fmvilas
Copy link
Member

fmvilas commented Aug 24, 2023

My two cents

By looking at Generator releases, we can see that Generator hasn't gotten any new features recently. Actually, only 2 in the last year. And they were related to bumping dependencies and supporting a new spec version. A similar thing happens when it comes to bug fixes. Most of the patch versions are just dependency bumps. I'd say Generator is pretty stable.

So I'd love you to consider the following option:

  • Maintain Generator v1.x in a separate branch. This version only supports the old Parser API. This may only get patches from now on and we set a deprecation notice and date.
  • Bump Generator to v2 where we only support the new Parser API.
  • Stop honoring the apiVersion field that was introduced recently. Now each major version version of the Generator is tied to a specific API version.
  • Make CLI and other packages depending on Generator to support installing multiple versions of Generator (actually just two).

We move the maintenance burden to Generator, CLI, Server API, and Studio. The impact of the changes will be reduced since Parser is a dependency of all the tools.

Another more radical approach would be to completely remove support for the old Parser API in Generator. We migrate all our templates and see who else complains. I've never seen anyone maintaining their own custom templates. They exist for sure but they may not be as many to justify so much complexity and flexibility. We offer them a good migration guide or they can stay with the older version. Since we don't have the data of how many users are going to be impacted, one way is to let them complain and then rollback to the previous plan, which wouldn't be a breaking change since it would be just a matter of supporting one more Parser API version (the old one).

Anyway, just wanted you to consider this alternative. I'm fine with anything you folks decide 👍

@smoya
Copy link
Member Author

smoya commented Aug 24, 2023

1st version has been published to NPM: https://www.npmjs.com/package/@smoya/multi-parser

@derberg
Copy link
Member

derberg commented Aug 29, 2023

First attempt to add apiVersion in Generator was last year already, when v3 was away. Idea as far as I remember was:

  1. to enable parser API v1 early, without feature branches and major releases. It was a consistent approach to what we did with renderers. We did not kick Nunjucks away, we added React as another engine to try out without breaking changes, harden it first
  2. without apiVersion we would complicate lives of many folks. Even though I agree with @fmvilas that Generator is stable and not many features are added (to library itself), for me and supporting a new spec version is a pretty important feature, we just do not see it as such as on a code level it is just a bump. So basically if few months ago we would simply introduce new parser API and release Generator v2, then we would decrease the adoption of new spec versions, as nobody would be able to use them in their templates (without upgrade to Generator v2)

Today, knowing we will most certainly not release any 2.x spec minor but jump to 3.x:

  • point 2 is no longer relevant, apiVersion is not needed. Template devs will anyway have to migrate their templates to spec 3.x and they can do it with new API only anyway. Otherwise they should not update templates and not make them compatible with Generator > 2
  • point 1 is still relevant, as we still did not really check new Parser API in templates (except markdown-template)

so yeah 😄

I'm ok to drop apiVersion and just support parser API v1 in the generator, but rather have it as a next-major with release candidate that we can test with templates, and we could integrate and see how it will work in cli and server-api that need to support Generator v1 and v2

@derberg
Copy link
Member

derberg commented Sep 13, 2023

one last thing regarding multi-parser - please just include it in the parser, not separate repository. It can be easily released and versioned together with parser. Can be even promoted in readme because it has a great advantage - it by default includes all custom schema parsers -> #339

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 12, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 11, 2024
@smoya smoya closed this as completed May 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants