search

planetscale / cli

The CLI for PlanetScale Database
https://planetscale.com/cli
Apache License 2.0
604 stars 51 forks source link

cmd/password: use a duration for TTL #819

Closed mdlayher closed 10 months ago

mdlayher commented 10 months ago

Go has a native duration type and it's much friendlier than requiring the caller to do integer math. --ttl 6h, --ttl 1h30m, etc.

To prevent misuse, require the caller to specify at least 1 second as the TTL when the flag is set.

mattrobenolt commented 10 months ago

I'm not sure I agree with this in this context. This definitely opens the door to attempting to use fractional seconds. I know it's being rounded, but trying to use 5500ms or something is just going to be wrong.

I think in the context of a TTL being seconds, this is a very common format outside of Go.

This is also a breaking change since someone who maybe had a script doing --ttl=30 will now fail.

mdlayher commented 10 months ago

As an option, we can easily prevent fractional seconds in the conversion function.

We can also make the flag a string and infer that --ttl 30 is 30 seconds, while still also recognizing --ttl 30s.

mattrobenolt commented 10 months ago

Sure. I'm doing to defer to others for this. I don't feel that it's worth it or solves anything.

mdlayher commented 10 months ago

The latest commit preserves the ability to pass bare integers in seconds while also allowing Go duration strings such as 30m. No sub-second rounding is permitted.