Improve options handling and add --error-retry-base-interval
#245
Conversation
Those two options have been deprecated 5 years ago. There is likely no reason to keep them around anymore. Signed-off-by: Ludovico de Nittis <[email protected]>
Signed-off-by: Ludovico de Nittis <[email protected]>
When loading a config file, we need to set the options to their default values. Otherwise, when merging it with the CLI options, we could end up with unexpected results. Additionally we need to distinguish between a CLI option that is set to its default value because it was not provided as a launch parameter or if it was explicitly set by the client. This is important because in `MergedWith()` we should prefer the CLI option only if it was explicitly set. Signed-off-by: Ludovico de Nittis <[email protected]>
Setting the base interval between error retries is a useful option, especially when the Internet connection might be unstable. Given that `error-retry` can already be set via the CLI, by adding an option for `error-retry-base-interval` too we can avoid the need to force users to ship a config file when its only purpose was to only change this value. Signed-off-by: Ludovico de Nittis <[email protected]>
When the Internet connection is unstable, a few requests might fail. In those cases, by default, Desync retries 3 times before giving up. However, if the connection momentarily becomes unstable and you retry immediately without waiting, the chances of those attempts still failing are very high. With this commit we set a more reasonable 500ms of base wait interval to give the clients an higher chance of success. Signed-off-by: Ludovico de Nittis <[email protected]>
This makes the parsing of the properties easier. AFAICT those has been marked as deprecated 5 years ago, so I just deleted them.
It's not entirely clear to me why the stores in
If you prefer something different than 500ms, just let me know. The important thing for us is to have the |
|
Looks good. Thank you |
Remove deprecated "http-timeout" and "http-error-retry" options
Those two options have been deprecated 5 years ago.
There is likely no reason to keep them around anymore.
Keep the CLI options while querying the stores in info
Correctly set the default option values and merge them with the config
When loading a config file, we need to set the options to their default
values. Otherwise, when merging it with the CLI options, we could end up
with unexpected results.
Additionally we need to distinguish between a CLI option that is set to
its default value because it was not provided as a launch parameter or
if it was explicitly set by the client. This is important because in
MergedWith()we should prefer the CLI option only if it was explicitlyset.
Add --error-retry-base-interval as a CLI option
Setting the base interval between error retries is a useful option,
especially when the Internet connection might be unstable.
Given that
error-retrycan already be set via the CLI, by adding anoption for
error-retry-base-intervaltoo we can avoid the need toforce users to ship a config file when its only purpose was to only
change this value.
Set a 500ms default value for error-retry-base-interval
When the Internet connection is unstable, a few requests might fail. In
those cases, by default, Desync retries 3 times before giving up.
However, if the connection momentarily becomes unstable and you retry
immediately without waiting, the chances of those attempts still failing
are very high.
With this commit we set a more reasonable 500ms of base wait interval to
give the clients an higher chance of success.