bdd / runitor

A command runner with healthchecks.io integration
BSD Zero Clause License
287 stars 16 forks source link

Runitor - create using invalid slug #176

Open pkExec opened 1 month ago

pkExec commented 1 month ago

If for example i use this: runitor -slug NOCAPSALLOWED_oops -create -ping-key replace_with_ping_key -- echo 'test'

I get:

Ping(start): nonretriable error response: 400 Bad Request
test
Ping(success): nonretriable error response: 400 Bad Request

And the check fails to get created.

I think it should: a) convert slug to lowercase (duh) b) maybe show a better error message.

bdd commented 1 month ago

Re: "option a"

Although I prefer to not do silent conversions, I think it's unlikely this limitation may change in the future, or someone is running their own instance with different slug rules.

Rewriting (interally or via a redirect) might also be an option for Healthchecks though, so I'm going to cc @cuu508. Pēteris, if you are confident slugs will always stay as lowercase, then maybe such silent conversion in runitor won't be an underhanded behavior.

Re: "option b"

I remembered why I didn't include the API messages when I first wrote runitor. They weren't adding much.

~% curl https://hc-ping.com/$(printf "k%.0s" {1..22})/aaa
not found%                                                                                                                    ~% curl https://hc-ping.com/$(printf "k%.0s" {1..22})/AAA
invalid url format%

No way to tell it's upset about caps in the slug.

Option C?

Maybe runitor can add "gotchas" support and show warning messages to make things easier. Like detecting caps in the slugs. ...Though I don't immediately recall what else can be put in here.

cuu508 commented 4 weeks ago

Rewriting (interally or via a redirect) might also be an option for Healthchecks though, so I'm going to cc @cuu508. Pēteris, if you are confident slugs will always stay as lowercase, then maybe such silent conversion in runitor won't be an underhanded behavior.

I currently have no plans to change the slug syntax rules. But no commitment, there could be a compelling use case for expanding the syntax rules in the future.

I would prefer no silent conversion in runitor. With silent conversion, the user can pass slugs like foobar and FooBar to runitor and it would Just Work. But, if the user is not careful, they may miss the fact foobar and FooBar will both map to the same check in Healthchecks.

With no silent conversion, runitor rejects (or warns about) FooBar. So the user has to explicitly handle conversion to lowercase (and so has to be aware of it).

The "gotchas" idea sounds good to me. Warn the user that "this will probably not work", but let them try anyway.