Select a result to preview

throttled-py의 백엔드 연동 기여 (2) - merge 직전에 다듬은 것들 - 기본값 계약, 테스트, 문서 drift

#devlog #backend #open-source

들어가며

이전 글에서는 throttled-py의 FastAPI contrib을 처음 설계하고, 메인테이너 리뷰를 거쳐 "미들웨어가 헤더를 소유"하던 구조에서 "데코레이터가 route-local context와 header policy를 소유"하는 구조로 바꾼 과정을 정리했습니다.

그 시점에는 큰 설계 방향은 합의된 상태였습니다. 남은 것은 merge 전에 사용자 입장에서 볼 수 있는 표면을 다듬는 일이었습니다. 이어진 리뷰에서 생각보다 중요한 지적이 하나 더 나왔습니다.

바로 key_func 기본값이었습니다.

처음 구현은 아래처럼 자연스러워 보였습니다.

limiter = Limiter("100/m", key_func=get_remote_address)

그리고 더 짧은 기본 사용 형태는 이렇게 만들었습니다.

limiter = Limiter("100/m")

문제는 이 기본 형태가 내부적으로 get_remote_address를 쓴다는 점이었습니다. 즉, 사용자가 아무것도 선택하지 않아도 FastAPI contrib의 public default가 "client IP 기반 rate limiting"으로 고정되는 구조였습니다.

이번 글에서는 이 리뷰를 어떻게 처리했는지 정리합니다.

1. P1: 기본값이 client IP라는 계약

메인테이너가 지적한 핵심은 "IP 기반 제한이 나쁘다"가 아니었습니다. get_remote_address 자체는 유용한 helper입니다. 문제는 그것이 constructor default라는 점이었습니다.

초기 구조는 대략 이랬습니다.

class Limiter:
    def __init__(
        self,
        quota: Quota | str,
        *,
        store: AsyncStoreP | None = None,
        using: RateLimiterTypeT = RateLimiterType.TOKEN_BUCKET.value,
        key_func: KeyFunc = get_remote_address,
        hooks: Sequence[Hook] | None = None,
    ) -> None:
        ...
        self._key_func = key_func

get_remote_address는 ASGI scope의 request.client.host를 읽습니다. 직접 연결된 클라이언트라면 괜찮습니다. 하지만 실제 배포에서는 FastAPI 앱 앞에 reverse proxy, load balancer, ingress, gateway가 있는 경우가 많습니다.

이때 request.client.host는 원래 호출자가 아니라 proxy 주소일 수 있습니다. 그러면 여러 사용자의 요청이 하나의 proxy address로 합쳐져 같은 rate-limit bucket에 들어갑니다.

더 큰 문제는 public API 계약입니다. 한 번 Limiter("100/m")의 의미가 client IP 기반으로 출시되면, 나중에 기본값을 바꾸는 것은 동작 변경입니다. 사용자에게는 breaking change가 됩니다.

메인테이너가 원한 방향은 명확했습니다.

2. key_func=None의 의미

여기서 고민이 생겼습니다.

key_func=None을 정말 "principal이 없음"으로 볼 것인가, 아니면 내부적으로 package-owned default principal을 넣을 것인가?

완전히 "없음"으로 모델링하면 저장소 키 구조도 바뀝니다.

fastapi:{method}:{route}
fastapi:{method}:{route}:{principal}

이렇게 principal segment 자체가 optional이 됩니다. 설계적으로는 매우 정직합니다. None이 진짜 상태가 되기 때문입니다.

하지만 이번 PR에서 거기까지 바꾸는 것은 범위가 컸습니다. KeyParts.principal 타입이 str | None이 되어야 하고, compose_key()의 계약과 테스트도 같이 바뀝니다. 이미 FastAPI contrib의 큰 설계는 합의된 상태였고, 이번 리뷰의 핵심은 "기본값이 client IP인 계약을 끊는 것"이었습니다.

그래서 이번 PR에서는 좁게 처리했습니다.

_DEFAULT_PRINCIPAL = "__throttled_global_principal__"


def _default_key_func(request: Request) -> str:  # noqa: ARG001
    """Return the built-in shared principal for omitted ``key_func``."""
    return _DEFAULT_PRINCIPAL

그리고 생성자는 이렇게 바꿨습니다.

class Limiter:
    def __init__(
        self,
        quota: Quota | str,
        *,
        store: BaseStore | None = None,
        using: RateLimiterTypeT = RateLimiterType.TOKEN_BUCKET.value,
        key_func: KeyFunc | None = None,
        hooks: Sequence[Hook] | None = None,
    ) -> None:
        ...
        self._key_func: KeyFunc = key_func or _default_key_func

이 방식은 None을 완전히 no-principal 상태로 모델링하지는 않습니다. 내부적으로는 여전히 항상 KeyFunc가 존재합니다. 하지만 중요한 public contract는 끊었습니다.

Limiter("100/m")

이제 이 코드는 client IP를 기본값으로 사용하지 않습니다. 같은 method와 route에 대해 모든 caller가 하나의 shared bucket을 씁니다.

반대로 IP 기반 제한을 원하면 명시적으로 써야 합니다.

from throttled.asyncio.contrib.fastapi import Limiter, get_remote_address

limiter = Limiter("100/m", key_func=get_remote_address)

이건 단순한 기본값 변경이 아니라 "누가 identity 추출 책임을 갖는가"를 정리한 것입니다. 기본 contrib은 deployment-specific client identity를 추측하지 않습니다. 사용자가 IP 기반 제한을 선택할 때만 request.client.host를 principal로 씁니다.

3. 테스트는 구현 모양이 아니라 동작을 봐야 한다

이 과정에서 테스트도 정리했습니다.

초기 테스트 중에는 closure 내부를 파고들어 Throttled 인스턴스가 어떤 store를 들고 있는지 확인하는 테스트가 있었습니다. 의도는 나쁘지 않았습니다. 사용자 제공 MemoryStore나 RedisStore가 실제로 내부 limiter에 전달되는지를 확인하고 싶었습니다.

문제는 검증 방식이 구현 모양에 너무 의존했다는 점입니다.

captured_throttled = x.__closure__[...].cell_contents
assert captured_throttled._store is user_store

이런 테스트는 동작보다 closure 구조와 private attribute에 기대고 있습니다. 리팩터링으로 wrapper의 closure 배치가 바뀌면 기능이 멀쩡해도 테스트가 깨집니다.

이미 같은 목적은 tests/asyncio/contrib/fastapi/test_store_backends.py에서 더 좋은 방식으로 검증하고 있었습니다.

그래서 closure/private _store assertion은 제거했습니다. 대체 테스트를 더 추가하지 않았습니다. 이미 동작 수준의 coverage가 있었기 때문입니다.

이 부분은 리뷰 대응 문구도 신중하게 잡았습니다. "의도는 있었지만 표현 방식이 잘못됐다"는 쪽이 정확했습니다.

The intent was to verify the MemoryStore and RedisStore injection paths, but the closure/private _store assertion was not the right way to express that.

오픈소스에서 테스트는 단순히 coverage를 올리는 도구가 아니라, 유지보수자가 코드를 바꿀 자유를 얼마나 남겨두는지도 결정합니다. 구현 내부를 너무 세게 붙잡는 테스트는 기능을 보호하기보다 리팩터링을 방해할 수 있습니다.

4. 문서에서 API Reference를 걷어내기

문서 리뷰도 꽤 중요했습니다.

처음 FastAPI guide에는 hand-written API Reference 섹션이 있었습니다. Limiter, Limiter.limit(), RateLimitMiddleware, exception handler 등을 표로 직접 설명했습니다.

겉보기에는 친절해 보이지만, 메인테이너는 이 방식을 피하자고 했습니다. 프로젝트에는 이미 docs/source/api.rst가 있고, Sphinx autodoc으로 docstring에서 API Reference를 생성하는 구조가 있습니다.

guide 안에 API Reference를 손으로 한 번 더 쓰면 drift가 생깁니다.

그래서 FastAPI guide의 API Reference 섹션은 제거하고, 중앙 API 문서에만 남겼습니다.

FastAPI Integration
===================

.. autoclass:: throttled.asyncio.contrib.fastapi.Limiter
    :members:
    :special-members: __init__

.. autoclass:: throttled.asyncio.contrib.fastapi.RateLimitMiddleware
    :members:

.. autoexception:: throttled.asyncio.contrib.fastapi.RateLimitExceededError
    :members:

.. autofunction:: throttled.asyncio.contrib.fastapi.rate_limit_exceeded_handler

.. autofunction:: throttled.asyncio.contrib.fastapi.get_remote_address

FastAPI guide는 setup, examples, constraints에 집중하도록 바꿨습니다. API의 정확한 signature와 parameter 설명은 docstring이 source of truth가 됩니다.

5. 예제와 setup contract를 가볍게 만들기

초기 guide는 note/warning block이 많았습니다.

전부 틀린 말은 아니었습니다. 하지만 첫 독자에게는 연동이 실제보다 더 fragile해 보일 수 있었습니다.

메인테이너가 제안한 방향은 "큰 warning block 대신 runnable example 가까이에 짧은 comment를 두자"였습니다.

그래서 문서 상단을 Examples 섹션으로 정리했습니다.

.. tab-set::

    .. tab-item:: Shared route quota

        .. literalinclude:: ../../../examples/contrib/fastapi/basic_example.py
           :language: python

    .. tab-item:: API key quota

        .. literalinclude:: ../../../examples/contrib/fastapi/custom_key_func_example.py
           :language: python

    .. tab-item:: Client IP quota

        .. literalinclude:: ../../../examples/contrib/fastapi/remote_address_example.py
           :language: python

    .. tab-item:: Per-route quotas

        .. literalinclude:: ../../../examples/contrib/fastapi/multi_route_example.py
           :language: python

각 예제 파일의 주석도 같은 스타일로 맞췄습니다.

# 1) Create a limiter with the default shared route quota.
limiter = Limiter("2/m")

# 2) Wire FastAPI integration hooks:
#    middleware adds RateLimit-* headers
#    handler renders HTTP 429 responses.
app = FastAPI()
app.add_middleware(RateLimitMiddleware)
app.add_exception_handler(RateLimitExceededError, rate_limit_exceeded_handler)


# 3) Apply the limiter to a route.
@app.get("/items")
@limiter.limit()
async def list_items(request: Request) -> dict[str, list[str]]:
    return {"items": ["apple", "banana"]}

이 주석은 일부러 짧게 유지했습니다.

RateLimitMiddleware는 rate-limit check가 통과한 응답에 RateLimit-* header를 붙입니다. rate_limit_exceeded_handler는 quota exhaustion을 HTTP 429로 렌더링합니다. 둘의 역할은 다르지만, "세 개가 모두 rate limiting 자체에 필수"라는 식으로 쓰면 오해가 생깁니다.

그래서 문서의 setup 설명도 아래처럼 낮췄습니다.

The setup has three parts:

1. **Limiter**: Checks decorated routes against a quota.
2. **RateLimitMiddleware**: Adds ``RateLimit-*`` headers to checked responses.
3. **rate_limit_exceeded_handler**: Renders quota exhaustion as HTTP 429 with
   ``Retry-After``.

크게 경고하지 않고, 각각 무엇을 하는지만 말합니다.

6. "successful responses"라는 표현도 정리하기

Copilot 리뷰가 하나 더 남긴 지적이 있었습니다. RateLimitMiddleware docstring에는 "successful responses"에 header를 주입한다고 적혀 있었지만, 실제 구현은 status code를 보지 않습니다.

미들웨어는 route가 실행된 뒤 request.state에 decorator-produced context가 있으면 response에 header를 붙입니다.

response: Response = await call_next(request)
context: RateLimitContext | None = getattr(request.state, _STATE_KEY, None)
if context is not None:
    _inject_rate_limit_headers(response.headers, context)
return response

즉, rate-limit check를 통과한 route가 내부에서 400이나 500을 반환하더라도 RateLimit-* header는 붙습니다. 이 동작은 의도적입니다. header는 response outcome이 아니라 quota state를 설명하기 때문입니다.

단, quota exhaustion 429는 별도입니다. decorator가 route 실행 전에 RateLimitExceededError를 발생시키고, 등록된 exception handler가 429 response와 Retry-After를 렌더링합니다.

그래서 docstring을 "successful responses"가 아니라 "responses from rate-limit-checked routes"로 고쳤습니다.

"""ASGI middleware for ``RateLimit-*`` headers on checked routes."""

이런 wording은 작아 보이지만 API contract에서는 중요합니다. 구현과 문서가 서로 다른 말을 하면 나중에 어느 쪽이 맞는지 불필요한 논쟁이 생깁니다.

7. main rebase와 uv.lock 충돌

리뷰 대응을 마친 뒤 main이 앞서 나갔습니다. PR 브랜치는 main보다 여러 커밋 뒤처졌고, uv.lock 충돌도 있었습니다.

여기서는 merge commit을 만들지 않고 rebase를 선택했습니다.

git rebase origin/main

충돌은 예상대로 uv.lock에서 났습니다. lockfile은 손으로 조립하기보다 main 쪽을 우선 받고, pyproject.toml의 FastAPI/httpx dependency 변경이 살아 있는지 확인한 뒤 uv lock으로 재생성했습니다.

git checkout --ours uv.lock
git add uv.lock pyproject.toml
git rebase --continue

UV_CACHE_DIR=/tmp/uv-cache uv lock

여기서 한 가지 주의할 점은 rebase 중 --ours와 --theirs의 의미입니다. rebase 중에는 --ours가 rebase 대상인 main 쪽입니다. 그래서 "main의 lockfile을 그대로 받는다"면 git checkout --ours uv.lock이 맞습니다.

rebase 후에는 main의 store type 리팩터링도 반영해야 했습니다. 예전에는 FastAPI limiter가 AsyncStoreP를 참조했는데, main에서는 BaseStore 중심으로 정리되어 있었습니다. Sphinx autodoc도 AsyncStoreP forward reference를 못 풀고 warning을 냈습니다.

그래서 FastAPI contrib 쪽 타입을 main에 맞췄습니다.

if TYPE_CHECKING:
    from throttled.asyncio.store import BaseStore
    from throttled.types import RateLimiterTypeT


class Limiter:
    def __init__(
        self,
        quota: Quota | str,
        *,
        store: BaseStore | None = None,
        ...
    ) -> None:
        ...
        self._store: BaseStore = store or MemoryStore()

테스트 쪽에 남아 있던 BaseStore[Any]도 제거했습니다. main 기준으로 BaseStore는 더 이상 generic이 아니기 때문입니다.

마지막으로 검증을 다시 돌렸습니다.

UV_CACHE_DIR=/tmp/uv-cache uv lock --check
UV_CACHE_DIR=/tmp/uv-cache uv run ruff check throttled/asyncio/contrib/fastapi tests/asyncio/contrib/fastapi examples/contrib/fastapi
UV_CACHE_DIR=/tmp/uv-cache uv run pytest tests/asyncio/contrib/fastapi -q
UV_CACHE_DIR=/tmp/uv-cache uv run sphinx-build -b html docs/source docs/build/html

결과는 다음과 같았습니다.

rebase 후 force push를 했기 때문에 리뷰 답글에 적어둔 commit SHA도 모두 바뀌었습니다. 이건 메인테이너에게 별도 코멘트로 알리고, inline reply의 SHA도 새 값으로 수정했습니다.

8. 후속 작업으로 남긴 것

이번 PR에서는 proxy-aware client identity extraction을 구현하지 않았습니다.

왜냐하면 이것은 get_remote_address 기본값 제거와는 별개의 설계 문제이기 때문입니다. proxy, ingress, load balancer 환경에서 어떤 header를 신뢰할 것인지, 어떤 proxy range를 trusted로 볼 것인지, 잘못된 X-Forwarded-For chain은 어떻게 다룰 것인지 같은 문제가 생깁니다.

이건 단순 helper 하나를 추가하는 일이 아니라 trust boundary를 설계하는 일입니다.

그래서 PR 답글에는 다음과 같은 방향으로 선을 그었습니다.

This removes the public default contract that tied the contrib to client-IP identity.
I would treat richer proxy / ingress identity extraction as follow-up design work.

기본 contrib은 특정 배포 환경의 client identity를 추측하지 않습니다. 사용자가 직접 key_func를 제공하거나, 후속 설계에서 명시적인 deployment-aware helper를 추가할 수 있습니다.

이 선긋기가 중요했습니다. 모든 것을 이번 PR에 넣으려 하면 scope가 계속 커집니다. 반대로 후속 작업으로 미루되, 왜 미루는지와 어떤 문제를 다룰지를 분명히 해두면 리뷰어도 납득하기 쉽습니다.

마치며

이번 리뷰에서 다시 떠올린 것은 "기본값은 기능보다 더 강한 계약일 수 있다"는 점이었습니다.

get_remote_address는 좋은 helper입니다. 하지만 helper로 두는 것과 constructor default로 두는 것은 결이 다른 선택이라고 느꼈습니다. 특히 client IP처럼 배포 환경에 따라 의미가 크게 달라지는 값은 더 그렇습니다.

또 하나는 문서의 유지보수 표면적이었습니다. 자세한 문서를 쓰는 것 자체는 어렵지 않지만, 같은 내용을 guide, API Reference, docstring에 반복하면 시간이 지나면서 drift가 쌓이기 쉽습니다. 이번에는 guide를 setup/examples/constraints에 집중시키고, API Reference는 Sphinx autodoc으로 중앙화하는 쪽을 택했습니다.

리뷰 답글도 코드만큼 관리가 필요하다는 것도 다시 느낀 부분이었습니다. rebase로 SHA가 바뀌면 이미 남긴 답글의 commit reference도 낡습니다. 오픈소스 PR에서는 이런 작은 정리가 리뷰어의 부담을 줄여주는 듯합니다.

FastAPI contrib은 설계, 테스트, 문서, 예제까지 어느 정도 리뷰 가능한 상태로 정리됐습니다. 다음 글에서는 이번 PR에서 굳어진 원칙들, 이를테면 "데코레이터가 route-local context를 소유한다", "header policy는 contrib이 결정한다", "default는 곧 공개 계약이다" 같은 것들을 framework에 종속되지 않는 contrib 패턴으로 추출해 보려고 합니다. Flask나 DRF 같은 후속 연동의 토대가 될 부분입니다.

감사합니다.

참고자료