-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Add DriverInfo class for upstream driver tracking #3880
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
base: master
Are you sure you want to change the base?
Conversation
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.
Edited: Sorry - github initially showed just one small formatting change. Will review it in details tomorrow.
Didn't go through all the changes.
petyaslavova
left a comment
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 think we should unify driver metadata handling:
-
Store the redis-py client version in DriverInfo.
If a DriverInfo object is provided, ignore lib_name and lib_version - it should be clearly stated in the docstrings. -
Propagate DriverInfo through pools and connections.
Pools should hold a driver_info attribute and pass it down so each connection reads driver fields only from that object. -
Fallback behavior:
If no DriverInfo is provided, initialize one using the existing lib_name / lib_version. -
Deprecate lib_name and lib_version.
Mark them as deprecated now and plan to remove them after a few releases.(we have an annotation that can be used to deprecate concrete arguments)
This will keep the driver info consistent, will remove duplicated parameters, and simplify the API going forward.
| if `True`, the response will be decoded to utf-8. | ||
| Argument is ignored when connection_pool is provided. | ||
| driver_info: | ||
| Optional DriverInfo object to identify upstream libraries. |
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 current implementation causes to have lib_name to be ignored - it will be valuable addition to the details in the docstrings.
Description of change
This PR adds a DriverInfo class to enable upstream drivers (like django-redis, celery, rq, etc.) to identify themselves when using redis-py as their underlying client.
Key features:
Example usage:
Pull Request check-list