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

Runitor - create using invalid slug #176

Open
pkExec opened this issue Nov 1, 2024 · 2 comments
Open

Runitor - create using invalid slug #176

pkExec opened this issue Nov 1, 2024 · 2 comments

Comments

@pkExec
Copy link

pkExec commented Nov 1, 2024

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
Copy link
Owner

bdd commented Nov 2, 2024

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
Copy link
Contributor

cuu508 commented Nov 5, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants