-
Notifications
You must be signed in to change notification settings - Fork 53
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
Python: adds DBSIZE command #1040
Conversation
in which case the client will route the command to the nodes defined by `route`. | ||
|
||
Returns: | ||
int: The number of keys in the currently selected database. |
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.
"currently selected database" is for a standalone description where there's more than a single DB in a server.
Suggested:
int: The number of keys in the database. In the case of routing the query to multiple nodes, returns the aggregated number of keys across the different nodes.
|
||
Examples: | ||
>>> await client.dbsize() | ||
10 # Indicates there are 10 keys in the current database. |
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.
not accurate. When routing isn't provided we're sending this command to multiple nodes (all primaries), the comment should say that
See https://redis.io/commands/dbsize for more details. | ||
|
||
Commands response: | ||
int: The number of keys in the currently selected database. |
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.
int: The number of keys in the database.
async def test_dbsize(self, redis_client: TRedisClient): | ||
assert await redis_client.custom_command(["FLUSHALL"]) == OK | ||
|
||
key_value_pairs = [(get_random_string(10), "foo") for _ in range(10)] |
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.
move it under the assertion in 1268
cluster_nodes = get_first_result(cluster_nodes) | ||
replica_count = cluster_nodes.count("slave") | ||
master_count = cluster_nodes.count("master") | ||
assert await redis_client.dbsize(AllNodes()) == 10 * ( |
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.
can be flakey if some of the replicas haven't finished syncing yet.. instead you can test with SlotKeyRoute:
- flushall
- setting specific key
- routing dbsize to this specific key and making sure it returns 1
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.
you can do the same with a transaction containing flush, set, dbsize. might be a bit shorter.
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.
fix & merge
dbc39f3
to
b0ff29a
Compare
b0ff29a
to
cd0ee03
Compare
Issue #, if available:
Description of changes:
"request_policy:all_shards"
"response_policy:agg_sum"
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.