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
Removed Redis parser_class reference from docs. #18052
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.
Hello! Thank you for your contribution 💪
As it's your first contribution be sure to check out the patch review checklist.
If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!
If you have any design or process questions then you can ask in the Django forum.
Welcome aboard ⛵️!
Interesting. redis-py still has a parser_class option but the classes seem to have been made private from 4.4.x onwards as they are all prefixed with "_" and the parsers section was removed from the README.md. I don't think it's helpful to remove the hyperlink, rather someone with more knowledge could suggest a correct link. |
unfortunately there is not link that describe the parser_class (i found one but it's for another third-party lib) |
I think it's okay to remove it when redis-py removes the documentation for the parser classes. The redis-py documentation says that redis-py automatically uses the hiredis-py parser class if you install hiredis-py. This tells us that we have two options:
|
docs/topics/cache.txt
Outdated
@@ -563,7 +563,7 @@ flag on the connection's socket:: | |||
|
|||
Here's an example configuration for a ``redis`` based backend that selects | |||
database ``10`` (by default Redis ships with 16 logical databases), specifies a | |||
`parser class`_ (``redis.connection.HiredisParser`` will be used by default if | |||
parser class (``redis.connection.HiredisParser`` will be used by default if |
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.
Using a custom parser class is so niche that it's not even documented in redis-py
docs. I'd remove it:
diff --git a/docs/topics/cache.txt b/docs/topics/cache.txt
index 3d67f9b03a..1fe9d335fb 100644
--- a/docs/topics/cache.txt
+++ b/docs/topics/cache.txt
@@ -562,10 +562,8 @@ flag on the connection's socket::
}
Here's an example configuration for a ``redis`` based backend that selects
-database ``10`` (by default Redis ships with 16 logical databases), specifies a
-`parser class`_ (``redis.connection.HiredisParser`` will be used by default if
-the ``hiredis-py`` package is installed), and sets a custom `connection pool
-class`_ (``redis.ConnectionPool`` is used by default)::
+database ``10`` (by default Redis ships with 16 logical databases), and sets a
+custom `connection pool class`_ (``redis.ConnectionPool`` is used by default)::
CACHES = {
"default": {
@@ -573,13 +571,11 @@ class`_ (``redis.ConnectionPool`` is used by default)::
"LOCATION": "redis://127.0.0.1:6379",
"OPTIONS": {
"db": "10",
- "parser_class": "redis.connection.PythonParser",
"pool_class": "redis.BlockingConnectionPool",
},
}
}
-.. _`parser class`: https://github.com/redis/redis-py#parsers
.. _`connection pool class`: https://github.com/redis/redis-py#connection-pools
.. _the-per-site-cache:
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.
@r3a70 Do you have time to keep working on 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.
@felixxm can you explain what i should be to do ?
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 left a diff with a suggestion in my previous 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.
ok i work on it.
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.
Thank you for this @r3a70
Trac ticket number
ticket-[number]
Branch description
Provide a concise overview of the issue or rationale behind the proposed changes.
Checklist
main
branch.