jaydoane commented on code in PR #4813:
URL: https://github.com/apache/couchdb/pull/4813#discussion_r1408677840
##########
src/config/src/config.erl:
##########
@@ -315,6 +315,19 @@ handle_call({delete, Sec, Key, Persist, Reason}, _From,
Config) ->
_ ->
ok
end,
+ % If persistent, there will always be a delete marker just written
+ % in the last .ini file, so only reload in the non-persistent case.
+ Persist orelse
+ begin
+ IniMap = ini_map(Config#config.ini_files),
+ case maps:find({Sec, Key}, IniMap) of
+ % ?DELETE markers not inserted
+ {ok, Val} when is_list(Val) ->
+ true = ets:insert(?MODULE, {{Sec, Key}, Val});
+ _ ->
+ ok
+ end
Review Comment:
With the current behavior, since we don't reload anything, the value is
`undefined` following a delete (until reload or restart ofc).
With the new behavior, in the `Persist == true` case, it's still `undefined`
due to the delete marker, so the only change is for `Persist == false` where,
as you point out, it reloads the last non deleted .ini value. I demonstrated
that behavior in this test:
```erlang
non_persistent_set_delete_reload_restart() ->
Persist = false,
?assertEqual("local.ini", config:get("section", "file")),
?assertEqual(ok, config:set("section", "file", "memory", Persist)),
?assertEqual("memory", config:get("section", "file")),
?assertEqual(ok, config:delete("section", "file", Persist)),
?assertEqual("local.ini", config:get("section", "file")),
```
It may seem odd, but at least it's consistent now, since neither case
changes after reload or restart.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
