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: Cache Manager #378

Merged
merged 15 commits into from
Nov 21, 2024
Merged

Conversation

bmgalego
Copy link
Contributor

This PR introduces a new cache manager to the agent runtime, centralizing most of the caching logic into a dedicated manager. Currently, this implementation only covers the following caches, twitter cache, discord cache, solana cache, browser and video cache and does not include files like video, audio and other attachments.
The cache manager provides a straightforward API (still a work in progress) that uses a cache adapter. For now using a filesystem adapter to test it awaiting for feedback to start writing the schemas and db adapters.
The caching api uses a key and the agentId to index the entries allowing for multi agent caching.
We could also include in the api an extra argument for expiration.

Relates to:

#243
#258
This pr should fix this issue since the multi agent twitter caching no longer conflicts, which was likely the root cause of the bug.

Risks

medium – this affects multiple packages.

Background

What kind of change is this?

Feature: Introduces a new caching manager.
Improvements: Refactors and centralizes caching logic for multiple packages.
Bug Fix: Resolves conflicts in multi-agent Twitter caching

Documentation changes needed?

My changes require a change to the project documentation.

Testing

Where should a reviewer start?

packages/core/src/cache.ts – Review the API implementation.
packages/agent/src/index.ts – Check how the cache manager is instantiated and integrated.
packages/client-twitter/src/base.ts – Verify the usage of the cache manager API in the Twitter client.

Detailed testing steps

@ponderingdemocritus
Copy link
Contributor

good idea and correct direction - lets make this work then merge up

@bmgalego
Copy link
Contributor Author

Added some basic tests and implemented the expiration api.
I think initialy we could go without the database adapters, starting with the filesystem should be easier to migrate too. I am building a script to help the migration.

@bmgalego bmgalego mentioned this pull request Nov 19, 2024
4 tasks
Copy link
Contributor

@ponderingdemocritus ponderingdemocritus left a comment

Choose a reason for hiding this comment

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

epic PR - some conflicts. Could adjust all logging to use elizaLogger.

I don't have many other comments on implementation. Lets test in the wild across all services and clients to make sure working as expected

@bmgalego bmgalego changed the title feat: Cache Manager (WIP) feat: Cache Manager Nov 20, 2024
@ponderingdemocritus ponderingdemocritus merged commit a1519b2 into elizaOS:main Nov 21, 2024
2 checks passed
@monilpat
Copy link
Collaborator

Thanks so much for working on this! Looks like there are some typing issues that I am fixing as a part of merging in my latest changes just wanted to give a heads up

@monilpat
Copy link
Collaborator

monilpat commented Nov 21, 2024

I think to prevent issues like this we need to make sure every package as a tsup.config.ts which agent was missing 2) and as per the docs run pnpm build

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.

3 participants