django-constance icon indicating copy to clipboard operation
django-constance copied to clipboard

Don't call backend.set when backend.get returns None and default value is None

Open i2xS opened this issue 4 years ago • 4 comments

Hi there!

Preconditions: Any constance variable default value: None current value: None

Reading value through config.VALUE_NAME leads to backend.set('VALUE_NAME', None) call, which updates None to None.

This behaviour led us to database failure on production when 80 simultaneous value reads happen, which eventually led to simultaneous writes and deadlocks. Deadlocks led to service restarts, emitting new connections to database and new deadlocks.

i2xS avatar Mar 03 '21 17:03 i2xS

Codecov Report

Merging #444 (bd09021) into master (a16cca3) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #444   +/-   ##
=======================================
  Coverage   82.11%   82.11%           
=======================================
  Files          15       15           
  Lines         548      548           
  Branches       88       88           
=======================================
  Hits          450      450           
  Misses         65       65           
  Partials       33       33           
Impacted Files Coverage Δ
constance/base.py 86.95% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a16cca3...bd09021. Read the comment docs.

codecov[bot] avatar Mar 03 '21 18:03 codecov[bot]

@i2xS please include a test so we can merge this.

camilonova avatar Oct 21 '21 18:10 camilonova

@i2xS should we close this one?

camilonova avatar Mar 11 '22 19:03 camilonova

Any reason not to merge this? Seems like this should be the desired behavior, and it's still relevant given this.

MichaelDanielTom avatar Aug 29 '22 18:08 MichaelDanielTom

@i2xS @MichaelDanielTom can you provide a test so we can merge this?

camilonova avatar Jun 27 '23 20:06 camilonova