Allow clients to specify validity durations for bounds#231
Conversation
52fc774 to
7bc8d63
Compare
|
The context being that we do not want clients to set bounds for arbitrarily long durations: Would be interested in hearing your opinions. |
7bc8d63 to
9046f01
Compare
I agree it is better to fail if the requested time can't be fulfilled, otherwise it could lead to unexpected behaviour. But any reason not to use an arbitrary number, document a maximum and minimum allowed value and fail the request if the value is out of the valid range? For me it looks a bit weird to have these times as an enum. |
If it is an enum, users will get a compilation error if they choose an invalid timestamp. If itis a number, then they will get a runtime error, which could mean more mistakes and more error-handling (so more code). Hence my first choice was an enum. But TBH, I do not have a strong opinion either way. I figured I could just create the PR and discuss it here. |
This commit adds a field named `validity_duration` to the `AddComponentBoundsRequest` message, which can be used by clients to specify a custom default duration to bounds while adding them. If not specified, a default validity duration of 5 seconds is applied. The decision to use an enumerated list of allowed durations has been impacted by our intention to not allow users to set arbitrarily long validity durations on bounds. Using an enum makes it clear while using the API. Signed-off-by: Tiyash Basu <tiyash.basu@frequenz.com>
9046f01 to
42f7836
Compare
|
I'm not completely against the enum either, which I guess it is a bad thing, as it is harder to decide :P I just find it a bit weird, but I never used it either, so maybe I'm just concerned that it might be a bit inflexible but it is an unjustified concern and it might be more than enough flexibility. So probably it is better to get someone that really use bounds to talk. Maybe @shsms @daniel-zullo-frequenz @Marenz ? |
|
The SDK won't be sending bounds once we upgrade to this version, just setting power. For power, the interval is not customizable right? If it is customizable, we'd have to get an option from the actors, so that the SDK doesn't have to make generalized decisions. Also, I'm curious about the motivation for asking the users to decide. Why/when would I as a user choose 15 minutes over the default 5 seconds, and repeat commands every 3 seconds? Are there any specific usecases/requirements that's driving this? |
The idea is to make it customizable.
Good point, I forgot to link the issue. It is linked now, and has the use-case mentioned there. |
|
ping |
This commit adds a field named
validity_durationto theAddComponentBoundsRequestmessage, which can be used by clients to specify a custom default duration to bounds while adding them. If not specified, a default validity duration of 5 seconds is applied.