instead of just returning the value for the default alias
that this wasn't done was a bug generally (the per-alias limits
simply didn't apply) but was exposed when running the CI against
non-sqlite databases, because in that setup the main database
would not get a limit set, but because the getter fell back
to the default (which wasn't set) there was an exception.
Counting incurs looking at all records which is too expensive if you have e.g.
1_000_000 issues.
Note that we take a different approach than the one for Events (where we
count-with-timeout). Reason for switching:
https://sqlite.org/forum/forumpost/fa65709226
For Events we have a known count for the non-query case (denormalized/counted
value), so we preserve what we had there. For Issues the trouble of keeping
counts right for muted/etc. is not (currently) worth it.
avoids crashing on "both true" when only `EMAIL_USE_TLS` is explicitly configured
> In your setup those are both true because the former is true by default,
> and the latter is what you have specified. Set the first to false and you
> should be good.
See #86
this is what makes sense given that:
* it's the lesser used of the to EMAIL_USE_xxx variables
* the two settings are mutually exclusive
This was simply wrong-when-introduced in 921d5bd4a3
See #86 (recent discussion of someone running into a related problem).
This commit fixes 3 related issues with the way runtime_limit was administered;
which could lead to race conditions (and hence: the wrong runtime_limit
applying at some point in time). Post-fix, the folllowing holds:
1. We use thread_locals to store this info, since there are at least 2 sources of
threaded code that touch this (snappea's workers and the django debugserver)
2. We distinguish between the "from connection settings" timeout and the
"temporarily overridden" ones, since we cannot assume
connection-initialization happens first (as per the comment in base.py)
3. We store runtime-limits per alias ('using'). Needed for [2] (each connection
may have a different moment-of-initialization, clobbering CM-set values from
the other connection) and also needed once you realize there may be
different defaults for the timeouts.
General context: I've recently started introducing the 'different runtime'
helper quite a bit more; and across connections (snappea!), which created more
and more doubts as to it actually working as advertised.
Thoughts on "using" being required. I used to think "you can reason about a
global timeout value, and the current transaction makes clear what you're
actually doing", but as per the notes above that doesn't really work.
Thoughts on reproducing:
A few thoughts/notes on reproducing problems with race conditions. Basic note:
that's always hairy. So in the end I settled on a solution that's hopefully
easy to reason about, even if it's verbose.
When I started work on this commit, I focussed on thread-safety; "proving the
problem" consisted of F5/^R on a web page with 2 context managers with different
timeouts, hoping to show that the stack unrolling didn't work properly.
However, during those "tests" I noticed quite a few resets-to-5s (from the
connection defaults), which prompted fix [2] from above.
Given what immediate_semaphore _is_, the principled need for this is obvious.
In practice, this wasn't needed yet (immediate_atomic is not used for
any non-default DB). But better fix it before we do decide on that.
it was reporting the actual sqlite get-write-lock timings, but since the
introduction of the immediate_semaphore those were 'basically never' blocking
since all the blocking was done in the semaphore (ignoring cross
gunicorn/snappea details)
Klaas van Schelven