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

Add SQL Conversation Storage #111

Merged
merged 9 commits into from
Dec 26, 2024
Merged

Add SQL Conversation Storage #111

merged 9 commits into from
Dec 26, 2024

Conversation

Rajaniraiyn
Copy link
Contributor

Issue number: #110

Summary

Changes

This PR adds a new feature, SQL Storage, to the Multi-Agent Orchestrator system. It introduces persistent conversation storage using the libsql library, supporting both local SQLite databases and remote databases. This implementation improves flexibility, reliability, and scalability for storing agent conversation history.

User experience

Before this change:

  • Conversation data was limited to DynamoDB or in-memory storage without a CustomStorage implementation.

After this change:

  • Users can store conversations persistently with support for both local and remote databases.
  • Provides robust solutions for edge and serverless deployments.
  • Simplifies setup with automatic schema creation and indexing.

Checklist

If your change doesn't seem to apply, please leave them unchecked.

  • I have performed a self-review of this change
  • Changes have been tested
  • Changes are documented
Is this a breaking change? No

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

…ction management features. Updated documentation to reflect changes in database connection handling and best practices for closing connections. Improved error handling during database initialization.
@fryz
Copy link
Contributor

fryz commented Dec 13, 2024

Any update on this PR?

I've got a similar use-case where we're using a SQL DB for storing conversations.
I've rolled my own storage engine for this outside of the multi-agent orchestrator framework, but having this built-in would be pretty nice.

Looks like the current HEAD of this fork needs a rebase. Happy to contribute if OP no longer has the bandwidth, but also not sure if the maintainers are willing to accept this. Looking for some feedback either way.

Thanks!

@cornelcroi
Copy link
Contributor

cornelcroi commented Dec 24, 2024

Hi @Rajaniraiyn,
Thanks for the PR.
We did some changes to the python version so that a user will install only the required libraries, not all of them.
Can you make changes so that it works for libsql similar to aws, anthropic and openai?
Check the setup.cfg and the init files.
The point is that we should install this lib only if we use this storage.

Also add a mention to this package in the doc for python.

Thanks.

@Rajaniraiyn
Copy link
Contributor Author

Hey @cornelcroi
I have made the requested changes,

  • made libsql installing when only used in python
  • mention sql storage in overview docs

Thanks

@cornelcroi
Copy link
Contributor

Can you add the pip install with the package like we did here ?
The users are more likely to skip the initial installation step so if they head to this storage, the installation of the lib must be recalled.

Thanks.

@Rajaniraiyn
Copy link
Contributor Author

@cornelcroi I have updated both DynamoDB and SQL storage docs for python, specifying extras dependency installation. Let me know any further changes required.
Thanks.

@cornelcroi
Copy link
Contributor

Great, thanks.
Merging.

@cornelcroi cornelcroi closed this Dec 26, 2024
@cornelcroi cornelcroi reopened this Dec 26, 2024
@cornelcroi cornelcroi merged commit 000a492 into awslabs:main Dec 26, 2024
@fryz
Copy link
Contributor

fryz commented Jan 3, 2025

Thanks for merging this :)

@fryz fryz mentioned this pull request Jan 3, 2025
5 tasks
@psqbt
Copy link

psqbt commented Jan 10, 2025

I am getting this error while using the SqlChatStorage with local db file

image

I am setting up the orchestrator in the fast api async method same as in fastapi streaming example

image

image

@cornelcroi
Copy link
Contributor

@fryz can you have a look?

@fryz
Copy link
Contributor

fryz commented Jan 10, 2025

CC @Rajaniraiyn (original contributor)

Would be happy to contribute but I'm more familiar with the Typescript side of things.
Looks like it's because the query invoking the read from the DB is async and not being handled correctly? (eg: await?)

@cornelcroi
Copy link
Contributor

CC @Rajaniraiyn (original contributor)

Would be happy to contribute but I'm more familiar with the Typescript side of things.

Looks like it's because the query invoking the read from the DB is async and not being handled correctly? (eg: await?)

Sorry, I meant to tag the original author 😊

@psqbt
Copy link

psqbt commented Jan 10, 2025

CC @Rajaniraiyn (original contributor)

Would be happy to contribute but I'm more familiar with the Typescript side of things. Looks like it's because the query invoking the read from the DB is async and not being handled correctly? (eg: await?)

@fryz Yes, async method not awaited....full trace below

image

@Rajaniraiyn
Copy link
Contributor Author

@psqbt Fixed Python async/sync inconsistencies and implemented a missing newly added method: #201

@psqbt
Copy link

psqbt commented Jan 10, 2025

Thanks @Rajaniraiyn !

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

Successfully merging this pull request may close these issues.

4 participants