feat: get_chance() 결과 per-market TTL 캐시 추가 by interruping · Pull Request #46 · interruping/upbeat

interruping · 2026-03-22T13:44:44Z

Summary

OrdersAPI / AsyncOrdersAPI에 마켓별 min_total TTL 캐시 도입 (기본 60초)
validate_min_order=True일 때 동일 마켓 반복 주문 시 get_chance() API 호출을 캐시로 대체하여 레이트 리밋 소진 방지
캐시 히트, TTL 만료, per-market 격리, 검증 실패 시에도 캐시 저장 등 6개 테스트 추가

Closes #45

Test plan

uv run pytest tests/api/test_orders.py -v — 기존 38개 테스트 + 신규 6개 모두 통과
uv run ruff check src/upbeat/api/orders.py — 린트 통과

🤖 Generated with Claude Code

DCA 등 동일 마켓 반복 주문 시 매번 get_chance() API를 호출하여 레이트 리밋이 불필요하게 소진되는 문제를 해결한다. min_total 값을 마켓별로 캐시하고 TTL(기본 60초) 경과 시 만료한다. Closes #45 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

캐시 히트, TTL 만료, per-market 격리, 캐시 히트 시 검증, 실패 시에도 캐시 저장, async 캐시 히트 테스트를 추가한다. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

interruping

PR #46 리뷰: `get_chance()` per-market TTL 캐시

전반적으로 잘 구현된 PR입니다. DCA 등 반복 주문 시 get_chance() 호출을 캐시로 줄여 레이트 리밋 소진을 방지하는 목적이 명확하고, 테스트 커버리지도 충실합니다.

잘된 점

time.monotonic() 사용 — 시스템 시계 변경에 영향받지 않아 TTL 계산에 적합
검증 실패 시에도 캐시를 저장하는 설계 — 동일 마켓 재시도 시 불필요한 API 호출 방지
per-market 격리 테스트, TTL 만료 테스트, 캐시 히트 시에도 검증이 동작하는 테스트 등 핵심 시나리오를 빠짐없이 검증

개선 제안

라인별 코멘트를 참고해주세요.

Reviewed by Claude (Opus 4.6) — AI Code Reviewer

interruping · 2026-03-22T13:46:44Z

src/upbeat/api/orders.py

        super().__init__(transport, credentials)
        self._validate_min_order = validate_min_order
+        self._min_total_ttl = min_total_ttl
+        self._min_total_cache: dict[str, tuple[str, float]] = {}


[suggestion] 만료된 캐시 엔트리가 dict에서 제거되지 않습니다. _get_cached_min_total에서 만료 감지 시 del self._min_total_cache[market]로 정리하면 장기 실행 프로세스에서 불필요한 메모리 점유를 방지할 수 있습니다.

물론 실제 마켓 수가 유한하므로 실질적인 문제는 아니지만, 방어적으로 정리하는 게 깔끔합니다.

Suggested change

self._min_total_cache: dict[str, tuple[str, float]] = {}

self._min_total_cache: dict[str, tuple[str, float]] = {}

def _get_cached_min_total(self, market: str) -> str | None:

entry = self._min_total_cache.get(market)

if entry is not None:

value, expiry = entry

if time.monotonic() < expiry:

return value

del self._min_total_cache[market]

return None

반영했습니다. 만료 감지 시 del self._min_total_cache[market]로 엔트리를 정리하도록 수정했습니다. (51d49b3)

확인했습니다. 동기/비동기 양쪽 모두 del self._min_total_cache[market]가 추가된 것을 확인했습니다. (51d49b3)

Reviewed by Claude (Opus 4.6) — AI Code Reviewer

interruping · 2026-03-22T13:46:44Z

src/upbeat/api/orders.py

        super().__init__(transport, credentials)
        self._validate_min_order = validate_min_order
+        self._min_total_ttl = min_total_ttl
+        self._min_total_cache: dict[str, tuple[str, float]] = {}


[duplication] _min_total_cache, _get_cached_min_total, 그리고 _check_min_order 내부의 캐시 조회/저장 로직이 OrdersAPI와 AsyncOrdersAPI에 거의 동일하게 중복됩니다.

캐시 로직 자체는 순수 동기 연산이므로 (time.monotonic() + dict 조회), mixin이나 공통 베이스에 추출하면 유지보수가 편해집니다.

class _MinTotalCacheMixin: _min_total_ttl: float _min_total_cache: dict[str, tuple[str, float]] def _init_cache(self, ttl: float) -> None: self._min_total_ttl = ttl self._min_total_cache = {} def _get_cached_min_total(self, market: str) -> str | None: ...

단, 이 정도 중복이면 현재 수준도 충분히 수용 가능합니다. 향후 캐시 로직이 복잡해질 때 리팩토링해도 늦지 않습니다.

동의합니다. 현재 수준에서는 중복이 크지 않으므로 그대로 유지하고, 향후 캐시 로직이 복잡해지면 mixin으로 리팩토링하겠습니다.

동의합니다. 현 수준의 중복은 수용 가능하고, 캐시 정책이 복잡해질 때 리팩토링하는 판단이 합리적입니다.

Reviewed by Claude (Opus 4.6) — AI Code Reviewer

interruping · 2026-03-22T13:46:44Z

src/upbeat/api/orders.py

+                )
+            return
+
        chance = self.get_chance(market=market)


[note] min_total_ttl이 OrdersAPI/AsyncOrdersAPI 생성자에만 노출되고, Upbeat/AsyncUpbeat 클라이언트에서는 전달되지 않습니다 (_client.py:89-92, _client.py:235-238).

사용자가 최상위 클라이언트를 통해 TTL을 설정할 수 없으므로, 의도적이라면 괜찮지만 그렇지 않다면 _client.py에도 min_total_ttl 파라미터를 추가하는 것이 좋겠습니다.

의도적으로 내부 디테일로 둔 것입니다. TTL은 대부분의 사용자에게 기본값(60초)으로 충분하고, 고급 사용자는 OrdersAPI를 직접 생성하여 설정할 수 있습니다. 클라이언트 레벨 노출이 필요해지면 후속 PR에서 추가하겠습니다.

의도된 설계라면 충분히 납득됩니다. 기본값 60초면 대부분의 시나리오에서 적절하고, 고급 사용자용 escape hatch가 이미 있으니 현재 수준으로 충분합니다.

Reviewed by Claude (Opus 4.6) — AI Code Reviewer

interruping · 2026-03-22T13:46:44Z

tests/api/test_orders.py

+
+        transport = _make_transport(handler)
+        api = OrdersAPI(
+            transport, CREDENTIALS, validate_min_order=True, min_total_ttl=0.0


[nit] min_total_ttl=0.0은 "삽입 즉시 만료"를 의미하므로 TTL 만료 테스트에 적합합니다. 다만, time.monotonic() 호출 간 시간 차가 0일 가능성이 이론적으로 존재합니다 (같은 tick). 실제로는 중간에 HTTP mock 처리가 있어 문제없지만, 더 명시적으로 하려면 min_total_ttl=-1.0 (과거 시점 만료)을 사용하는 방법도 있습니다.

현재도 충분히 안정적이라 변경 필수는 아닙니다.

좋은 지적입니다. min_total_ttl=-1.0으로 변경하여 "과거 시점 만료"를 명시적으로 표현하도록 수정했습니다. (51d49b3)

확인했습니다. min_total_ttl=-1.0으로 변경되어 "과거 시점 만료"의 의도가 명확해졌습니다. (51d49b3)

Reviewed by Claude (Opus 4.6) — AI Code Reviewer

- _get_cached_min_total에서 만료 감지 시 del로 엔트리 정리 - TTL 만료 테스트에서 min_total_ttl=-1.0 사용하여 의도 명확화 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

gylim and others added 2 commits March 22, 2026 22:44

test: min_total 캐시 동작 검증 테스트 추가

783388f

캐시 히트, TTL 만료, per-market 격리, 캐시 히트 시 검증, 실패 시에도 캐시 저장, async 캐시 히트 테스트를 추가한다. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

interruping commented Mar 22, 2026

View reviewed changes

interruping merged commit 5a8df6c into main Mar 22, 2026
3 checks passed

-        self._min_total_cache: dict[str, tuple[str, float]] = {}
+        self._min_total_cache: dict[str, tuple[str, float]] = {}
+    def _get_cached_min_total(self, market: str) -> str | None:
+        entry = self._min_total_cache.get(market)
+        if entry is not None:
+            value, expiry = entry
+            if time.monotonic() < expiry:
+                return value
+            del self._min_total_cache[market]
+        return None

Conversation

interruping commented Mar 22, 2026

Summary

Test plan

Uh oh!

interruping left a comment

Choose a reason for hiding this comment

PR #46 리뷰: get_chance() per-market TTL 캐시

잘된 점

개선 제안

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

PR #46 리뷰: `get_chance()` per-market TTL 캐시