SQLite cache: Use requests_cache.CachedSession for better concurrency
#130
Conversation
Users on Apple M2 Max reported DB lock errors: [grafana_wtf.core] INFO: Fetching dashboards in parallel with 5 concurrent requests [requests_cache.backends.sqlite] WARNING: Database is locked in thread 6272069632; retrying (1/3) Also segmentation faults, and leaks: UserWarning: resource_tracker: There appear to be 1 leaked semaphore objects to clean up at shutdown
| from sys import modules | ||
|
|
||
| import niquests | ||
| import urllib3 | ||
|
|
||
| # Amalgamate the module namespace to make all modules aiming | ||
| # to use `requests`, in fact use `niquests` instead. | ||
| modules["requests"] = niquests | ||
| modules["requests.adapters"] = niquests.adapters | ||
| modules["requests.sessions"] = niquests.sessions | ||
| modules["requests.exceptions"] = niquests.exceptions | ||
| modules["requests.packages.urllib3"] = urllib3 |
There was a problem hiding this comment.
@Ousret: Could this monkeypatching / amalgamation code be also provided by Niquests itself, as a convenience shorthand? To be invoked, for example, like:
import niquests
niquests.patch_all()Can't patch itself while it's already loaded, hm? Can it? We barely remember it, but isn't it the same with Gevent, like that... ?
import gevent
gevent.patch_all()Edit: Ah, slightly similar, it is:
from gevent import monkey
monkey.patch_all()-- https://www.gevent.org/api/gevent.monkey.html
wdyt?
There was a problem hiding this comment.
Unfortunately, this isn't the way to go. We should not tamper with sys.module outside of a development environment.
Try this instead
import requests_cache
import niquests
class CacheSession(requests_cache.session.CacheMixin, niquests.Session):
...
if __name__ == "__main__":
s = CacheSession()
for i in range(60):
r = s.get('https://httpbin.org/delay/1')While its still not ideal, it should work more accordingly.
I am thinking about forking popular extensions and see how we can ease this.
There was a problem hiding this comment.
Thank you very much for your advise. We will submit a patch to work with requests-cache in the way you are suggesting it.
There was a problem hiding this comment.
Converged that into a patch, thanks again.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #130 +/- ##
==========================================
+ Coverage 86.50% 86.68% +0.17%
==========================================
Files 8 9 +1
Lines 1060 1074 +14
==========================================
+ Hits 917 931 +14
Misses 143 143
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Applied remarks collected from grafana-toolbox/grafana-client#146 and grafana-toolbox/grafana-wtf#130
Applied remarks collected from grafana-toolbox/grafana-client#146 and grafana-toolbox/grafana-wtf#130
Applied remarks collected from grafana-toolbox/grafana-client#146 and grafana-toolbox/grafana-wtf#130
About
@JensRichnow recently reported problems with concurrency on fast / highly concurrent machines, like the Apple M2 Max.
@JWCook suggested to use
requests_cache.CachedSessionto improve the situation.This patch aims to do just that. Thank you very much!
Details
Users on Apple M2 Max reported DB lock errors:
Also segmentation faults, and leaks:
NB: @Ousret, @JWCook: Just in case you didn't know yet -- Apparently, using Niquests together with Requests-Cache works well, at least according to the succeeding test suite.