teamhide / fastapi-boilerplate

FastAPI boilerplate for real world production
1.12k stars 178 forks source link

안녕하세요 TeamHide님, 질문이 있습니다. #27

Closed rumbarum closed 9 months ago

rumbarum commented 9 months ago

기존에 작성 하셨던 stand_alone_session 에 DI 를 달아서 FastAPI BackgroundTask 용으로 사용하고 있습니다.

# Di
    writer_engine = providers.Singleton(
        create_async_engine,
        config.WRITER_DB_URL,
        pool_recycle=3600,
        connect_args={"server_settings": {"jit": "off"}},
    )
    reader_engine = providers.Singleton(
        create_async_engine,
        config.READER_DB_URL,
        pool_recycle=3600,
        connect_args={"server_settings": {"jit": "off"}},
    )
    async_session_factory = providers.Singleton(
        sessionmaker,
        class_=AsyncSession,
        autocommit=False,
        autoflush=True,
        expire_on_commit=False,
        sync_session_class=RoutingSession,
    )
    session = providers.Singleton(
        async_scoped_session,
        session_factory=async_session_factory,
        scopefunc=get_session_context,
    )
from uuid import uuid4
from dependency_injector.wiring import Provide
from sqlalchemy.ext.asyncio import async_scoped_session
from .session_maker import reset_session_context, set_session_context

session: async_scoped_session = Provide["session"]

def standalone_session(func):
    async def _standalone_session(*args, **kwargs):
        session_id = str(uuid4())
        context = set_session_context(session_id=session_id)

        try:
            result = await func(*args, **kwargs)
            await session.commit()
        except Exception as e:
            await session.rollback()
            raise e
        finally:
            await session.remove()
            reset_session_context(context=context)
        return result

    return _standalone_session
@standalone_session
@inject
async def delete_all_user_data(user_id: UUID, user_oauth_id: UUID, session: async_scoped_session = Provide["session"]):
    """
    사용자의 모든 데이터를 삭제합니다.
    """
    await session.execute("DELETE FROM item_orders WHERE user_id = :user_id;", {"user_id": user_id})
    ....

기 작성 되었던 Transactional 과 비슷하다고 생각해서 기존에 try 블록에 await session.commit() 가 빠졌구나 하고 추가해서 쓰고 있었습니다.

오늘 코드 짜다가 func 내 session 에서 commit 하지 않고, standalone_session 데코레이터에 있는 commit으로 커밋 되겠거니 하고 넘겼습니다. 커밋이 안되었고, 예전에 왜 되었지 하고 쓴것들보니 다 func내에서 .commit()을 하고 있었네요. func내에 있는 session과 standalone_session에 있는 session의 .id() 확인 해보니 안되는게 맞는것 같기도 합니다.

session 객체를 동일하게 사용하려고, provide.Singleton을 사용한건데 제가 무엇을 잘못 이해하고 있는지 답변을 부탁드려도 될까요?

rumbarum commented 9 months ago

standalone_session 에 들어가는 session을 provider로 찍어내면 기대대로 동작합니다.

standalone_session 의 session과, func 내의 session이 동일합니다.

from .session_maker import reset_session_context, set_session_context

session_factory: Provider[async_scoped_session] = Provide["session.provider"]

def standalone_session(func):
    async def _standalone_session(*args, **kwargs):
        session = session_factory()
        session_id = str(uuid4())
        context = set_session_context(session_id=session_id)
        try:
            result = await func(*args, **kwargs)
            await session.commit()
        except Exception as e:
            await session.rollback()
            raise e
        finally:
            await session.remove()
            reset_session_context(context=context)
        return result

    return _standalone_session
teamhide commented 9 months ago

@rumbarum 바름님 안녕하세요. 첫 번째 코드를 보면 engine, session_factory등 모든 부분을 Singleton으로 구성하셨는데 따로 이유가 있을까요? 파이썬에서는 모듈 레벨 임포트 시 기본적으로 싱글톤이 보장됩니다. 따라서 DI 라이브러리를 통해 싱글톤으로 선언하지 않아도 된다고 생각해요. 참고 문서

그리고 추가적으로 몇가지 말씀드리자면,

현재 코드의 구성을 보면 session의 context를 미들웨어에서 설정합니다. 하지만 테스트 코드에서 통합 테스트가 아닌 경우 이러한 context를 설정해주는 부분이 없기 때문에 context에 대한 문제가 발생했어요. 그래서 standalone_session 데코레이터를 통해 context를 설정해주는 코드를 작성했었는데요. 이 부분 현재 개선해서 fixture로 빼두었습니다. 코드 여기서 session_context fixture 참고하시면 좋을 것 같습니다.

이러한 context 설정을 통해 비동기 서버에서 SQLAlchemy를 context local로 사용할 수 있는데, 알고 계실지도 모르지만 자세한 내용은 관련 문서 여기서 scoped_session 단락 확인하시면 됩니다.

결론적으로 테스트 코드이던지, 실제 서버 코드이던지 session에 대한 처리를 따로 해주실 필요는 없습니다. 어떠한 곳에서라도 단순히 session을 import해서 사용하는게 목적이었고 그렇게 돌아가도록 작성한 코드라서요. 참고로 현재 전체 코드에 대한 변경사항이 대폭 적용되어 전체 코드를 다시 한번 보시는것도 좋을 것 같네요.

도움이 되면 좋겠네요 :)

rumbarum commented 9 months ago

아무래도 DI의 문제로 보이네요. 현상 한번 파보고 이슈 제보해야 겠네요.

첫 번째 코드를 보면 engine, session_factory등 모든 부분을 Singleton으로 구성하셨는데 따로 이유가 있을까요?

저는 전역 객체들도 import 가 아닌, DI Container로 주입하려고 이렇게 했습니다. 모듈에서 임포트를 DI inject로 대입했다고 보시면 됩니다. 이렇게 했던건 사실 영식님 코드가 시작이었네요 . 코드링크 이 코드가 잘 이해가 안가서 보다가 객체 정의 해논 곳과 객체 초기화 하는 곳이 달라서 이렇게 되었다고 보고 정의와 주입을 한번에 몰아서 하려고 DI 로 통합했습니다.

그래서 아래처럼 컨테이너에 정의해놓고 사용하고 있습니다.

  redis_backend = providers.Factory(RedisBackend)
  redis_key_maker = providers.Factory(CustomKeyMaker)
  redis = providers.Singleton(
      Redis.from_url,
      # this value should be called when declared. If not, repr(config.value) will be injected.
      url=f"redis://{config.REDIS_HOST()}:{config.REDIS_PORT()}",
  )
  cache_manager = providers.Singleton(
      CacheManager, backend=redis_backend, key_maker=redis_key_maker
  )

stand_alone_session이 원래는 fixture 용도 였군요, 메인 로직과 상관없는 DB 접근을 BackgroudTasks에서 하려고 했는데 기존 미들웨어 context가 먼저 remove 되더라구요. 그래서 거기에서 사용하고 있습니다. 생각해보니 테스트에도 사용하긴 했네요.

암튼 영식님 코드가 FastAPI 이해에 아주 많은 도움이 되었고 지금도 계속 참고하고 있습니다! 지금 repo에도 DI 적용하시는것 같은데, 제 적용기도 공유드립니다. 도움 되시면 좋겠네요. 문서링크