Skip to content

Conversation

@oliverhaas
Copy link
Contributor

@oliverhaas oliverhaas commented Dec 24, 2025

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)

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:

  • make_key() now only applied to hash key (first parameter)
  • Fields are no longer transformed with make_key()
  • version kwarg was also missing from some methods

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.

@oliverhaas oliverhaas force-pushed the refactor/fix-hash-method-parameters branch from b2b95a8 to 57386d6 Compare December 24, 2025 13:05
@codecov
Copy link

codecov bot commented Dec 24, 2025

Codecov Report

❌ Patch coverage is 16.30435% with 77 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.0%. Comparing base (755dfb2) to head (6756c9a).

Files with missing lines Patch % Lines
tests/test_backend_hash.py 13.3% 72 Missing ⚠️
django_redis/client/default.py 44.5% 1 Missing and 4 partials ⚠️
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             
Flag Coverage Δ
mypy 38.0% <16.4%> (-0.1%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@oliverhaas oliverhaas changed the title refactor: fix hash method parameter naming and key handling fix: hash method parameter naming and key handling Dec 24, 2025
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 <yk2ykwang@gmail.com>
@oliverhaas oliverhaas force-pushed the refactor/fix-hash-method-parameters branch from 57386d6 to d95e930 Compare December 24, 2025 16:45
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.
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.

1 participant