Skip to content
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

fix initial cursor in delete script #253

Merged
merged 3 commits into from
Aug 22, 2024
Merged

Conversation

minrk
Copy link
Member

@minrk minrk commented Aug 21, 2024

empty string is not a valid cursor in latest redis-server (seems valid in 7.2.5, not 7.4, but not 100% sure), unclear if it ever should have been.

repeat...until always executes block at least once, so didn't need the empty string to not match the until condition.

logging closure fixed as well

closes #252

empty string is not a valid cursor in latest redis-server,
unclear if it ever should have been

repeat...until always executes block at least once

logging closure fixed as well
- redis.close deprecated in favor of redis.aclose
- make sure to close external redis proxy client to avoid warnings during teardown
@minrk
Copy link
Member Author

minrk commented Aug 21, 2024

I also fixed the warnings about unclosed connections caused by not closing the redis client in the external_redis fixture, and updated to use redis.aclose when available (assumed available in tests)

@@ -115,7 +121,7 @@ def _register_delete_script(self):
"""
_delete_lua = """
local all_keys = {};
local cursor = "";
local cursor = "0";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is the whole real fix for the bug.

@manics manics merged commit b68c0fb into jupyterhub:main Aug 22, 2024
11 checks passed
@minrk minrk deleted the redis-cursor branch August 22, 2024 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"invalid cursor script" from redis on clicking 'stop' button for server
2 participants