Fish Trophy

chipx86 got a fish trophy!

Fish Trophy

Conditionally reload logging settings when loading site configuration.

Review Request #14141 — Created Sept. 4, 2024 and updated — Latest diff uploaded

Information

Review Board
release-7.x

Reviewers

When first initializing Review Board, we'd make use of default logging
settings, which weren't necessarily based on stored configuration. This
initialization happened prior to loading the site configuration, with
the expectation that on the first request, we'd load again and then
reload logging settings. However, this leaves a window of time in which
incorrect settings may be applied.

The reason we didn't always unconditionally reload logging settings is
that it could be a bit slow or disruptive. The proper tradeoff, which is
implemented in this change, is to compare settings and determine whether
a reload is required.

load_site_config() now stores the original version of the logging
settings (from the Django settings, which the logging configuration
pulls from) and compares them once settings are applied. If there were
any changes, it will force a logging restart.

The middleware that tries to handle all this on the first request no
longer tells load_site_config() to do a full-reload of settings. In
reality, this only ever forced a logging restart, and was a bit of a
hack to begin with, and led to spurious log messages about loading
logging settings when new threads started up.

Tested the initial initialize() and verified that the logging
settings had taken effect.

Tested load_site_config() with and without changes to logging
settings. Verified it only reloaded logging when there was a
difference.

Commits

Files