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

Use @asyncapi/cli under the hood #485

Closed
smoya opened this issue Feb 20, 2024 · 19 comments · Fixed by asyncapi/cli#1200
Closed

Use @asyncapi/cli under the hood #485

smoya opened this issue Feb 20, 2024 · 19 comments · Fixed by asyncapi/cli#1200
Labels
enhancement New feature or request

Comments

@smoya
Copy link
Member

smoya commented Feb 20, 2024

Reason/Context

Server-API is not so different to what https://github.com/asyncapi/cli is. The big difference remains on the Ports they expose. Beyond that, the logic they perform is precisely the same:

  • CLI's Port (CLI):
    • Inbound adapter: TTY
    • Domain logic: Validate, Convert, Generate, Optimize, Bundle...
    • Outbound adapters: TTY log, files in filesystem.
  • Server-API's Port (HTTP REST):
    • Inbound adapter: HTTP Request
    • Domain logic: Validate, Convert, Generate, Convert, Bundle...
    • Outbound adapter: HTTP Response

In the current Server-API implementation, all the actions the user can perform (commands in CLI) are coded from scratch, using exactly the same dependencies as the CLI has (Parser-JS, Generator, etc). As mentioned above, they look pretty much the same as the commands in CLI.

One of the main issues of having all of this duplication of work is that Server-API falls behind what CLI can do pretty often. Then, people trying to, for example, generate code through the Studio, ends up with a different output than when they use the CLI.
For example, there have been efforts in CLI to enable all commands to work for v3 of the spec. However, Server-API still falls way behind on giving support for v3. See #294 and asyncapi/studio#926.

By using https://github.com/asyncapi/cli in Server-API, we would break that gap; every new update on the CLI (on those commands the Server-API relies on) will be automatically adopted by just updating the dependency.

Description

The point is that neither the CLI and other dependencies (such the Generator) are completely ready to make this possible. For example, CLI doesn't export the functions inside the commands.

We would need to work on the CLI repository to extract all the logic inside each command to exportable functions (command handlers) so we can reuse them here.

I strongly believe on this change as a must, otherwise Server-API will keep falling behind due to the low bandwidth maintainers are running lately. I also believe it is a good win to invest on this rather to instead find more maintainers to maintain the beast.

Relates somehow to https://github.com/orgs/asyncapi/discussions/212

cc @magicmatatjahu @BOLT04 and rest of CLI Owners folks @Souvikns @derberg to know wdyt.

@smoya smoya added the enhancement New feature or request label Feb 20, 2024
@derberg
Copy link
Member

derberg commented Feb 20, 2024

what about pulling server-api inside CLI repo? every new CLI release also provides the server. We do not need separate versioning for server-api, why even calling it server-api. Just can be part of CLI and we could have command asyncapi start server and it would play well also with studio started through CLI 🤔

@smoya
Copy link
Member Author

smoya commented Feb 20, 2024

what about pulling server-api inside CLI repo? every new CLI release also provides the server. We do not need separate versioning for server-api, why even calling it server-api. Just can be part of CLI and we could have command asyncapi start server and it would play well also with studio started through CLI 🤔

I completely like this idea 👍

@fmvilas
Copy link
Member

fmvilas commented Feb 20, 2024

Just can be part of CLI and we could have command asyncapi start server and it would play well also with studio started through CLI 🤔

This 💯

@smoya
Copy link
Member Author

smoya commented Feb 20, 2024

For reference, this topic has been already raised at some point but nothing moved forward:

@Amzani
Copy link

Amzani commented Feb 20, 2024

Two for the price of one baby 👍

@Amzani
Copy link

Amzani commented Feb 20, 2024

I'll take it

Copy link
Member

derberg commented Feb 21, 2024

@smoya yeah that was long time before.

as current CLI codeowner I'm completely fine if you sunset server-api and slowly move it under cli repo. For sure with some MVP first, to validate if it makes sense.

cc @Souvikns @magicmatatjahu

but we need also @BOLT04 voice as he is owner here too.

of course, current owners of server-api would have an option to become owners in CLI. We anyway need to upgrade the contribution structure as already suggested asyncapi/cli#781

@smoya
Copy link
Member Author

smoya commented Feb 21, 2024

cc @Souvikns @magicmatatjahu

but we need also @BOLT04 voice as he is owner here too.

I pinged them additionally via Slack.

@Souvikns
Copy link
Member

what about pulling server-api inside CLI repo? every new CLI release also provides the server. We do not need separate versioning for server-api, why even calling it server-api. Just can be part of CLI and we could have command asyncapi start server and it would play well also with studio started through CLI 🤔

Yeah, I like this idea.

@Amzani
Copy link

Amzani commented Feb 22, 2024

We can start implementing the most critical API endpoints, studio for instance only uses /generate.
Not sure if this API is used somewhere else outside our tooling, I checked with @smoya yesterday and we found out that we don't have any stats.
Another alternative (If this API is only used by studio) is to not migrate at all and let tools use libraries instead of APIs, In this case, Studio will have to use @asyncapi/generator.

@fmvilas
Copy link
Member

fmvilas commented Feb 23, 2024

Another alternative (If this API is only used by studio) is to not migrate at all and let tools use libraries instead of APIs, In this case, Studio will have to use @asyncapi/generator.

This would be perfect but it's currently not possible because Studio is a client-side app and Generator doesn't work on the browser. If we ever make Studio a server-side app then it should be easy to do.

@Amzani
Copy link

Amzani commented Feb 23, 2024

/progress 40

Migrated the /generate endpoint to #asyncapi/cli . It's working in my local dev. Still need some testing, and figure out the deployment part.

CLI architecture will change to accommodate new changes and the migration of remaining endpoints. I'll be using Hexagonal architecture.

Before (CLI)

src/.
├── commands
│   ├── config
│   │   └── context
│   ├── generate
│   ├── new
│   └── start
├── errors
├── hooks
│   └── command_not_found
├── models
└── utils

After (CLI)

.
├── adapters
│   ├── rest
│   │   ├── configs
│   │   ├── controllers
│   │   ├── exceptions
│   │   ├── logs
│   │   └── middlewares
│   └── cli
│       ├── commands
│       │   ├── config
│       │   │   └── context
│       │   ├── generate
│       │   ├── new
│       │   └── start
│       └── hooks
│           └── command_not_found
├── errors
├── internal
│   └── models
├── logs
├── ports
└── utils
  • internal: Only business logic
  • ports: They define the contract or API that the application exposes for external actors, and this will contain interfaces (e.g generator interface)
  • adapters Implementation of those interfaces (eg. API / REST / GRPC...)

@BOLT04
Copy link
Member

BOLT04 commented Feb 24, 2024

what about pulling server-api inside CLI repo? every new CLI release also provides the server. We do not need separate versioning for server-api, why even calling it server-api. Just can be part of CLI and we could have command asyncapi start server and it would play well also with studio started through CLI 🤔

fully agreed @derberg and @smoya 🙂. This seems to be the best approach, mainly to keep CLI enhancements and new features in sync with this API that has fallen way too much behind.

Not sure if this API is used somewhere else outside our tooling, I checked with @smoya yesterday and we found out that we don't have any stats.

@Amzani I don't think this API is used in other tools in the ecosystem besides studio like you said. We would need to bring the API deployment parts to the CLI repo and focus on the used endpoints yes.
@magicmatatjahu do you know any other tool/app using this API in PRD now?

@Amzani
Copy link

Amzani commented Feb 27, 2024

As we don't have 100% visibility of who is using this API, and don't want to break current clients, I suggest using a new domain for the new API that will be hosted by CLI during the migration phase. api.internal.asyncapi.com or api.cli.asyncapi.com

Can someone help with the new domain and digital ocean app?
cc @smoya @fmvilas @derberg

@smoya
Copy link
Member Author

smoya commented Feb 28, 2024

As we don't have 100% visibility of who is using this API, and don't want to break current clients, I suggest using a new domain for the new API that will be hosted by CLI during the migration phase. api.internal.asyncapi.com or api.cli.asyncapi.com

Can someone help with the new domain and digital ocean app? cc @smoya @fmvilas @derberg

Domain stuff goes on either @fmvilas or @derberg afaik.
Digital Ocean Infra is created through Terraform file located in https://github.com/asyncapi/server-api/blob/master/deployments/apps/main.tf. We would need to add that into the CLI repository and change some values like the domain and most probably the app name (to avoid conflicts with current server-api).
I can help supervising that deployment.

@fmvilas
Copy link
Member

fmvilas commented Mar 8, 2024

Ping me on Slack whenever you know which domain you want and where to point. Happy to assist with that.

@Amzani
Copy link

Amzani commented Mar 11, 2024

/progress 50

I've added some basic APIs tests in asyncapi/cli#1200
Note: This change will not only fix some generators for 3.0.0 but also enable new UX/UI enhancements to the CLI (asyncapi/cli#1214)

@derberg
Copy link
Member

derberg commented Mar 11, 2024

sounds good, just remember that once we will deploy server api from context of cli repo, we will have to add DIGITALOCEAN_ACCESS_TOKEN that is already here

@Amzani
Copy link

Amzani commented Mar 12, 2024

/progress 60

Looking at how to reorganize different commands: asyncapi/cli#781, and discovered that we are using an old version of @oclif/core that we need to migrate to the latest version

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.

6 participants