12:24 p.m.
Hi Daniel
On 12/21/19 1:25 PM, Daniel Wagner wrote:
I think we are on the same page regarding how the method call should
behave. So it's either everything okay or something went wrong. If
something went wrong nothing will change, correct?
And with that in mind I looked at the code and if I am not mistaken the
'nothing will change' promise is not completely fulfilled. The change
wont be written on disk but some changes might have been done on the
config settings in memory.
I guess I documented the behavior more accurately in the v2 of this
patch as that was accepted. My point was that reverting the changes in
case there is even one error (with the property values) might not be the
best way to do it. Instead, writing the changes to disk when at least
one of the values has been changed, and doing the write only once,
seemed better. One of the reasons was that user changing the values
could be prompted to correct the values, if the UI checks are lacking -
or to better inform the developer that better checks for some property
values in the UI are needed.
Handling these error cases is quite challenging. It would be good to
have the property changes done as an atomic transaction, but it would
mean checking the values first, and writing a property validator for
this. For me this did not make sense as all of the properties are not
required to be present in the dict, so the property set to be changed
does not have to be complete. So there can be a small set of property
values set, which indicates that partial changes VPN property values are
allowed and thus, some values can be ignored and for those property
values that were changed the PropertyChanged signal is emitted. And if
UI does have detection that which property values were not changed, then
these can be informed to user and a reduced set with corrected values
can be sent.
After all, the main motivation was to avoid the unnecessary disk writes
when a large set of properties are to be changed at once. Depending on
the UI design, the change can be immediate after changing one value, or
the changes are sent as a whole, without comparing to the previous value
of a property at the UI level. My aim was to have this as flexible as it
seems reasonable.
However, I understand the desire to have atomic transactions here as
well in order to ensure that all requested property value changes are
done, or nothing is done. This could be a part of TODOs as well, to
extend both service.c and vpn-provider.c property value setting D-Bus
methods to have similar functionality. What do you think Daniel, should
the atomic transaction approach be applied to both?
> P.S. The first reply on 1/3 arrived to my mailbox, this 2/3 reply
> didn't :)
Argh, I've complained once more about it. If nothing happens we might
need to move to another hosting service.
Something has been apparently done as now the replies, and all other
messages sent to the list as well, are arriving to my mailbox. I think
before holidays I did not even see the v2 of the 3/3 patch in the web
app at all.
Cheers,
Jussi