Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warmer blocking should be enabled on cache, rather than individual warmers #333

Closed
whitfin opened this issue Mar 22, 2024 · 2 comments · Fixed by #343
Closed

Warmer blocking should be enabled on cache, rather than individual warmers #333

whitfin opened this issue Mar 22, 2024 · 2 comments · Fixed by #343
Assignees
Milestone

Comments

@whitfin
Copy link
Owner

whitfin commented Mar 22, 2024

RIght now whether a warmer is blocking on startup or not is defined in the actual warmer itself. This is misleading, because if you then use Cachex.warm/2, you're able to warm it in the opposite way (ignoring the definition).

The way to approach this is to change the signature of warmers in the cache record to look something like this:

warmers(required: [], lazy: [])

Having them named and organized in this way makes the warmer definition clearer, and makes it clear that it can be overridden per warmer. I don't like the names above, but they're WIP until I come up with something better.

This is obviously a breaking change and as such will be included in 4.0.

@whitfin whitfin added this to the v3.7.0 milestone Mar 22, 2024
@whitfin whitfin self-assigned this Mar 22, 2024
@whitfin whitfin modified the milestones: v3.7.0, v4.0.0 Mar 23, 2024
@whitfin
Copy link
Owner Author

whitfin commented Mar 24, 2024

Perhaps required and background?

@whitfin
Copy link
Owner Author

whitfin commented Mar 26, 2024

I think I changed my mind here, because the naming is a little awkward. I think keeping the flag on the warmer itself does actually make sense - but i think that renaming it to something like required is much clearer.

If the warmer is required, it will be executed before the cache completes the supervision tree. This value will default to true. It makes some logic a little bit awkward in the actual initial warming, but I think this is the most obvious.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant