Add errorhandling for missing cache in ranked statistics #2563
Conversation
Codecov Report
@@ Coverage Diff @@
## 5.6.x #2563 +/- ##
==========================================
- Coverage 53.75% 53.72% -0.04%
==========================================
Files 558 558
Lines 40587 40605 +18
==========================================
- Hits 21819 21815 -4
- Misses 18768 18790 +22
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
I haven't run a manual test (yet), but this looks solid enough 😄 I only have two inline suggestions.
Also, one big request: This really solves a problem with a new function that was introduced in 5.6.0. IMO, this belongs in a 5.6.1 patch release, so this should be rebased to the 5.6.x branch and the base of this PR updated to that branch.
python/nav/web/sortedstats/views.py
Outdated
| if result and not result.data: | ||
| result = None | ||
| except InvalidCacheBackendError as e: | ||
| _logger.error("Error accessing cache for ranked statistics: %s".format(e)) |
There was a problem hiding this comment.
This log statement appears to use some inconsistent string formatting techniques. As always, when using loggers, I suggest using the built-in string formatting of the logger object, which ensures that time is spent to format the arguments only if the current log level dictates that this log record should be emitted:
| _logger.error("Error accessing cache for ranked statistics: %s".format(e)) | |
| _logger.error("Error accessing cache for ranked statistics: %s", e) |
python/nav/web/sortedstats/views.py
Outdated
| cache = get_cache() | ||
| cache.set(cache_key, result, timeout=timeout) | ||
| except InvalidCacheBackendError as e: | ||
| _logger.error("Error accessing cache for ranked statistics: %s".format(e)) |
There was a problem hiding this comment.
This log statement appears to use some inconsistent string formatting techniques. As always, when using loggers, I suggest using the built-in string formatting of the logger object, which ensures that time is spent to format the arguments only if the current log level dictates that this log record should be emitted:
| _logger.error("Error accessing cache for ranked statistics: %s".format(e)) | |
| _logger.error("Error accessing cache for ranked statistics: %s", e) |
If cache with that name doesnt exist, then everything will crash when the module is imported if its defined directly
stveit
force-pushed
the
ranked-stats-caching-crash
branch
from
d35b3f0
to
199cb16
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
|
rebased on 5.6.x and base changed to such |
Fixes #2561
Error from cache not being configured should not be handled. When trying to get data from cache, it will now behave as if the cache is empty. When trying to write, it will now just skip the writing step instead of crashing.
An error message will be displayed on the ranked statistics page as well.