Skip to content

Match async Redis interface with sync + introduce aioredis integration tests#293

Merged
ekzhu merged 8 commits intoekzhu:masterfrom
Varun0157:async-redis-tests
Dec 26, 2025
Merged

Match async Redis interface with sync + introduce aioredis integration tests#293
ekzhu merged 8 commits intoekzhu:masterfrom
Varun0157:async-redis-tests

Conversation

@Varun0157
Copy link
Copy Markdown
Contributor

@Varun0157 Varun0157 commented Dec 1, 2025

Similar to #284, we parametrize the existing tests in the experimental aio layer to run for both MongoDB and Redis storage layers.

On running the tests, the following issues were discovered and resolved:

  • The AsyncRedisStorage did not have a close method - this has been added

The following disparities from the sync storage were discovered and resolved:

  • a buffer parameter was added to remove() and remove_val() to allow for buffered deletion and match the sync interface
  • Added validation to ensure that only bytes keys are allowed if prepickle is False.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR extends the async storage layer to support Redis alongside MongoDB, matching the async Redis interface with the sync version, and introduces parametrized integration tests that run against both storage backends.

Key changes:

  • Parametrized async LSH tests to run for both MongoDB (aiomongo) and Redis (aioredis) storage backends
  • Added missing close() method to AsyncRedisStorage for proper resource cleanup
  • Enhanced remove() and remove_val() methods in async Redis storage to support buffered deletion via buffer parameter, matching the sync interface
  • Added validation to enforce bytes-only keys when prepickle=False, with helpful error messages

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
uv.lock Adds pytest-asyncio and backports-asyncio-runner dependencies for async test support
test/aio/test_lsh_mongo.py Removed - consolidated into parametrized test suite
test/aio/test_lsh.py New parametrized test file supporting both MongoDB and Redis storage backends, migrated from unittest to pytest
pyproject.toml Adds pytest-asyncio test dependency and configures asyncio_mode = "auto"
datasketch/experimental/aio/storage.py Implements close() method for AsyncRedisStorage and adds buffer parameter to remove() and remove_val() methods
datasketch/experimental/aio/lsh.py Adds bytes key validation for prepickle=False mode and updates __setstate__ to preserve prepickle setting during unpickling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ekzhu ekzhu merged commit 18bac67 into ekzhu:master Dec 26, 2025
8 checks passed
@ekzhu
Copy link
Copy Markdown
Owner

ekzhu commented Dec 26, 2025

@Varun0157 thanks for the PR! Apologies for the late response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants