-
Notifications
You must be signed in to change notification settings - Fork 19
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
Feature/change redis struct #97
base: production
Are you sure you want to change the base?
Feature/change redis struct #97
Conversation
MahdiPresario001
commented
Nov 4, 2024
•
edited
Loading
edited
- Responses not longer go through redis pubsub, but a response queue is created for each query, with a TTL
- Contenders selection for organic queries:
- switch from redis to hiredis for better performance
- change factory config function to use a global var
- passing psqldb connection instead of the entire psql db variable to psql functions (contenders selection mainly)
sync with prod
|
||
_config = None | ||
|
||
async def factory_config() -> Config: |
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.
Any reason for this?
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.
Only issue with this is the cache is not configurable now.
Is there any benefit over the other solution? looks to be doing the exact same thing but with two funcs and global var as opposed to one function
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.
the psql connection errors were solved by this. was proposed by @narigama
it's cleaner this way, we only create the config once, no caching needed (which was causing many issues)
LGTM apart from the cache stuff, not sure if/why we need to do that |
d98c4b0
to
2357fcd
Compare
8e1fffe
to
d51d702
Compare