-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Configuration
This write-up was triggered by looking at defaults and that's the main concern right now. However it is not necessarily limited to that.
Note on terminology: "config users" means any 3rd party coming in contact with our config, notably wrappers and anyone scripting using the rest api.
Created by gh-md-toc
Consistent behaviour and meaning of config elements in Syncthing's codebase and on APIs/serialisation.
-
XML On disk config
-
JSON
API -
protobuf
Specification / Generation of go types.
Currently our handling of defaults is inconsistent:
-
Implicit
The zero value means default, i.e. a default value gets used at runtime instead of zero. -
Explicit
-
The string "default" is stored and translated to the actual defaults at runtime.
-
The actual default value is written to the config object.
-
-
Tags from
ext.default
in protobuf. -
Default values set in
.prepare
method on config objects. -
Configuration getters (with
Raw
-prefix on the corresponding element). -
"Consumer": Default value specified in code using the config, e.g.
folder
usingdefaultPullerPause
ifFolderconfiguration.PullerPauseS == 0
.
-
When creating a new config object.
-
When unmarshalling an entire config, defaults for options and GUI are applied, but not for folders and devices. When unmarshalling parts of the config (e.g. on
/rest/config/options
), no defaults are applied. -
When changing config at runtime,
.prepare
is called on folders/devices which explicitly sets some defaults.
Besides their actual value, 0
and < 0
may mean unlimited or disabled (both can mean both). Plus as seen above, the zero value can also represent some default.
In other places there e.g. FSWatcherEnabled
and FSWatcherDelayS
.
When encountering a zero value replace it with the actual default value. We don't need to do anything special before/while deserialising and all defaults can be declared in protobuf with ext.default
. Anyone looking at a config can directly see what values are actually used.
Problems:
-
The zero value has a meaning (disabled) for some config elements.
-
Sometimes the default is only known at runtime (e.g. based on number of CPU cores).
Applying defaults on unset fields only works fine for JSON with a bit of care, but is a pain if possible at all with XML [1]. In protobuf there's no such thing as an unset field (it's just a zero field). The only option that's consistent between all these is to not set defaults when deserialising. That's consistent with using explicit defaults and zero always being replaced by the default in a later step after deserialisation.
For elements where some value means disabled, instead represent that meaning by an additional config element (as in FSWatcherEnabled
and FSWatcherDelayS
).
Downside: Ugly and verbose. And requires changes by config users.
For elements where the value does not unambiguously represent its meaning, make them a type with. E.g. something generic like having a state
field that has a value from an enum with values like enabled
, disabled
, unlimited
(and maybe default
for runtime defaults). Or just element specific types with simple fields (simple as in bool, int, ... fields, not enums). The advantage is that all meaning is in the config declaration, no meaning encoded in value i.e. a config user doesn't need to know what the value means.
Downside: Requires changes by config users.
- Move all default values currently specified as variables/constants in Go to an
ext.default
field in protobuf. Apply these values on directly on config elements (instead of translating the zero value to the default at runtime). Unless of course the zero value has a meaning of it's own (see Always use explicit defaults).
[1] Details are gory, but the main gist is: The approach with JSON is deserialising once to get a [][]byte
for all the folders, then iterating over that and applying defaults before deserialising individual folders. Doing something similar with xml's ,innerxml
fail because of the "inner": We lose the attributes. That can likely be solved using ,any,attr
to get attributes and apply them after deserialisation, but it becomes complicated and xml specific.