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

Synchronous Session support. #13

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fotinakis
Copy link

This PR implements support for SQLAlchemy synchronous Session, alongside existing support for AsyncSession.

Based on @frankie567's answer in this discussion on "Synchronous SQLAlchemy?": fastapi-users/fastapi-users#1144 (comment)

  • Copied SQLAlchemyUserDatabase --> SQLAlchemySynchronousUserDatabase and then:
    • Accept a sqlalchemy Session arg instead of an AsyncSession
    • Remove the await keyword when working with this session (e.g. await self.session.commit() becomes self.session.commit())
    • The queries didn't have to change
  • Copied SQLAlchemyAccessTokenDatabase --> SQLAlchemySynchronousAccessTokenDatabase and applied the same changes

Tested:

  • Added a SYNC_DATABASE_URL that uses a non-async sqlite adapter
  • Used pytest parameterized fixtures to run the same tests with the sync & async engines
  • Also tested similar setup in a real-world app

Followup:

  • If this is accepted, I'd be happy to update documentation to reference these new object types

@fotinakis
Copy link
Author

@frankie567 friendly ping on this — I believe this is feature complete & fully tested in the existing style.

@fotinakis
Copy link
Author

@frankie567 friendly ping here, don't want this to get stale

@BredoGen
Copy link

It'll be great if sync SQLAlchemy adapter will be supported out-of-box.

Are there any plans to merge this?

@jerivas
Copy link

jerivas commented Nov 2, 2023

For anyone who is using sync sessions and doesn't want to maintain mirror classes, here's a hack to make all methods on Session awaitable based on AsyncSession:

import inspect
from typing import Annotated, Any, Generic

from fastapi_users.authentication.strategy.db import AP
from fastapi_users.models import ID, UP
from fastapi_users_db_sqlalchemy import (
    SQLAlchemyBaseOAuthAccountTable,
    SQLAlchemyUserDatabase,
)
from fastapi_users_db_sqlalchemy.access_token import SQLAlchemyAccessTokenDatabase
from sqlalchemy.ext.asyncio import AsyncSession
from sqlalchemy.orm import Session


class FakeAsyncSession:
    def __init__(self, session: Session):
        self.session = session

    def __getattr__(self, name: str) -> Any:
        """
        If the method being called is async in AsyncSession, create a fake async version
        for Session so callers can `await` as usual. Think `commit`, `refresh`,
        `delete`, etc.
        """
        async_session_attr = getattr(AsyncSession, name, None)
        session_attr = getattr(self.session, name)
        if not inspect.iscoroutinefunction(async_session_attr):
            return session_attr

        async def async_wrapper(*args, **kwargs):
            return session_attr(*args, **kwargs)

        return async_wrapper


class UserDatabase(Generic[UP, ID], SQLAlchemyUserDatabase[UP, ID]):
    session: Session

    def __init__(
        self,
        session: AsyncSession,
        user_table: type[UP],
        oauth_account_table: type[SQLAlchemyBaseOAuthAccountTable] | None = None,
    ):
        super().__init__(session, user_table, oauth_account_table)
        self.session = FakeAsyncSession(session)


class AccessTokenDatabase(Generic[AP], SQLAlchemyAccessTokenDatabase[AP]):
   session: Session

    def __init__(self, session: AsyncSession, access_token_table: type[AP]):
        super().__init__(session, access_token_table)
        self.session = FakeAsyncSession(session)

Then both UserDatabase and AccessTokenDatabase can be used in place of the classes provided by this library.

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.

3 participants