-
Notifications
You must be signed in to change notification settings - Fork 9
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
Set or change the Destination path from the CLI #87
Comments
I think this is a fair idea. It does make sense that we want to keep user on their terminal, especially when they're configuring some their development configuration anyway. I'm curious if a better solution is to encourage better adoption of Terraform? Although I can see how it's not possible for every team. From the 2 options you have, I think option 1 is way more viable. I feel the complexity of adding feature to the listen command may not be worth it. Can you share the TUI UX you have in mind? |
What about dynamically changing if a This allows:
|
@leggetter what if a source has multiple connections to different CLI destinations, how do we know which destination to override? |
@alexluong, how does the CLI know which destination to inherit and use when it connects, and multiple CLI Destinations exist? |
@leggetter it doesn't, it just sends the events to every connected destination when there's a new event. |
@alexluong, so, there must be some server-side websocket handling code that decides which CLI destination the CLI should inherit? |
@leggetter my understanding is that when a request comes to the source, the server will turn that request into events to relevant destinations (via connection rules logic). The websocket server will deliver each event to the CLI, which will then proxy to the local CLI path. However, from the CLI, it proxies based on source (you can specify the connection too but it's not relevant in this discussion). Not sure if this clarifies things. I'm not entirely sure what your idea is to go about this 😅 |
@alexluong could you create a branch to experiment with this and find any problems we need to solve? Using a flag avoids a breaking change and/or avoids the need for additional optional prams to be passed since @mkherlakian I'm going to suggest we don't change the default behaviour to prompt for a CLI path. We could change to default to |
Ok. But how do you choose the connection that this applies to in this case? Without a connection, the CLI doesn't know which destination to update unless there's only one dest |
Hey @leggetter, so let me elaborate the point a bit just to make sure we understand your idea a bit better. Let's say we have a connection set up like this where a source has 2 connections to 2 CLI destinations. This is how it looks when proxying:
So, from your idea, if we run I understand your intention is to make changing CLI path easier, but we need to think through these edge cases before we can proceed with the simple use case. Besides, another case we should think about is the multi-source support that we will support soon. In that case, there are many different destinations with different path, how should we determine which path to override? |
@alexluong thanks for clarifying. @mkherlakian see above. Would providing a connection name resolved then problem?
Honestly, for newcomers who don't know about sources and connections it adds another unknown param sonim not super happy about it. |
Yes, this would sort of resolve the problem. The only thing is I don't see a ton of value in this, but we can make this work. Basically this will override the CLI path. That's great for temporary work, but if this is a long-term change, than wouldn't updating the destination be better for future usage as well as collaboration?
The concern is this may add more complexity instead of reducing it. The thing is the user must set up their connection & destination first, one way or the other, before doing this. Therefore, I don't quite see the problem this solution is solving. |
Why not?
The way I see it, we should be updating the destination if the path param is specified in the CLI args, or creating it if it doesn't exist.
Why can't the connection and destination be created on the fly?
On the contrary - If we instruct users to just issue the one command, and everything is created for them, even if they are not familiar with source, dest, and connection constructs, they can get started immediately. It reduces friction quite a bit. Ideally we can do this with a guest accounts as well. |
They can, but not with just It seems I'm not quite following the purpose of what we're trying to do here. Maybe @leggetter or @mkherlakian can help write a short summary to make sure we're aligned. From this original message: I thought this is to assist with the UX for continued usage instead of the initial adoption. But from your message @mkherlakian, it seems you're focusing more on the initial friction and user experience? I'm also confused between 2 goals:
So overall it's just a bit difficult what we're trying to solve for. Either of those, or both of those, are all good, but I hope we should clarify the intention so we can solve the right problem. |
@alexluong @mkherlakian have a look at this #90 Try:
(jumping on a flight so this is a quick draft PR) |
Thanks for the PR @leggetter, so from what I can see, here are the key behaviors I noticed from the PR:
I think we can certainly work with this if this is the behavior you'd like. Maybe some small improvements around logging to communicate what we're doing to avoid confusion, but other than that I do think this should work if the behaviors above make sense to yall. |
That works, thanks @leggetter! Let's polish and release. It would be nice to solve the multiple destinations problem too - maybe if you pass a CLI path that matches one of the connections, you select that connection. And if it doesn't match, you create that destination? |
Considering another approach -
Problem is listen now accepts a source, would be difficult to distinguish between source and connection. |
This could work, so if you pass
You can pass a source AND a connection.
We actually already support the CLI path in place of
@leggetter let me know if you'd like to polish yourself or if you'd like my help here! |
@mkherlakian @alexluong here's another pass #92 I've mapped out various scenarios and shown the old behavior and the new behavior. A reasonably big functional change is to remove the interactivity. Reasoning is in the PR. |
Fixed in #97 |
Running the following will prompt for a path for the CLI:
However, after this point, if you want to change the path, you need to go to the dashboard and edit from the UI.
Can we add support for setting the CLI path on the original request and changing the "CLI path" for a Destination from the CLI?
There's a great discussion and some good work gone into #68 as an experiment for adding broader CRUD support and we ultimately decided against it (for now).
However, this feels like a common sense feature to support.
Options I can think of:
@alexluong WDYT?
The text was updated successfully, but these errors were encountered: