Skip to content

feat: get_chance() 결과 per-market TTL 캐시 추가 #46

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.

Sign up for GitHub

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

Merged
interruping merged 3 commits into from
Mar 22, 2026
Merged

feat: get_chance() 결과 per-market TTL 캐시 추가 #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
747e337
feat: get_chance() 결과 per-market TTL 캐시 추가
Mar 22, 2026
783388f
test: min_total 캐시 동작 검증 테스트 추가
Mar 22, 2026
51d49b3
refactor: 리뷰 반영 — 만료 캐시 엔트리 정리 및 TTL 테스트 개선
Mar 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions src/upbeat/api/orders.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import time
from decimal import Decimal
from typing import Any

Expand Down Expand Up @@ -35,6 +36,9 @@ def _compute_bid_total(
return Decimal(price)


_DEFAULT_MIN_TOTAL_TTL: float = 60.0


class OrdersAPI(_SyncAPIResource):
_validate_min_order: bool

Expand All @@ -44,9 +48,21 @@ def __init__(
credentials: Credentials | None,
*,
validate_min_order: bool = False,
min_total_ttl: float = _DEFAULT_MIN_TOTAL_TTL,
) -> None:
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]] = {}
Copy link
Copy Markdown
Owner Author

@interruping interruping Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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

Copy link
Copy Markdown
Owner Author

@interruping interruping Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Owner Author

@interruping interruping Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Reviewed by Claude (Opus 4.6) — AI Code Reviewer


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

def _check_min_order(
self,
Expand All @@ -61,8 +77,25 @@ def _check_min_order(
total = _compute_bid_total(price, volume, ord_type)
if total is None:
return

cached = self._get_cached_min_total(market)
if cached is not None:
min_total = Decimal(cached)
if total < min_total:
raise ValidationError(
f"Order total {total} is below minimum {min_total} for {market}",
market=market,
total=str(total),
min_total=cached,
)
return

chance = self.get_chance(market=market)
Copy link
Copy Markdown
Owner Author

@interruping interruping Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Copy link
Copy Markdown
Owner Author

@interruping interruping Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Owner Author

@interruping interruping Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Reviewed by Claude (Opus 4.6) — AI Code Reviewer

if chance.market.bid is not None:
self._min_total_cache[market] = (
chance.market.bid.min_total,
time.monotonic() + self._min_total_ttl,
)
min_total = Decimal(chance.market.bid.min_total)
if total < min_total:
raise ValidationError(
Expand Down Expand Up @@ -305,9 +338,21 @@ def __init__(
credentials: Credentials | None,
*,
validate_min_order: bool = False,
min_total_ttl: float = _DEFAULT_MIN_TOTAL_TTL,
) -> None:
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]] = {}
Copy link
Copy Markdown
Owner Author

@interruping interruping Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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:
        ...

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

Copy link
Copy Markdown
Owner Author

@interruping interruping Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Owner Author

@interruping interruping Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Reviewed by Claude (Opus 4.6) — AI Code Reviewer


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

async def _check_min_order(
self,
Expand All @@ -322,8 +367,25 @@ async def _check_min_order(
total = _compute_bid_total(price, volume, ord_type)
if total is None:
return

cached = self._get_cached_min_total(market)
if cached is not None:
min_total = Decimal(cached)
if total < min_total:
raise ValidationError(
f"Order total {total} is below minimum {min_total} for {market}",
market=market,
total=str(total),
min_total=cached,
)
return

chance = await self.get_chance(market=market)
if chance.market.bid is not None:
self._min_total_cache[market] = (
chance.market.bid.min_total,
time.monotonic() + self._min_total_ttl,
)
min_total = Decimal(chance.market.bid.min_total)
if total < min_total:
raise ValidationError(
Expand Down
95 changes: 95 additions & 0 deletions tests/api/test_orders.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,3 +700,98 @@ async def test_async_validation_passes(self) -> None:
market="KRW-BTC", side="bid", ord_type="price", price="6000"
)
assert isinstance(result, OrderCreated)


# ── TestMinOrderCache ──────────────────────────────────────────────────


class TestMinOrderCache:
def test_cache_hit_skips_get_chance(self) -> None:
chance_calls = 0

def handler(request: httpx.Request) -> httpx.Response:
nonlocal chance_calls
if request.url.path == "/v1/orders/chance":
chance_calls += 1
return _multi_handler(request)

transport = _make_transport(handler)
api = OrdersAPI(transport, CREDENTIALS, validate_min_order=True)
api.create(market="KRW-BTC", side="bid", ord_type="price", price="6000")
api.create(market="KRW-BTC", side="bid", ord_type="price", price="6000")
assert chance_calls == 1

def test_cache_expires_after_ttl(self) -> None:
chance_calls = 0

def handler(request: httpx.Request) -> httpx.Response:
nonlocal chance_calls
if request.url.path == "/v1/orders/chance":
chance_calls += 1
return _multi_handler(request)

transport = _make_transport(handler)
api = OrdersAPI(
transport, CREDENTIALS, validate_min_order=True, min_total_ttl=-1.0
)
api.create(market="KRW-BTC", side="bid", ord_type="price", price="6000")
api.create(market="KRW-BTC", side="bid", ord_type="price", price="6000")
assert chance_calls == 2

def test_cache_is_per_market(self) -> None:
chance_calls = 0

def handler(request: httpx.Request) -> httpx.Response:
nonlocal chance_calls
if request.url.path == "/v1/orders/chance":
chance_calls += 1
return _multi_handler(request)

transport = _make_transport(handler)
api = OrdersAPI(transport, CREDENTIALS, validate_min_order=True)
api.create(market="KRW-BTC", side="bid", ord_type="price", price="6000")
api.create(market="KRW-ETH", side="bid", ord_type="price", price="6000")
assert chance_calls == 2

def test_cache_hit_still_validates(self) -> None:
transport = _make_transport(_multi_handler)
api = OrdersAPI(transport, CREDENTIALS, validate_min_order=True)
# First call populates the cache
api.create(market="KRW-BTC", side="bid", ord_type="price", price="6000")
# Second call should use cache but still raise for low total
with pytest.raises(ValidationError) as exc_info:
api.create(market="KRW-BTC", side="bid", ord_type="price", price="3000")
assert exc_info.value.min_total == "5000"

def test_cache_populated_on_validation_failure(self) -> None:
chance_calls = 0

def handler(request: httpx.Request) -> httpx.Response:
nonlocal chance_calls
if request.url.path == "/v1/orders/chance":
chance_calls += 1
return _multi_handler(request)

transport = _make_transport(handler)
api = OrdersAPI(transport, CREDENTIALS, validate_min_order=True)
with pytest.raises(ValidationError):
api.create(market="KRW-BTC", side="bid", ord_type="price", price="3000")
with pytest.raises(ValidationError):
api.create(market="KRW-BTC", side="bid", ord_type="price", price="3000")
assert chance_calls == 1

@pytest.mark.asyncio
async def test_async_cache_hit_skips_get_chance(self) -> None:
chance_calls = 0

async def handler(request: httpx.Request) -> httpx.Response:
nonlocal chance_calls
if request.url.path == "/v1/orders/chance":
chance_calls += 1
return _multi_handler(request)

transport = _make_async_transport(handler)
api = AsyncOrdersAPI(transport, CREDENTIALS, validate_min_order=True)
await api.create(market="KRW-BTC", side="bid", ord_type="price", price="6000")
await api.create(market="KRW-BTC", side="bid", ord_type="price", price="6000")
assert chance_calls == 1
Loading