-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#2537] Implement a multi-zgw backend proxy to the individual clients #1281
[#2537] Implement a multi-zgw backend proxy to the individual clients #1281
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1281 +/- ##
===========================================
- Coverage 44.26% 44.22% -0.04%
===========================================
Files 972 972
Lines 35414 35571 +157
===========================================
+ Hits 15677 15733 +56
- Misses 19737 19838 +101 ☔ View full report in Codecov by Sentry. |
0a8095e
to
ca2b643
Compare
return resp.json() | ||
|
||
def setUp(self): | ||
self.a_url = "http://foo/bar/rows" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.a_url = "http://foo/bar/rows" | |
self.url_1 = "http://foo/bar/rows" |
885beb2
to
2d3ad96
Compare
fbda08a
to
6378e7f
Compare
src/open_inwoner/openzaak/clients.py
Outdated
def failing_responses(self) -> list[ZgwClientResponse]: | ||
return list(r for r in self if r.exception is not None) | ||
|
||
@cached_property |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is workable in the context of uwsgi, where we have threading active within the same process. In-memory caching without taking this into account is a bit of a minefield, please err on the side of caution considering our domain
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I naively thought cached_property
was thread-safe, which it is, but apparently the problem here is not so much with cache consistency as it is with some pretty severe thread contention issues, which was fixed in Python 3.12 (see the note in the docs at the end of the cached_property
section). In any case, this was all just generally interesting FYI, but I take your point: even a small risk of cache consistency troubles here is not worth it, and premature, marginal optimization was not called for here in any case. I've changed them to plain @property
fields.
6378e7f
to
84c6b2c
Compare
No description provided.