hookdeck / hookdeck-cli

Alternative to ngrok for localhost asynchronous web development (e.g. webhooks). No account required.
https://hookdeck.com?ref=github-hookdeck-cli
Apache License 2.0
260 stars 9 forks source link

Adds --cli-path and changes behavior to remove connection name and path terminal interactivity #92

Closed leggetter closed 1 month ago

leggetter commented 1 month ago

The interactive prompts for the connection label/name and the CLI path are confusing unless you already understand what the Hookdeck mental model of Source -> Connection -> Destination. Why force this mental model upon developers? Instead, use some sensible defaults and allow the connection name and CLI path to be optionally passed for use when developers need or want to understand the mental model.

New defaults

The quickstart command will now be the following and will require no interaction within the terminal:

hookdeck listen {port} {source_name}

We could have a default for the Source name. However, I don't know what this would be, and thinking about where the event is coming from is a reasonable step towards the Hookdeck mental model.

Behavior

For Old = New behavior, ✅ indicates the behavior has not changed. ⚠️ indicates a change in behavior.

Command Connection Exists? Old = New behavior Old New
hookdeck listen {port} No ⚠️ Prompts for source, connection, and path. Creates new connection with user provided details. Prompts for source. Creates new connection with default Connection and Destination details.
hookdeck listen {port} {source} No ⚠️ Prompts for path and connection. Creates new connection with user provided details. Creates new connection with default Connection and Destination details
hookdeck listen {port} {source} Yes Runs using existing connections between {source} and all CLI Destinations. Runs using existing connections between {source} and all CLI Destinations.
hookdeck listen {port} {source} --cli-path /webhooks No N/A N/A Creates new connection. Runs with /webhooks as path all other default details.
hookdeck listen {port} {source} --cli-path /webhooks Yes N/A N/A Runs using existing connection between {source} and a CLI Destination. If existing Destination CLI path differs from /webhooks it updates the Destination. If more than one matching connection is found, asks the user to specify a connection (see Note 1).
hookdeck listen {port} {source} {connection} No ⚠️ Prompts for path. Creates new connection with user provided details. Creates new connection with /webhooks as path and {connection} as connection name.
hookdeck listen {port} {source} {connection} Yes Runs using connection that matches provided details. Runs using connection that matches provided details.
hookdeck listen {port} {source} {connection} --cli-path /webhooks No N/A N/A Creates new connection named {connection} with default Connection and Destination name but with /webhooks as path.
hookdeck listen {port} {source} {connection} --cli-path /webhooks Yes N/A N/A Runs using existing connection named {connection} between {source} and a CLI Destination. If existing Destination CLI path differs from /webhooks it updates the Destination CLI path. If more than one matching connection is found, asks the user to specify a connection (see Note 1).

Note 1

DEBUG output only present due to `--log-level debug' flag.

./hookdeck-cli listen 3000 test-inbound  --cli-path /flarb --log-level debug
[Tue, 23 Jul 2024 17:29:00 BST] DEBUG Connection exists for Source "test-inbound", Connection "", and CLI path "/flarb"
Multiple CLI destinations found. Cannot set the CLI path on multiple destinations.
Specify a single destination to update the CLI path. For example, pass a connection name:

  hookdeck listen http://localhost:3000 test-inbound connection-name --cli-path /flarb
alexluong commented 1 month ago

We could have a default for the Source name. However, I don't know what this would be

Since source name is just to identify the source but doesn't have any meaningful impact, we can potentially generate a random name (maybe "adjective noun random_number")?


Sharing this message I sent in the other PR, curious to hear if you agree with this thought process here.

https://github.com/hookdeck/hookdeck-cli/pull/90#issuecomment-2245290983

CleanShot 2024-07-23 at 23 51 58

leggetter commented 1 month ago

We could have a default for the Source name. However, I don't know what this would be

Since source name is just to identify the source but doesn't have any meaningful impact, we can potentially generate a random name (maybe "adjective noun random_number")?

Yeah, could do. @mkherlakian - WDYT?

Sharing this message I sent in the other PR, curious to hear if you agree with this thought process here.

#90 (comment)

CleanShot 2024-07-23 at 23 51 58

It's a good question: would the user expect a side effect to take place by passing a flag? IMO it's reasonable.

mkherlakian commented 1 month ago

@leggetter Many thanks for documenting this, very helpful!

Yeah, could do. @mkherlakian - WDYT?

Yes, or something along hte lines of source_<random_letters_numbers>

It's a good question: would the user expect a side effect to take place by passing a flag? IMO it's reasonable.

I don't think that we need to explicitly disallow the switching of the path, i.e. no need for a separate flag IMO

Runs using existing connection named {connection} between {source} and a CLI Destination. If existing Destination CLI path differs from /webhooks it updates the Destination CLI path. If more than one matching connection is found, asks the user to specify a connection (see Note 1).

I'm trying to map out scenarios for this one - Is there a potential.case where we'd end up creating 2 cli destinations (ie 2 connections) on separate runs without the user specifying a connection explicitly, which would lead to the above becoming a friction point? i.e. if I do hookdeck listen {port} {source} --cli-path /webhooks

and then do hookdeck listen {port} {source} --cli-path /webhooks_new

That would update the path of the existing connection, or would it create a new one?

Last but not least, the user auth problem is somtehing we'll need to solve, meaning if we also can't do something like --api-key ... and have it work without hte CI step, it still feels janky.

alexluong commented 1 month ago

That would update the path of the existing connection, or would it create a new one?

@mkherlakian from the table that @leggetter described, it seems your workflow would result in 2 separate connections / destinations.

Last but not least, the user auth problem is somtehing we'll need to solve, meaning if we also can't do something like --api-key ... and have it work without hte CI step, it still feels janky.

Are you referring to this issue #89? Can you take a look? I think this is an issue with the API or with the design and not a CLI bug.

mkherlakian commented 1 month ago

That would update the path of the existing connection, or would it create a new one?

@mkherlakian from the table that @leggetter described, it seems your workflow would result in 2 separate connections / destinations.

No, not according to the second row, I just reread, think that answers the question but I'll wait for @leggetter's confirmation

image

Last but not least, the user auth problem is somtehing we'll need to solve, meaning if we also can't do something like --api-key ... and have it work without hte CI step, it still feels janky.

Are you referring to this issue #89? Can you take a look? I think this is an issue with the API or with the design and not a CLI bug.

I'll answer on that issue.

leggetter commented 1 month ago

@leggetter Many thanks for documenting this, very helpful!

Yeah, could do. @mkherlakian - WDYT?

Yes, or something along hte lines of source_<random_letters_numbers>

Ok, I'll look into this.

Runs using existing connection named {connection} between {source} and a CLI Destination. If existing Destination CLI path differs from /webhooks it updates the Destination CLI path. If more than one matching connection is found, asks the user to specify a connection (see Note 1).

I'm trying to map out scenarios for this one - Is there a potential.case where we'd end up creating 2 cli destinations (ie 2 connections) on separate runs without the user specifying a connection explicitly, which would lead to the above becoming a friction point? i.e. if I do hookdeck listen {port} {source} --cli-path /webhooks

and then do hookdeck listen {port} {source} --cli-path /webhooks_new

That would update the path of the existing connection, or would it create a new one?

It is possible to create two connections to the CLI with other commands. I discovered this with the current CLI when creating the CLI/Docker quickstart example. And that's why the cURL command in that example passes a Connection name (note that with this CLI update the cURL command is no longer required in the Docker example).

However, when the --cli-path is supplied, additional logic kicks in. The logic checks to ensure there is only one matching connection to be updated. If more than one connection is found, the user is informed to pass a {connection} to ensure one unique connection has the Destination path updated.

Last but not least, the user auth problem is somtehing we'll need to solve, meaning if we also can't do something like --api-key ... and have it work without hte CI step, it still feels janky.

The following works with the API key (as shown in the docker example):

hookdeck ci --api-key $HOOKDECK_API_KEY
hookdeck listen $PORT $SOURCE_NAME $CONNECTION_NAME --cli-path /webhook

That said, #89 does highlight unexpected behavior.

mkherlakian commented 1 month ago

It is possible to create two connections to the CLI with other commands. I discovered this with the current CLI when creating the CLI/Docker quickstart example. And that's why the cURL command in that example passes a Connection name (note that with this CLI update the cURL command is no longer required in the Docker example).

However, when the --cli-path is supplied, additional logic kicks in. The logic checks to ensure there is only one matching connection to be updated. If more than one connection is found, the user is informed to pass a {connection} to ensure one unique connection has the Destination path updated.

Perfect, that's what we want

The following works with the API key (as shown in the docker example):

hookdeck ci --api-key $HOOKDECK_API_KEY hookdeck listen $PORT $SOURCE_NAME $CONNECTION_NAME --cli-path /webhook That said, https://github.com/hookdeck/hookdeck-cli/issues/89 does highlight unexpected behavior.

Yes, I'm answering on the issue itself - this works because ci creates a cli user. It's janky though because the user won't see the session in their dashboard - it'll fall back eventually because we look for connected sessions and assign one, nit it's not "clean"