feat: Introduce repository pattern to prepare adding other database providers #646

Noezor · 2023-07-13T22:07:38Z

Original goal of this MR was to make the repo able to store data in multiple datasources, not only supabase, and implement neon.

Given how big the MR already is, we reduced the scope to introducing a repository pattern behind which we can access one of the implemented databases.

Another MR will follow to add sqlalchemy for other sql-compatible systems - starting with neon.

For supabase, we decided to keep the API because this is the prefered way to access the datasource there.

vercel · 2023-07-13T22:07:41Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 1, 2023 5:51pm

vercel · 2023-07-13T22:07:43Z

Someone is attempting to deploy a commit to the Quivr-app Team on Vercel.

A member of the Team first needs to authorize it.

StanGirard · 2023-07-14T05:52:22Z

Cant wait to see the end :)

gozineb · 2023-07-19T09:25:55Z

@Noezor, great idea !! Feel free to add the SQL alchemy repository in backend/core/repository/SQLAlchemy. We're slowly trying to move all dependancy to supabase to core/repository/Supabase. The idea is to have repository functions that are agnostic from the database.

Noezor · 2023-07-19T10:06:55Z

Work has been moved to raoufchebri#1

The current solution is that we want to keep the supabase integration the way it is, but add Sqlalchemy for the others (including neon). We are keeping the supabase integration this way instead of using Sqlalchemy as supabase does not use connection pooling so using the API is the favored method.

Abstract Supabase from database query calls

Noezor

The return values are not typed. Today, Supabase does not always return objects that are mappable directly to existing defined objects. We may want to add some level of normalization in the repository to make sure the return types always match and avoid doing repository-custom logic out of them. WDYT @StanGirard @gozineb ?

Noezor · 2023-07-20T08:19:26Z

backend/models/brains.py

-            .execute()
-        )
-        return [item["brains"] for item in response.data]
+        return self.commons["db"].get_user_brains(user_id)


We follow the use of commons here, but I am not sure pushing the db object in a dict enables the VSCode python server to be helpful and know the type of the variable?
We should use the typing of the Repository in another MR.

@gozineb actually I noticed that for the other items on commons, my VSCode does not seem to be able to infer a type, even though we have CommonsDep = Annotated[dict, Depends(common_dependencies)] below. WDYT of having an explicit Dataclass CommonsDep instead of resolving the types ad-hoc?

Agree 44c6a2a

@Noezor really like it, super clean !

Keeping this for another PR, wdyt about the current state of this one?

There are conflicts that needs to be fixed before merging @Noezor ;)

gozineb · 2023-07-20T12:24:13Z

backend/routes/chat_routes.py

-        commons["supabase"].table("chats").delete().match(
-            {"chat_id": chat_id}
-        ).execute()
+        commons["db"].delete_chat


Method is not called here

StanGirard · 2023-07-25T20:26:10Z

@Noezor @raoufchebri any update or help needed ? Would love to see this feature implemented <3

Noezor · 2023-07-25T20:32:56Z

Hey! Sure, still top of mind for me but I'm pretty lazy with fixing the conflicts, not a lot of time right now
@raoufchebri ?

Noezor · 2023-07-29T17:13:30Z

I've been trying to resolve the commits but this is a bit of a headache...

For example, the new

        commons = common_dependencies()
        response = (
            commons["supabase"]
            .table("brains_users")
            .insert(
                {
                    "brain_id": str(self.id),
                    "user_id": str(user_id),
                    "rights": rights,
                    "default_brain": default_brain,
                }
            )
            .execute()
        )
        response = self.commons["db"].create_brain(self.name)
        self.id = response.data[0]["brain_id"]
        return response.data

has a side effect with side.id = blah, which means that we now need to refactor harder what we did and it is not entirely trivial to resolve. Help appreciated @StanGirard

Noezor · 2023-08-01T07:23:42Z

@StanGirard how should we proceed about getting this merged? I fixed the issues, but now there is again more conflicts

StanGirard · 2023-08-01T07:25:14Z

We will help you :)

StanGirard

What a PR ! @raoufchebri & @Noezor thanks a lot !!!!

Noezor · 2023-08-01T21:43:54Z

What a PR ! @raoufchebri & @Noezor thanks a lot !!!!

There's more to come though! 😊

…roviders (#646) * add sqlalchemy models * add neon settings * add insert brain * abstract supabase from Brain class * abstract supabase from Brain class * abstract supabase from /models * update Database to Repository * update neon_tables to pg_tables * update chat, api-key and message * update vector class * update settings * update env vars for test * Update backend-tests.yml * fix test * fix fetch_user_requests_count() * fix fetch_user_requests_count() * fix increment_user_request_count * fix increment_user_request_count * fix asset upload_response message * fix pyright * fix brain_subscription * fix brain_subscription * fix brain_subscription * fix get user request stat * update create_brain_user * add delete brain vector and user * add delete brain vector and user * correctly call function --------- Co-authored-by: Noé Pion <noe.pion@onfido.com> Co-authored-by: raoufchebri <raouf@chebri.com> Co-authored-by: Stan Girard <girard.stanislas@gmail.com>

add sqlalchemy models

34aad24

vercel bot deployed to Preview – docs July 13, 2023 22:07 View deployment

raoufchebri added 2 commits July 17, 2023 11:15

add neon settings

95c71f2

add insert brain

5366562

vercel bot deployed to Preview – docs July 17, 2023 20:33 View deployment

raoufchebri force-pushed the feat/add-neon branch from 5366562 to 34aad24 Compare July 18, 2023 09:46

raoufchebri added 5 commits July 18, 2023 14:29

abstract supabase from Brain class

66044bf

abstract supabase from Brain class

adf9142

abstract supabase from /models

816550c

update Database to Repository

d04490c

update neon_tables to pg_tables

7a744fb

Noezor had a problem deploying to preview July 18, 2023 21:50 — with GitHub Actions Failure

raoufchebri had a problem deploying to preview July 18, 2023 21:50 — with GitHub Actions Failure

raoufchebri added 11 commits July 19, 2023 13:08

update chat, api-key and message

4d1f46d

update vector class

325a829

update settings

3c68aed

update env vars for test

4238680

Update backend-tests.yml

eef5f0d

fix test

5efb0ff

fix fetch_user_requests_count()

d31bb40

fix fetch_user_requests_count()

c9d821f

fix increment_user_request_count

a6d5a2f

fix increment_user_request_count

4d26619

fix asset upload_response message

545898f

raoufchebri added 7 commits July 20, 2023 00:37

fix brain_subscription

04a0c5f

fix brain_subscription

eb42a9e

fix get user request stat

9de137e

update create_brain_user

96f0932

add delete brain vector and user

56719e1

add delete brain vector and user

d1fe756

Merge pull request #1 from raoufchebri/raouf-add-neon

f171c16

Abstract Supabase from database query calls

vercel bot deployed to Preview – docs July 20, 2023 08:06 View deployment

Noezor commented Jul 20, 2023

View reviewed changes

gozineb reviewed Jul 20, 2023

View reviewed changes

Noezor changed the title ~~Draft: feat: add neon~~ feat: Introduce repository pattern to prepare adding other database providers Jul 20, 2023

correctly call function

bda09c4

vercel bot deployed to Preview – docs July 20, 2023 19:35 View deployment

mattzcarey mentioned this pull request Jul 24, 2023

[Hosting] AAContributor I can deploy Quivr on multiple cloud providers (AWS, GCP, ?) #322

Closed

Merge remote-tracking branch 'upstream/main' into feat/add-neon

12b0d81

vercel bot deployed to Preview – docs July 30, 2023 22:06 View deployment

Merge branch 'main' into feat/add-neon

96c04ba

StanGirard had a problem deploying to preview August 1, 2023 17:51 — with GitHub Actions Failure

vercel bot deployed to Preview – docs August 1, 2023 17:51 View deployment

StanGirard approved these changes Aug 1, 2023

View reviewed changes

StanGirard merged commit 303ba72 into QuivrHQ:main Aug 1, 2023

StanGirard mentioned this pull request Aug 1, 2023

chore(main): release 0.0.50 #824

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Introduce repository pattern to prepare adding other database providers #646

feat: Introduce repository pattern to prepare adding other database providers #646

Noezor commented Jul 13, 2023 •

edited

Loading

vercel bot commented Jul 13, 2023 •

edited

Loading

vercel bot commented Jul 13, 2023

StanGirard commented Jul 14, 2023

gozineb commented Jul 19, 2023 •

edited

Loading

Noezor commented Jul 19, 2023

Noezor left a comment

Noezor Jul 20, 2023

Noezor Jul 20, 2023

mamadoudicko Jul 21, 2023

gozineb Jul 21, 2023

Noezor Jul 23, 2023

StanGirard Jul 23, 2023

gozineb Jul 20, 2023

StanGirard commented Jul 25, 2023

Noezor commented Jul 25, 2023

Noezor commented Jul 29, 2023

Noezor commented Aug 1, 2023

StanGirard commented Aug 1, 2023

StanGirard left a comment

Noezor commented Aug 1, 2023

feat: Introduce repository pattern to prepare adding other database providers #646

feat: Introduce repository pattern to prepare adding other database providers #646

Conversation

Noezor commented Jul 13, 2023 • edited Loading

vercel bot commented Jul 13, 2023 • edited Loading

vercel bot commented Jul 13, 2023

StanGirard commented Jul 14, 2023

gozineb commented Jul 19, 2023 • edited Loading

Noezor commented Jul 19, 2023

Noezor left a comment

Choose a reason for hiding this comment

Noezor Jul 20, 2023

Choose a reason for hiding this comment

Noezor Jul 20, 2023

Choose a reason for hiding this comment

mamadoudicko Jul 21, 2023

Choose a reason for hiding this comment

gozineb Jul 21, 2023

Choose a reason for hiding this comment

Noezor Jul 23, 2023

Choose a reason for hiding this comment

StanGirard Jul 23, 2023

Choose a reason for hiding this comment

gozineb Jul 20, 2023

Choose a reason for hiding this comment

StanGirard commented Jul 25, 2023

Noezor commented Jul 25, 2023

Noezor commented Jul 29, 2023

Noezor commented Aug 1, 2023

StanGirard commented Aug 1, 2023

StanGirard left a comment

Choose a reason for hiding this comment

Noezor commented Aug 1, 2023

Noezor commented Jul 13, 2023 •

edited

Loading

vercel bot commented Jul 13, 2023 •

edited

Loading

gozineb commented Jul 19, 2023 •

edited

Loading