Skip to content

ref(workflows): Use WorkflowId directly instead of _ActionFilterCacheKey when caching action filters#111492

Merged
kcons merged 1 commit intomasterfrom
kcons/simplfy
Mar 25, 2026
Merged

ref(workflows): Use WorkflowId directly instead of _ActionFilterCacheKey when caching action filters#111492
kcons merged 1 commit intomasterfrom
kcons/simplfy

Conversation

@kcons
Copy link
Member

@kcons kcons commented Mar 24, 2026

Should be functionally equivalent, but less wordy.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 24, 2026
@kcons kcons marked this pull request as ready for review March 25, 2026 17:50
@kcons kcons requested a review from a team as a code owner March 25, 2026 17:50
@kcons kcons requested a review from saponifi3d March 25, 2026 17:50
@kcons
Copy link
Member Author

kcons commented Mar 25, 2026

Thoughts? Visually clearer, but arguably marginally less safe.

@saponifi3d
Copy link
Contributor

I'm down, i think the cache mapping layer gives us that layer of protection to drop the cache access. Is the hope that we can eventually fully remove CacheAccess or keeping it around for composite keys?

@kcons
Copy link
Member Author

kcons commented Mar 25, 2026

I'm down, i think the cache mapping layer gives us that layer of protection to drop the cache access. Is the hope that we can eventually fully remove CacheAccess or keeping it around for composite keys?

Might be misunderstanding, but CacheAccess is already gone. This change is moving from a wrapper type around WorkflowId as a key to just using WorkflowId directly; this is intentionally supported in the CacheMapping design, as the key derivation lives in the class and not in the key type, which means we can use anything comparable as a key. It just so happens that many keys are composite and made up of one or more ints, and for those it's much clearer to have an explicit key type so we always do, but in the case of single items (or unambiguous existing types) we don't really need the cache class wrapper.

@kcons kcons merged commit f51d314 into master Mar 25, 2026
83 checks passed
@kcons kcons deleted the kcons/simplfy branch March 25, 2026 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants