-
Notifications
You must be signed in to change notification settings - Fork 31
Asynced the Database #659 and #649 #660
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,53 +18,56 @@ class AbstractWrapper(ABC, Generic[T, PK]): | |
|
|
||
| model: type[T] | ||
|
|
||
| def get_all(self) -> list[T]: | ||
| async def get_all(self) -> list[T]: | ||
| """ | ||
| Get all data wrapper for the unspecified model | ||
|
|
||
| :return: a list of all model instances | ||
| """ | ||
| with get_db_session() as session: | ||
| return list(session.exec(select(self.model)).all()) | ||
| async with get_db_session() as session: | ||
| result = await session.execute(select(self.model)) | ||
| return list(result.scalars().all()) | ||
|
|
||
| def get_by_id(self, obj_id: PK) -> T: | ||
| async def get_by_id(self, obj_id: PK) -> T: | ||
| """ | ||
| Retrieve data wrapper for the unspecified model | ||
|
|
||
| :param obj_id: PK of the model instance to be retrieved | ||
| :return: the retrieved instance | ||
| """ | ||
| with get_db_session() as session: | ||
| obj = session.get(self.model, obj_id) | ||
| async with get_db_session() as session: | ||
| obj = await session.get(self.model, obj_id) | ||
| if not obj: | ||
| raise ValueError(f"{self.model.__name__} with ID {obj_id} not found.") | ||
| return obj | ||
|
|
||
| def create(self, data: dict[str, Any]) -> T: | ||
| async def create(self, data: dict[str, Any]) -> T: | ||
| """ | ||
| Post data wrapper for the unspecified model | ||
|
|
||
| :param data: the JSON object of the model instance to be created | ||
| :return: the newly created instance | ||
| """ | ||
| with get_db_session() as session: | ||
| async with get_db_session() as session: | ||
| obj = self.model(**data) | ||
| session.add(obj) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i mean since we're going nuts and awaiting everything, does this need await as well?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep, though looking at it, we could just add session: AsyncSession = Depends(get_db_session) into the function args of the abstract wrapper |
||
| session.commit() | ||
| session.refresh(obj) | ||
| await session.commit() | ||
| await session.refresh(obj) | ||
| return obj | ||
|
|
||
| def delete_by_id(self, obj_id: PK) -> T: | ||
| async def delete_by_id(self, obj_id: PK) -> T: | ||
| """ | ||
| Delete data wrapper for the unspecified model | ||
|
|
||
| :param obj_id: PK of the model instance to be deleted | ||
| :return: the deleted instance | ||
| """ | ||
| with get_db_session() as session: | ||
| obj = session.get(self.model, obj_id) | ||
| async with get_db_session() as session: | ||
| obj = await session.get(self.model, obj_id) | ||
| if not obj: | ||
| raise ValueError(f"{self.model.__name__} with ID {obj_id} not found.") | ||
| session.delete(obj) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. revert this change to return obj, not make a obj copy |
||
| session.commit() | ||
| return obj | ||
| # Preserve object state before deleting | ||
| obj_copy = self.model(**{c.name: getattr(obj, c.name) for c in obj.__table__.columns}) # type: ignore[attr-defined] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not use session.delete(obj)?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah cursor bugged out here thinking that db deletion deletes the object from the memory, though it doesnt. pls return obj, not obj_copy |
||
| session.delete(obj) # type: ignore[unused-coroutine] | ||
| await session.commit() | ||
| return obj_copy | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,16 +8,17 @@ | |
| from gs.backend.data.tables.aro_user_tables import AROUserAuthToken | ||
|
|
||
|
|
||
| def get_all_auth_tokens() -> list[AROUserAuthToken]: | ||
| async def get_all_auth_tokens() -> list[AROUserAuthToken]: | ||
| """ | ||
| Get all the auth tokens | ||
| """ | ||
| with get_db_session() as session: | ||
| auth_tokens = list(session.exec(select(AROUserAuthToken)).all()) | ||
| async with get_db_session() as session: | ||
| result = await session.execute(select(AROUserAuthToken)) | ||
| auth_tokens = list(result.scalars().all()) | ||
| return auth_tokens | ||
|
|
||
|
|
||
| def add_auth_token(token: str, user_data_id: UUID, expiry: datetime, auth_type: AROEnums) -> AROUserAuthToken: | ||
| async def add_auth_token(token: str, user_data_id: UUID, expiry: datetime, auth_type: AROEnums) -> AROUserAuthToken: | ||
| """ | ||
| Add auth token to the db | ||
|
|
||
|
|
@@ -26,29 +27,29 @@ def add_auth_token(token: str, user_data_id: UUID, expiry: datetime, auth_type: | |
| :param expiry: the date in which this token expires | ||
| :param auth_type: the type of auth token this is, can only be from AROAuthToken | ||
| """ | ||
| with get_db_session() as session: | ||
| async with get_db_session() as session: | ||
| auth_token = AROUserAuthToken(token=token, user_data_id=user_data_id, expiry=expiry, auth_type=auth_type) | ||
|
|
||
| session.add(auth_token) | ||
| session.commit() | ||
| session.refresh(auth_token) | ||
| await session.commit() | ||
| await session.refresh(auth_token) | ||
| return auth_token | ||
|
|
||
|
|
||
| def delete_auth_token_by_id(token_id: UUID) -> list[AROUserAuthToken]: | ||
| async def delete_auth_token_by_id(token_id: UUID) -> list[AROUserAuthToken]: | ||
| """ | ||
| Delete the auth token based on the token id | ||
|
|
||
| :param token_id: the unique identifier for a particular auth token | ||
| """ | ||
|
|
||
| with get_db_session() as session: | ||
| auth_token = session.get(AROUserAuthToken, token_id) | ||
| async with get_db_session() as session: | ||
| auth_token = await session.get(AROUserAuthToken, token_id) | ||
|
|
||
| if auth_token: | ||
| session.delete(auth_token) | ||
| session.commit() | ||
| session.delete(auth_token) # type: ignore[unused-coroutine] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yea, this seems more logical. also the await seems inconsistent. sometimes session uses await yet other times it does not.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in terms of db connection, awaits should only be used on IO or expensive processes, so usually commit and refresh session methods |
||
| await session.commit() | ||
| else: | ||
| print("Token does not exist") | ||
|
|
||
| return get_all_auth_tokens() | ||
| return await get_all_auth_tokens() | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this allow for the db to be asynchronous? i thought it already was asynchronous
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the new one is the async driver for psql connection