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

Firestore memory #1975

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

jeffbryner
Copy link

Description

Adds firestore memory to solve #1974


Type of change

  • [ x] New feature (non-breaking change which adds functionality)

Checklist

  • Adherence to standards: Code complies with Agno’s style guidelines and best practices.
  • Formatting and validation: You have run ./scripts/format.sh and ./scripts/validate.sh to ensure code is formatted and linted.
  • Self-review completed: A thorough review has been performed by the contributor(s).
  • Documentation: Docstrings and comments have been added or updated for any complex logic.
  • Examples and guides: Relevant cookbook examples have been included or updated (if applicable).
  • Tested in a clean environment: Changes have been tested in a clean environment to confirm expected behavior.
  • Tests (optional): Tests have been added or updated to cover any new or changed functionality.

Additional Notes

firestore works best without having to create indexes. The data model collection_name/user_id/memories allows for selecting all memories and using an orderby without having to create a composite index which is a bit tricky in firestore (requires async calls, no callback when completed, not ready immediately).

@jeffbryner jeffbryner requested a review from a team as a code owner February 1, 2025 19:09
@manthanguptaa
Copy link
Contributor

Can you please add a corresponding cookbook and add the steps to setup firestore and get the required env variables at the top of the cookbook?

@jeffbryner
Copy link
Author

Hi @manthanguptaa I've added the cookbook

Comment on lines +1 to +17
from typing import Optional, List
from datetime import datetime, timezone

try:
from google.cloud import firestore
from google.cloud.firestore import Client
from google.cloud.firestore import CollectionReference
from google.cloud.firestore_v1.base_query import FieldFilter, BaseQuery
import google.auth
except ImportError:
raise ImportError(
"`firestore` not installed. Please install it with `pip install google-cloud-firestore`"
)

from agno.memory.db import MemoryDb
from agno.memory.row import MemoryRow
from agno.utils.log import logger
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
from typing import Optional, List
from datetime import datetime, timezone
try:
from google.cloud import firestore
from google.cloud.firestore import Client
from google.cloud.firestore import CollectionReference
from google.cloud.firestore_v1.base_query import FieldFilter, BaseQuery
import google.auth
except ImportError:
raise ImportError(
"`firestore` not installed. Please install it with `pip install google-cloud-firestore`"
)
from agno.memory.db import MemoryDb
from agno.memory.row import MemoryRow
from agno.utils.log import logger
from typing import Optional, List
from datetime import datetime, timezone
from agno.memory.db import MemoryDb
from agno.memory.row import MemoryRow
from agno.utils.log import logger
try:
from google.cloud import firestore
from google.cloud.firestore import Client
from google.cloud.firestore import CollectionReference
from google.cloud.firestore_v1.base_query import FieldFilter, BaseQuery
import google.auth
except ImportError:
raise ImportError(
"`firestore` not installed. Please install it with `pip install google-cloud-firestore`"
)

def __init__(
self,
collection_name: str = "memory",
db_name: Optional[str] = "(default)",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
db_name: Optional[str] = "(default)",
db_name: Optional[str] = "agno",

Comment on lines +55 to +66
def create(self) -> None:
"""Create the collection index
Avoiding index creation by using a user/memory model

Returns:
None
"""
try:
logger.info(f"Mocked call to create index for '{self.collection_name}'")
except Exception as e:
logger.error(f"Error creating collection: {e}")
raise
Copy link
Contributor

@manthanguptaa manthanguptaa Feb 14, 2025

Choose a reason for hiding this comment

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

I noticed this doesn’t seem to be doing anything. Was that intentional? Just wanted to check.

Comment on lines +1 to +8
"""
This recipe shows how to store agent sessions in a MongoDB database.
Steps:
1. Run: `pip install openai google-cloud-firestore agno` to install dependencies
2. Make sure your gcloud project is set up and you have the necessary permissions to access Firestore
3. Run: `python cookbook/memory/09_persistent_memory_firestore.py` to run the agent
"""

Copy link
Contributor

Choose a reason for hiding this comment

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

can you please also mention steps on how to set up gcloud locally and set up the project?

Comment on lines +195 to +205
def drop_table(self) -> None:
"""Drop the collection
Returns:
None
"""
try:
# todo https://firebase.google.com/docs/firestore/solutions/delete-collections
# no easy way
logger.info("call to drop table")
except Exception as e:
logger.error(f"Error dropping collection: {e}")
Copy link
Contributor

Choose a reason for hiding this comment

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

we will need to implement this as it is an important function

Comment on lines +215 to +226
def clear(self) -> bool:
"""Clear the collection
Returns:
bool: True if the collection was cleared, False otherwise
"""
try:
# todo https://firebase.google.com/docs/firestore/solutions/delete-collections
logger.info("call to clear the collection")
return True
except Exception as e:
logger.error(f"Error clearing collection: {e}")
return False
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly here

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.

2 participants