fix: hash method parameter naming and key handling#812
Open
oliverhaas wants to merge 2 commits intojazzband:masterfrom
Open
fix: hash method parameter naming and key handling#812oliverhaas wants to merge 2 commits intojazzband:masterfrom
oliverhaas wants to merge 2 commits intojazzband:masterfrom
Conversation
oliverhaas
force-pushed
the
refactor/fix-hash-method-parameters
branch
from
b2b95a8 to
57386d6
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #812 +/- ##
========================================
- Coverage 38.2% 38.0% -0.1%
========================================
Files 48 49 +1
Lines 3727 3771 +44
Branches 301 301
========================================
+ Hits 1420 1430 +10
- Misses 2006 2040 +34
Partials 301 301
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
oliverhaas
changed the title
Changed hash method parameters to align with Redis/Valkey terminology: - Renamed 'name' → 'key' (the Redis key for the hash) - Renamed 'key' → 'field' (the field within the hash) - Changed type from 'str' to 'KeyT' for consistency with other methods - Added 'version' parameter support to hlen() and hkeys() Fixed make_key() application: - make_key() now only applied to hash key (first parameter) - Fields are no longer transformed with make_key() - This fixes the namespacing issue where fields were incorrectly prefixed - hkeys() now returns plain field names without reverse_key processing Added comprehensive test for version support across all hash methods. This improves API clarity and consistency with Redis/Valkey documentation where 'key' refers to the Redis key (name of the hash structure) and 'field' refers to a field within that hash. Changed methods: - hset(key, field, value, ...) - hdel(key, field, ...) - hlen(key, ...) - hkeys(key, ...) - hexists(key, field, ...) All hash-related tests pass with the new implementation. Co-authored-by: 2ykwang <[email protected]>
oliverhaas
force-pushed
the
refactor/fix-hash-method-parameters
branch
from
57386d6 to
d95e930
Compare
Moved all hash operation tests from test_backend.py to a new dedicated test_backend_hash.py file for better organization and maintainability. Tests moved: - test_hset - test_hdel - test_hlen - test_hkeys - test_hexists - test_hash_version_support - test_hash_key_structure_in_redis All 84 hash tests (7 tests × 12 cache configurations) pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changed hash method parameters to align with Redis/Valkey terminology:
NOTE: This is the naming convention of the Redis and Valkey docs, and unfortunately redis-py code has the naming differently, but I would go with the former, which also fits more with Django's naming.
Fixed make_key() application including version kwarg:
See also #765, and I tried to credit 2ykwang in the commits, hopefully that works. Without the naming changes above their PR was a bit confusing though, so I packed this into one.
Let me know if I should change something here.