search

canonical / go-dqlite

Go bindings for libdqlite
https://dqlite.io
Apache License 2.0
432 stars 69 forks source link

set client timeout in dqlite demo #295

Closed letFunny closed 5 months ago

letFunny commented 5 months ago

Change the policy from retrying indefinitely to having a timeout to improve user experience. The timeout is configurable via a CLI flag (--timeout) and defaults to 2s.

cole-miller commented 5 months ago

@SimonRichardson I know that Juju uses the demo shell, would having a hard timeout like this interfere with what you're doing?

One possible alternative would be to print a brief message every time we go around the retry loop, so you get some feedback about what's going on.

SimonRichardson commented 5 months ago

We could make it configurable with 2 seconds being the default. No matter what the value is, it's probably not right for someone.

Background: We're currently using the dqlite app for accessing the shell.

If we're changing it here, we should also change it in the other location to be consistent.

https://github.com/canonical/go-dqlite/blob/19403452931bf45174dfe1db3b210274a35f4458/cmd/dqlite/dqlite.go#L116-L124

See:

https://github.com/canonical/go-dqlite/blob/19403452931bf45174dfe1db3b210274a35f4458/cmd/dqlite/dqlite.go#L92-L102

cole-miller commented 5 months ago

Thanks, this seems like a reasonable improvement. While reviewing I noticed that we should remove the extra space here: https://github.com/canonical/go-dqlite/blob/19403452931bf45174dfe1db3b210274a35f4458/cmd/dqlite/dqlite.go#L118 but we can leave that for a follow-up.