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

[Feature] Add support to use any sql database as the metadata storage for embedchain apps #1273

Merged
merged 4 commits into from
Feb 19, 2024

Conversation

deshraj
Copy link
Collaborator

@deshraj deshraj commented Feb 19, 2024

Description

Changes:

  • Use sqlalchemy instead of sqlite3 to support multiple databases so that Embedchain can be used easily in production setting
  • Refactor code
  • Remove stale code

Some more work to be done related to this PR. Will update the docs in the follow up PR.

Type of change

  • New feature (non-breaking change which adds functionality)
  • Refactor (does not change functionality, e.g. code style improvements, linting)

How Has This Been Tested?

  • Unit Test

@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Feb 19, 2024
@deshraj deshraj changed the title [Feature] Add support for any sql database as the metadata storage for embedchain apps [Feature] Add support to use any sql database as the metadata storage for embedchain apps Feb 19, 2024
Copy link

codecov bot commented Feb 19, 2024

Codecov Report

Attention: 51 lines in your changes are missing coverage. Please review.

Comparison is base (6c12bc9) 56.58% compared to head (865f77e) 56.78%.

Files Patch % Lines
embedchain/core/db/database.py 71.69% 15 Missing ⚠️
embedchain/app.py 27.27% 8 Missing ⚠️
embedchain/embedchain.py 50.00% 7 Missing ⚠️
embedchain/memory/base.py 75.86% 7 Missing ⚠️
...versions/40a327b3debd_create_initial_migrations.py 69.56% 7 Missing ⚠️
embedchain/migrations/env.py 79.16% 5 Missing ⚠️
embedchain/client.py 83.33% 1 Missing ⚠️
embedchain/store/assistants.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1273      +/-   ##
==========================================
+ Coverage   56.58%   56.78%   +0.19%     
==========================================
  Files         146      150       +4     
  Lines        5954     6067     +113     
==========================================
+ Hits         3369     3445      +76     
- Misses       2585     2622      +37     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@deshraj deshraj merged commit 5e2e7fb into main Feb 19, 2024
5 checks passed
@deshraj deshraj deleted the user/dyadav/add-sqlalchemy branch February 19, 2024 21:04

def upgrade() -> None:
# ### commands auto generated by Alembic - please adjust! ###
op.create_table(
Copy link

@karlismea karlismea Feb 28, 2024

Choose a reason for hiding this comment

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

Getting an exception when starting my bot:
` File "/mnt/c/Users/karlis/Documents/chatApp/.venv/lib/python3.10/site-packages/embedchain/migrations/versions/40a327b3debd_create_initial_migrations.py", line 23, in upgrade
op.create_table(
File "", line 8, in create_table
File "", line 3, in create_table
File "/mnt/c/Users/karlis/Documents/chatApp/.venv/lib/python3.10/site-packages/alembic/operations/ops.py", line 1311, in create_table
return operations.invoke(op)
File "/mnt/c/Users/karlis/Documents/chatApp/.venv/lib/python3.10/site-packages/alembic/operations/base.py", line 445, in invoke
return fn(self, operation)
File "/mnt/c/Users/karlis/Documents/chatApp/.venv/lib/python3.10/site-packages/alembic/operations/toimpl.py", line 131, in create_table
operations.impl.create_table(table)
File "/mnt/c/Users/karlis/Documents/chatApp/.venv/lib/python3.10/site-packages/alembic/ddl/impl.py", line 366, in create_table
self._exec(schema.CreateTable(table))
File "/mnt/c/Users/karlis/Documents/chatApp/.venv/lib/python3.10/site-packages/alembic/ddl/impl.py", line 207, in _exec
return conn.execute(construct, multiparams)
File "/mnt/c/Users/karlis/Documents/chatApp/.venv/lib/python3.10/site-packages/sqlalchemy/engine/base.py", line 1408, in execute
return meth(
File "/mnt/c/Users/karlis/Documents/chatApp/.venv/lib/python3.10/site-packages/sqlalchemy/sql/ddl.py", line 180, in _execute_on_connection
return connection._execute_ddl(
File "/mnt/c/Users/karlis/Documents/chatApp/.venv/lib/python3.10/site-packages/sqlalchemy/engine/base.py", line 1519, in _execute_ddl
ret = self._execute_context(
File "/mnt/c/Users/karlis/Documents/chatApp/.venv/lib/python3.10/site-packages/sqlalchemy/engine/base.py", line 1839, in _execute_context
return self._exec_single_context(
File "/mnt/c/Users/karlis/Documents/chatApp/.venv/lib/python3.10/site-packages/sqlalchemy/engine/base.py", line 1979, in _exec_single_context
self._handle_dbapi_exception(
File "/mnt/c/Users/karlis/Documents/chatApp/.venv/lib/python3.10/site-packages/sqlalchemy/engine/base.py", line 2335, in _handle_dbapi_exception
raise sqlalchemy_exception.with_traceback(exc_info[2]) from e
File "/mnt/c/Users/karlis/Documents/chatApp/.venv/lib/python3.10/site-packages/sqlalchemy/engine/base.py", line 1960, in _exec_single_context
self.dialect.do_execute(
File "/mnt/c/Users/karlis/Documents/chatApp/.venv/lib/python3.10/site-packages/sqlalchemy/engine/default.py", line 924, in do_execute
cursor.execute(statement, parameters)
sqlalchemy.exc.OperationalError: (sqlite3.OperationalError) table ec_chat_history already exists
[SQL:
CREATE TABLE ec_chat_history (
app_id VARCHAR NOT NULL,
id VARCHAR NOT NULL,
session_id VARCHAR NOT NULL,
question TEXT,
answer TEXT,
metadata TEXT,
created_at TIMESTAMP,
PRIMARY KEY (app_id, id, session_id)
)

]`

Copy link

Choose a reason for hiding this comment

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

Same, this release caused a regression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:XL This PR changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants