Skip to content

Support Redis 8 #282

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

Merged
merged 5 commits into from
Feb 13, 2025
Merged

Support Redis 8 #282

merged 5 commits into from
Feb 13, 2025

Conversation

tylerhutcherson
Copy link
Collaborator

@tylerhutcherson tylerhutcherson commented Feb 13, 2025

This PR adds testing support for Redis 8 (currently redis:8.0-M03 from Docker) to our GitHub Actions pipeline. Overall, our entire test suite passed against Redis 8 with one notable exception:

Breaking Change in Redis 8, Dialect 2 Only
When a search query references a field that is not present in the index schema, Redis 8 now raises:

redis.exceptions.ResponseError: Unknown field at offset NN near <unknown-field-name>

Previously (and in other dialects), Redis would allow such queries but return empty results. This new behavior is arguably more correct, but it breaks a test that expects an empty result when searching on a non-indexed field.

Context and Discussion

A single test failed where our code intentionally queries a field not in the index, expecting empty results. With Redis 8, Dialect 2, this now raises an error.
The team discussed whether to:

  • Adjust the test to skip or otherwise handle this new behavior (since querying a non-indexed field arguably should be an error).
  • Catch and raise a more user-friendly message in our library. The Redis error text can be cryptic, so providing a clearer explanation to users might be beneficial.

Decision

  • Testing: We will proceed with adding Redis 8 to our CI matrix.
  • The Failing Test: We will update this test to either skip on Redis 8 or handle the error more gracefully.
  • Error Messaging: We may consider adding a better exception message so that developers get a more understandable error when they inadvertently query non-indexed fields. But this will go in a different PR.

@tylerhutcherson tylerhutcherson added the enhancement New feature or request label Feb 13, 2025
@tylerhutcherson tylerhutcherson marked this pull request as ready for review February 13, 2025 15:44
rbs333
rbs333 previously approved these changes Feb 13, 2025
Copy link
Collaborator

@rbs333 rbs333 left a comment

Choose a reason for hiding this comment

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

👍

@tylerhutcherson tylerhutcherson merged commit f56e610 into main Feb 13, 2025
36 checks passed
@tylerhutcherson tylerhutcherson deleted the feat/RAAE-626-test-redis-8 branch February 13, 2025 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants