Coder Social home page Coder Social logo

Comments (15)

MikeRalphson avatar MikeRalphson commented on July 28, 2024 2

Or at least introduce the concept of a default context, which a simple / single input document will use. I think I understand the use of a context for related resources, but I'm unsure why I would want more than one context in a single CLI 'session'.

from cli.

derberg avatar derberg commented on July 28, 2024 1

Ouch, lots of frustrations here 😅 tell this imaginary user that I recommend 🍵 with a bit of lemon balm 😆

Keep in mind this CLI is in a super early stage of development at 0.3 release and many basic improvements are missing (like good UX messages to the user). Of course long term instead of No contexts saved yet, run asyncapi --help to know more. we should be much more descriptive, explaining what are the possible options if defaults were not met.

Context purpose is to be optional, not default. Except of WebSocket use case, developers work in many cases with more than just one AsyncAPI file. Contexts are to support such work, this is why you want to be able to easily switch between files with their names instead of remembering paths.

Contexts are not optional as they are not completed feature. The --file flag was added in 0.3.

Would look for files with the name asyncapi.yml, asyncapi.yaml, or asyncapi.json in the current directory. If it's found, it validates the file. If it's not, it fails and shows a message saying it couldn't find an asyncapi file and suggesting how to continue.

This is exactly what we discussed with @Souvikns on his initial PR for contexts but did not want to over-complicate the PR, same was about --file flag. This functionality will follow. Plan is -> if --file is not passed, and if --context is not passed, and if there is no default context already set that could be used, we look for asyncapi.yml, asyncapi.yaml, or asyncapi.json in the current directory, otherwise help message with good explanation of possible options.

So, too summarize, we do have UX in mind, but please do not expect all good things at 0.3 version 😉

PS. about verb noun vs noun verb, it is not related to context itself, so I suggest you report a separate issue if you want to discuss further.

from cli.

fmvilas avatar fmvilas commented on July 28, 2024 1

Alright, so I think this issue doesn't make sense anymore as it seems it was already the intention to make it optional and a good and frequent use-case (when using docker-compose) has been argued. Other issues have been created to fix the exposed problems. I'm closing it but feel free to reopen if you think there's still a case.

from cli.

github-actions avatar github-actions commented on July 28, 2024

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.

Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

from cli.

Souvikns avatar Souvikns commented on July 28, 2024

I really like the idea about enabling contexts, at it removes confusion for the newcomers but also keeps it for the experince users to leverage it if they want it.

from cli.

fmvilas avatar fmvilas commented on July 28, 2024

Or at least introduce the concept of a default context

Yes, sorry, I had that in the issue title but somehow lost it. Changing it.

from cli.

derberg avatar derberg commented on July 28, 2024

btw, I actually checked and asyncapi.yml, asyncapi.yaml, or asyncapi.json in the current directory is already supported in 0.3 version. @Souvikns you are fast 🥳 💪🏼

from cli.

magicmatatjahu avatar magicmatatjahu commented on July 28, 2024

Referring to the Łukasz's comment, we can treat the AsyncAPI docs as the first-citizens of the our CLI, so passing them not as flags, but as standalone arguments. Idea is described here.

from cli.

fmvilas avatar fmvilas commented on July 28, 2024

Well, first of all I'd like to apologize if someone has felt offended by the tone of the message. Please don't take this personally as an attack to what's been developed so far 🙏 I thought it'd be funny to add some example of a raging user, that's why the all caps and the :rage1: and 🤦 emojis. I'm very aware this is 0.3.0 (very early stage) and not judging your work, especially @Souvikns and @jotamusik who have been working a lot on this project. I'm sorry if this has offended any of you 🙏

we should be much more descriptive, explaining what are the possible options if defaults were not met

Agree. Just keep in mind that when you have to explain too much, it means something is failing in the UX.

Except of WebSocket use case, developers work in many cases with more than just one AsyncAPI file. Contexts are to support such work, this is why you want to be able to easily switch between files with their names instead of remembering paths.

You'd have to explain this a bit more to me. Why is it different when using Websockets? 🤔

from cli.

derberg avatar derberg commented on July 28, 2024

With WebSocket you deal with one backend, one AsyncAPI file. With other architectures, broker-centric for example, you deal with more than just one microservice, so multiple files. Example https://github.com/amadeus4dev/amadeus-async-flight-status where you have 3 services and therefore 3 files. You do not work with these files every day, most likely from time to time, so you add them to context so it is easier to find and go back.

For such case you work like this

$ asyncapi context list
monitor : /Users/me/sources/amadeus-async-flight-status/monitor/asyncapi.yaml
notifier : /Users/me/sources/amadeus-async-flight-status/notifier/asyncapi.yaml
subscriber : /Users/me/sources/amadeus-async-flight-status/subscriber/asyncapi.yaml
$ asyncapi context use notifier
$ asyncapi validate
$ asyncapi list channels
$ asyncapi add channel
$ asyncapi ...

Otherwise you work like

$ asyncapi validate /Users/me/sources/amadeus-async-flight-status/notifier/asyncapi.yaml
$ asyncapi list channels /Users/me/sources/amadeus-async-flight-status/notifier/asyncapi.yaml
$ asyncapi add channel /Users/me/sources/amadeus-async-flight-status/notifier/asyncapi.yaml

or like

$ asyncapi validate --file /Users/me/sources/amadeus-async-flight-status/notifier/asyncapi.yaml
$ asyncapi list channels --file /Users/me/sources/amadeus-async-flight-status/notifier/asyncapi.yaml
$ asyncapi add channel --file /Users/me/sources/amadeus-async-flight-status/notifier/asyncapi.yaml

Does it make sense now?
And anyway, contexts are optional, you do not need them, you have --file or asyncapi.yml, asyncapi.yaml, or asyncapi.json in the current directory

from cli.

fmvilas avatar fmvilas commented on July 28, 2024

Oh, I see. Thanks for explaining.

or asyncapi.yml, asyncapi.yaml, or asyncapi.json in the current directory

That's not how it's working. Maybe how it's intended to work?

Captura de pantalla 2021-07-26 a las 13 33 45

You do not work with these files every day, most likely from time to time, so you add them to context so it is easier to find and go back.

Just to make a point regarding this. If it's something you only use "from time to time" it seems way too much to create a feature like contexts. It overcomplicates the CLI unnecessarily IMHO (implementation and usage). When I'm working with different microservices at the same time, I usually have different terminals open because I anyway need them to start, stop, or reload the microservice. But that's probably just how I work.

It seems the idea is taken from the kubectl CLI. I think it makes sense there because you open a terminal and become an operator for different clusters that are usually defined in the same k8s config file. Therefore, you don't have this option of changing to another directory because the other contexts are defined in the same k8s file. I know, it's not always like this but it's a common option and you have to take it into consideration (and they did it, good for them). Of course, you can always open another terminal and operate each cluster on a terminal but that's not convenient because you'd have to open a terminal exclusively for operating a given cluster. Hence, contexts is a clever feature. However, with AsyncAPI, an asyncapi.yaml file defines only one service and you will anyway need to open another terminal to start/stop the service (if you're working with it, of course).

In short, the more I think about contexts, the more I think the feature is not justified here. Will it make sense in the future? Probably. Or probably not. I'm just seeing this as a very early optimization. Precisely because it's 0.3.0, I think this very edge-case shouldn't be considered yet.

from cli.

fmvilas avatar fmvilas commented on July 28, 2024

Nonetheless, I see a case where contexts could be useful tho. For those tools that need multiple asyncapi files as the input, it would be super useful. For instance, if we wanted to leverage https://github.com/asyncapi/app-relations-discovery from the CLI, having to pass a bunch of asyncapi files each time is not really convenient. In such case, having something like a context would be super cool but it should allow us to add more than one asyncapi file to the context.

asyncapi create context --name=graph
asyncapi add file ./service/dummy1/asyncapi.yaml
asyncapi add file ./service/dummy2/asyncapi.yaml
asyncapi add file ./service/dummy3/asyncapi.yaml
asyncapi create graph

from cli.

derberg avatar derberg commented on July 28, 2024

Contexts feature is there already, optional, I don't see a point of removing it really.

but it should allow us to add more than one asyncapi file to the context

As you can see in my example, you have multiple files in the context because you can add multiple contexts, just one can be the default of course:

$ asyncapi context list
monitor : /Users/me/sources/amadeus-async-flight-status/monitor/asyncapi.yaml
notifier : /Users/me/sources/amadeus-async-flight-status/notifier/asyncapi.yaml
subscriber : /Users/me/sources/amadeus-async-flight-status/subscriber/asyncapi.yaml

Using some kind of name as you showed in your example, or rather a label could be good. Something to consider when we integrate relations discovery, or diff.

That's not how it's working. Maybe how it's intended to work?

It works on my side 😄 but I have some files in my context. You found a bug #35

Precisely because it's 0.3.0, I think this very edge-case shouldn't be considered yet.

it took some time discussing the initial CLI setup (#1) and there were no objections for context (that I've put there from the very beginning, clearly stating kubectl inspiration). As I showed, in broker-centric arch it is useful and therefore not edge-case really.

When I'm working with different microservices at the same time, I usually have different terminals open because I anyway need them to start, stop, or reload the microservice. But that's probably just how I work.

The current most common practice that I'm aware of is that people prefer mono repo (like in the case of amadeus showcase) and definitely not open and start each service one by one in a separate terminal but use docker-compose to make your life easier, thus single terminal window. I don't believe we should consider such a flow of working as an edge-case

from cli.

fmvilas avatar fmvilas commented on July 28, 2024

As you can see in my example, you have multiple files in the context because you can add multiple contexts, just one can be the default of course:

🤔 Now I'm more confused then. These are different contexts, right? What I mean is having a single context with multiple asyncapi files. I don't think we're meaning the same.

it took some time discussing the initial CLI setup (#1) and there were no objections for context

To be honest, I always understood contexts as a way to have multiple files as the input for a command. That's why I thought it would be cool.

but use docker-compose to make your life easier, thus single terminal window

This is a good argument for contexts, I agree 👍

from cli.

fmvilas avatar fmvilas commented on July 28, 2024

PS. about verb noun vs noun verb, it is not related to context itself, so I suggest you report a separate issue if you want to discuss further.

I opened an issue: #37

from cli.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.