django-redis icon indicating copy to clipboard operation
django-redis copied to clipboard

fix incorrect namespacing applied to Redis hash field keys

Open 2ykwang opened this issue 8 months ago • 6 comments

This changes addresses an issue where namespacing was applied to the field instead of the key.

behavior:

  • before: Namespacing was applied to the field parameter (e.g, hash key) when it should have been applied to the key parameter.
  • after: Namespacing is correctly applied to the key parameter, leaving the field name unaltered.

2ykwang avatar Mar 08 '25 08:03 2ykwang

Codecov Report

Attention: Patch coverage is 63.63636% with 4 lines in your changes missing coverage. Please review.

Project coverage is 38.0%. Comparing base (f9f5d0c) to head (a3a4665). Report is 36 commits behind head on master.

Files with missing lines Patch % Lines
django_redis/client/default.py 63.7% 1 Missing and 3 partials :warning:
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #765     +/-   ##
========================================
+ Coverage    37.9%   38.0%   +0.1%     
========================================
  Files          44      44             
  Lines        3253    3257      +4     
  Branches      244     244             
========================================
+ Hits         1231    1235      +4     
  Misses       1778    1778             
  Partials      244     244             
Flag Coverage Δ
mypy 38.0% <63.7%> (+0.1%) :arrow_up:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

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

codecov[bot] avatar Mar 08 '25 08:03 codecov[bot]

should I look into something specific about the GitHub CI failure? @WisdomPill

2ykwang avatar Mar 10 '25 00:03 2ykwang

hello @2ykwang,

thanks for your contribution!

so only changelog and ruff are left to fix:

  • changelog needs a file called changelog.d/765.bugfix because this PR is number 765 and its reason is a bug fix, inside the file write a description of what the PR is fixing, this will later be put into the changelog when releasing automatically with towncrier
  • as for ruff you can remove ANN101 and add A004 here and it will solve itself

WisdomPill avatar Mar 11 '25 07:03 WisdomPill

I’ve added the changelog @WisdomPill

2ykwang avatar Mar 11 '25 14:03 2ykwang

also FYI, there's no need to mention me in a comment, just ask for a review, I will see the notifications and I will be happy to review 😄

WisdomPill avatar Mar 11 '25 16:03 WisdomPill

LGTM, I wonder how tests didn't break. Could you try using different version and checking the key in the tests?

because hset, hexists, and all other methods that require keys use the same namespacing logic via make_key, ensuring consistent key generation. and there are no test cases that specifically verify the namespacing of hash key name

2ykwang avatar Mar 12 '25 04:03 2ykwang