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

feat(processing_engine): Implement Processing Engine Cache. #26111

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jacksonrnewhouse
Copy link
Contributor

Closes #26093.

This implements caches that are available to plugin code in-line with the specs in the ticket. In particular, users can store any python objects under a string key, storing it either to the "global" or "local" cache, where the local cache is namespaced to the individual trigger. Additionally, for the test endpoints those will be swapped in for namespaced caches with a fallback TTL of 30 minutes.

This is currently working, only things left to do are write tests and delete caches when triggers are disabled or deleted.

Copy link
Member

@pauldix pauldix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for a POC. Although I am concerned about how it's going to perform if there are many cache accesses. Wondering if the locking setup is going to work ok.


#[getter]
fn cache(&self) -> PyResult<PyCache> {
GLOBAL_CACHE_STORE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this?

false
}

fn cleanup(&mut self) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function seems like it's going to be problematic if we get into a situation with many keys in the cache. Might not be a problem, but something to look out for. Figuring out some way to look for expirations without holding a write lock might be good for later.

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.

Plugin Cache API
2 participants