[ENG-740] 2/6 - DSR cache storage by johnewart · Pull Request #7463 · ethyca/fides

johnewart · 2026-02-23T23:24:50Z

Description Of Changes

Adds a DSR cache store on top of the indexing RedisCacheManager that has well-known operations to avoid "arbitrary" key names and ensures consistency. Does not integrate the new store with any APIs or other code, this is just the DSR cache itself.

Code Changes

Adds a DSR cache store, does not integrate it yet

Steps to Confirm

No manual steps, has tests.

Pre-Merge Checklist

vercel · 2026-02-23T23:24:55Z

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
fides-plus-nightly	Ignored	Preview	Mar 19, 2026 8:48pm
fides-privacy-center	Ignored		Mar 19, 2026 8:48pm

greptile-apps · 2026-02-23T23:27:27Z

Greptile Summary

Introduces a DSR cache store on top of the RedisCacheManager with standardized key naming (dsr:{id}:{part}) and lazy migration from legacy key formats. The store provides type-specific convenience methods for DSR-related cache operations and maintains an index for efficient listing and clearing.

Key changes:

New DSRCacheStore class with convenience methods for identity, custom fields, encryption, DRP, masking secrets, async execution, and retry count
KeyMapper utility to map between legacy and new key formats for backward compatibility
Comprehensive test coverage including in-memory implementations and migration scenarios
Test fixtures to prevent full app/DB startup for cache-only tests

Confidence Score: 3/5

Safe to merge with minor documentation fix needed
The implementation is solid with comprehensive tests and clear migration strategy. However, the documentation references a non-existent function, and the SCAN pattern usage could potentially match unintended keys (though this is noted in previous review threads).
Pay attention to src/fides/common/cache/__init__.py (fix documentation) and src/fides/common/cache/dsr_store.py (SCAN pattern already flagged in previous threads)

Important Files Changed

Filename	Overview
src/fides/common/cache/init.py	Added `DSRCacheStore` and `DSR_KEY_PREFIX` to exports, but the documented example function `get_dsr_cache_store()` doesn't exist
src/fides/common/cache/dsr_store.py	New DSR cache store with lazy migration support; SCAN pattern `{dsr_id}` in `get_all_keys()` and `clear()` could match unintended keys
src/fides/common/cache/key_mapping.py	Maps legacy Redis key patterns to new DSR store format; clean implementation with clear documentation

_{Last reviewed commit: 48c0bb7}

greptile-apps

_{7 files reviewed, 5 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-23T23:27:31Z

src/fides/common/cache/dsr_store.py

+        keys = self._manager.get_keys_by_index(index_prefix)
+        if keys:
+            return keys
+        legacy_keys = list(self._redis.scan_iter(match=f"*{dsr_id}*", count=500))


SCAN pattern *{dsr_id}* could match unintended keys. If a DSR ID like "abc" exists, keys containing "abc" anywhere (e.g., dsr:xyz-abc-123:field or some-other-key-abc) will be matched. Use anchored pattern like dsr:{dsr_id}:* for new keys, and handle legacy separately if needed.

This is by design to keep consistency with the existing way keys are globbed by the DSR clearing and not break existing data that may be in Redis until everything moves over to the new index method

greptile-apps · 2026-02-23T23:27:31Z

src/fides/common/cache/dsr_store.py

+        complete cleanup in mixed-key scenarios.
+        """
+        # Use SCAN to find ALL keys (indexed + legacy)
+        all_keys_via_scan = list(self._redis.scan_iter(match=f"*{dsr_id}*", count=500))


Same SCAN pattern issue here. *{dsr_id}* could delete unrelated keys that happen to contain the DSR ID substring. Use dsr:{dsr_id}:* for new keys and handle legacy keys with separate patterns.

tests/common/cache/test_dsr_store.py

greptile-apps · 2026-02-23T23:27:39Z

Additional Comments (1)

tests/common/cache/test_dsr_store.py
Variable s should use a more descriptive name

    def remove_key_from_index(self, index_prefix: str, key: str) -> None:
        index_set = self._index.get(index_prefix)
        if index_set is not None:
            index_set.discard(key)

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

JadeCara · 2026-02-24T17:02:34Z

src/fides/common/cache/__init__.py

+from contextlib import contextmanager
+from typing import Iterator


These don't seem to be used (yet) - do we need them imported here?

JadeCara

Overall looks good! I am excited for this change :)

Deserialize keys to strings when in `get_keys_by_index` - preferring deliberate conversion over `decode_responses` Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

greptile-apps

_{7 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

codecov · 2026-02-24T23:06:02Z

Codecov Report

❌ Patch coverage is 0% with 168 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.28%. Comparing base (b1a43cc) to head (48c0bb7).
⚠️ Report is 7 commits behind head on johnewart/ENG-740/0-redis-cache-manager.

Files with missing lines	Patch %	Lines
src/fides/common/cache/dsr_store.py	0.00%	90 Missing ⚠️
src/fides/common/cache/key_mapping.py	0.00%	74 Missing ⚠️
src/fides/common/cache/__init__.py	0.00%	4 Missing ⚠️

❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project check has failed because the head coverage (84.28%) is below the target coverage (85.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@                             Coverage Diff                             @@
##           johnewart/ENG-740/0-redis-cache-manager    #7463      +/-   ##
===========================================================================
- Coverage                                    85.53%   84.28%   -1.25%     
===========================================================================
  Files                                          564      564              
  Lines                                        37812    37910      +98     
  Branches                                      4399     4403       +4     
===========================================================================
- Hits                                         32341    31951     -390     
- Misses                                        4479     4956     +477     
- Partials                                       992     1003      +11

☔ 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.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

johnewart added 5 commits February 23, 2026 14:59

Add base redis cche manager

cb2f89d

Add pipelining to index operations for atomicity

990dfde

Add tests and make index operations pipelined

60bd3ff

Formatting fixes

2231959

Cleanup tests

f9bb52e

johnewart requested a review from JadeCara February 23, 2026 23:24

johnewart requested a review from a team as a code owner February 23, 2026 23:24

greptile-apps bot reviewed Feb 23, 2026

View reviewed changes

Add tests for index operations

64a5f7f

JadeCara reviewed Feb 24, 2026

View reviewed changes

JadeCara approved these changes Feb 24, 2026

View reviewed changes

johnewart and others added 9 commits February 24, 2026 10:36

Update src/fides/common/cache/manager.py

901effd

Deserialize keys to strings when in `get_keys_by_index` - preferring deliberate conversion over `decode_responses` Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

Add TTL management (opt-in) to manager

2d3d6e6

Add new tests dir to noxfiles

bbd2926

Formatting

fe7bd8b

Add changelog

b1a43cc

Add DSR cache store + tests

34b86a2

Add DSR store to exports

5a80d71

Formatting fixes

9365451

Dedupe import and add a TODO about moving it

48c0bb7

johnewart marked this pull request as draft February 24, 2026 22:23

johnewart marked this pull request as ready for review February 24, 2026 22:23

johnewart force-pushed the johnewart/ENG-740/1-dsr-cache-storage branch from 90adcf8 to 48c0bb7 Compare February 24, 2026 22:23

greptile-apps bot reviewed Feb 24, 2026

View reviewed changes

Base automatically changed from johnewart/ENG-740/0-redis-cache-manager to main February 24, 2026 23:09

ethyca deleted a comment from greptile-apps bot Feb 25, 2026

johnewart and others added 2 commits February 24, 2026 17:23

Update tests/common/cache/test_dsr_store.py

ebfd156

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

Merge branch 'main' into johnewart/ENG-740/1-dsr-cache-storage

9a8bfd1

vercel bot deployed to Preview – fides-plus-nightly February 25, 2026 19:37 View deployment

Merge branch 'main' into johnewart/ENG-740/1-dsr-cache-storage

c5b6886

vercel bot deployed to Preview – fides-plus-nightly March 19, 2026 19:32 View deployment

johnewart added 3 commits March 19, 2026 12:39

Fix merge error

db882eb

Fix another merge issue

6965fb3

Add missing mock pipeline

1a3fdf6

johnewart enabled auto-merge March 19, 2026 21:05

johnewart added this pull request to the merge queue Mar 19, 2026

Merged via the queue into main with commit 40ed936 Mar 19, 2026
55 of 57 checks passed

johnewart deleted the johnewart/ENG-740/1-dsr-cache-storage branch March 19, 2026 21:14

		from contextlib import contextmanager
		from typing import Iterator

Conversation

johnewart commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description Of Changes

Code Changes

Steps to Confirm

Pre-Merge Checklist

Uh oh!

vercel bot commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

johnewart Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

johnewart Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot commented Feb 23, 2026

Uh oh!

JadeCara Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

JadeCara left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Feb 24, 2026

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

johnewart commented Feb 23, 2026 •

edited

Loading

vercel bot commented Feb 23, 2026 •

edited

Loading

greptile-apps bot commented Feb 23, 2026 •

edited

Loading