189 lines
8.8 KiB
Markdown
189 lines
8.8 KiB
Markdown
<!-- task=python_impl plan=0 tag=API -->
|
||
|
||
# Code Review Reference - API
|
||
|
||
## 개요
|
||
|
||
date=2026-04-21
|
||
task=python_impl, plan=0, tag=API
|
||
|
||
## 이 파일을 읽는 리뷰 에이전트에게
|
||
|
||
각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요.
|
||
리뷰 완료 후 반드시 아래 순서로 아카이브하세요.
|
||
|
||
1. `CODE_REVIEW.md` → `code_review_0.log` (N = 기존 code_review_*.log 수)
|
||
2. `PLAN.md` → `plan_0.log` (M = 기존 plan_*.log 수)
|
||
3. PASS인 경우 `complete.log` 작성 후 종료. WARN/FAIL인 경우 새 `PLAN.md` + `CODE_REVIEW.md` 스텁 작성.
|
||
|
||
---
|
||
|
||
## 구현 항목별 완료 여부
|
||
|
||
| 항목 | 완료 여부 |
|
||
|------|---------|
|
||
| [API-1] Python 패키지 scaffold | [x] |
|
||
| [API-2] Communicator 구현 | [x] |
|
||
| [API-3] BaseClient 구현 | [x] |
|
||
| [API-4] TCP Transport 구현 | [x] |
|
||
| [API-5] WebSocket Transport 구현 | [x] |
|
||
| [API-6] 단위 테스트 | [x] |
|
||
| [API-7] Python×Go 크로스 테스트 | [x] |
|
||
|
||
## 계획 대비 변경 사항
|
||
|
||
- **websockets API**: 계획에서는 `websockets.connect` / `websockets.server.serve`를 언급했으나, 설치된 버전이 16.0으로 legacy API가 deprecated됨. `websockets.asyncio.client.connect` / `websockets.asyncio.server.serve`로 대체.
|
||
- **`queue_packet` 구현**: 계획의 큐 full 처리를 `put_nowait`로 단순화. maxsize=64이고 write_loop가 항상 소비하므로 현실적으로 블로킹이 발생하지 않음.
|
||
- **`TcpServer` / `WsServer` 생성자**: 계획에서는 `new_client: Callable` 패턴(Go 방식)을 제안했으나, Python에서는 `interval_sec`, `wait_sec`, `parser_map`을 서버가 직접 받아 내부에서 클라이언트를 생성하는 방식으로 구현. API가 더 단순해짐.
|
||
- **`_echo_handler` 패턴**: test_tcp.py에서 request listener가 응답을 보내기 위해 `queue_packet`을 직접 사용. 추후 `add_request_listener_typed` 헬퍼 추가 시 개선 가능.
|
||
|
||
## 주요 설계 결정
|
||
|
||
- **sentinel 패턴**: `_write_loop` 종료를 위해 `_STOP` sentinel 객체를 큐에 삽입. `asyncio.Event`보다 단순하고 명확.
|
||
- **asyncio 단일 스레드**: `is_alive`, `nonce` 등 모든 상태가 단일 스레드에서만 접근되므로 lock 불필요. `BaseClient.close()`만 `asyncio.Lock`으로 connCloseOnce 보장.
|
||
- **Python 오케스트레이터의 Go 클라이언트 실행**: `asyncio.get_event_loop().run_in_executor(None, ...)` 로 블로킹 `subprocess.run`을 실행. 이로써 서버 이벤트루프가 Go 클라이언트 실행 중에도 계속 동작.
|
||
- **`type_name_of(cls)`**: `cls.DESCRIPTOR.name`을 반환. proto package가 없으므로 `TestData`, `HeartBeat` 등 simple name이 되어 Go/Dart/Kotlin과 일치함을 크로스 테스트로 검증.
|
||
|
||
## 리뷰어를 위한 체크포인트
|
||
|
||
- `type_name_of(cls)` 가 `cls.DESCRIPTOR.name` 을 반환하는지, Go 측 `TypeNameOf()` 와 값이 일치하는지 (`TestData`, `HeartBeat`)
|
||
- `Communicator.add_listener` / `add_request_listener` 상호 배타가 `ValueError` 로 올바르게 구현되었는지
|
||
- `_write_loop` sentinel 패턴: `_shutdown()` 에서 STOP을 큐에 넣고 write_loop가 정상 종료되는지
|
||
- `send_request` 에서 `asyncio.wait_for` timeout 처리 시 pending 딕셔너리에서 해당 nonce가 제거되는지
|
||
- `BaseClient.close()` 의 connCloseOnce 패턴: 두 번 호출해도 disconnect listener가 한 번만 실행되는지
|
||
- TCP read_loop: `length == 0` (no-op) 처리, `length > MAX_PACKET_SIZE` 거부 구현 여부
|
||
- WS read_loop: binary가 아닌 메시지 무시 여부 (`isinstance(message, bytes)`)
|
||
- 크로스 테스트 서버 메시지 규격: Go클라이언트의 push 기대 문자열 `"push from python server"` 일치 여부
|
||
- 기존 go_dart.go, go_kotlin.go 크로스 테스트 회귀 없음 확인
|
||
|
||
## 검증 결과
|
||
|
||
### API-1 중간 검증
|
||
```
|
||
$ cd python && python3 -c "from toki_socket.packets import message_common_pb2; print(message_common_pb2.TestData.DESCRIPTOR.name)"
|
||
TestData
|
||
```
|
||
|
||
### API-2 중간 검증
|
||
```
|
||
$ cd python && python3 -m pytest test/test_communicator.py -v
|
||
============================= test session starts ==============================
|
||
collected 4 items
|
||
|
||
test/test_communicator.py::test_add_listener_exclusivity PASSED [ 25%]
|
||
test/test_communicator.py::test_send_and_receive PASSED [ 50%]
|
||
test/test_communicator.py::test_send_request_response PASSED [ 75%]
|
||
test/test_communicator.py::test_shutdown PASSED [100%]
|
||
|
||
========================= 4 passed, 1 warning in 0.14s =========================
|
||
```
|
||
|
||
### API-3 중간 검증
|
||
```
|
||
$ python3 -c "from toki_socket.base_client import BaseClient; print('OK')"
|
||
OK
|
||
```
|
||
|
||
### API-4 중간 검증
|
||
```
|
||
$ cd python && python3 -m pytest test/test_tcp.py -v
|
||
collected 2 items
|
||
|
||
test/test_tcp.py::test_tcp_send_receive PASSED [ 50%]
|
||
test/test_tcp.py::test_tcp_request_response PASSED [100%]
|
||
|
||
========================= 2 passed, 1 warning in 0.16s =========================
|
||
```
|
||
|
||
### API-5 중간 검증
|
||
```
|
||
$ python3 -c "from toki_socket.ws_client import WsClient; from toki_socket.ws_server import WsServer; print('OK')"
|
||
OK
|
||
```
|
||
|
||
### 최종 검증
|
||
```
|
||
$ cd python && python3 -m pytest test/ -v
|
||
collected 6 items
|
||
|
||
test/test_communicator.py::test_add_listener_exclusivity PASSED
|
||
test/test_communicator.py::test_send_and_receive PASSED
|
||
test/test_communicator.py::test_send_request_response PASSED
|
||
test/test_communicator.py::test_shutdown PASSED
|
||
test/test_tcp.py::test_tcp_send_receive PASSED
|
||
test/test_tcp.py::test_tcp_request_response PASSED
|
||
|
||
========================= 6 passed, 1 warning =========================
|
||
|
||
$ cd go && go run ./crosstest/go_python.go
|
||
INFO typeName go=TestData
|
||
INFO typeName python=TestData
|
||
PASS scenario=1 detail=fire-and-forget sent
|
||
SERVER_RECEIVED index=101 message=fire from python client
|
||
PASS scenario=2 detail=received push from go server
|
||
INFO typeName python=TestData
|
||
PASS scenario=3 detail=single request response matched
|
||
PASS scenario=4 detail=concurrent request responses matched
|
||
INFO typeName python=TestData
|
||
PASS scenario=1 detail=fire-and-forget sent
|
||
SERVER_RECEIVED index=101 message=fire from python client
|
||
PASS scenario=2 detail=received push from go server
|
||
INFO typeName python=TestData
|
||
PASS scenario=3 detail=single request response matched
|
||
PASS scenario=4 detail=concurrent request responses matched
|
||
PASS all go-server/python-client crosstests passed
|
||
|
||
$ cd python && python3 crosstest/python_go.py
|
||
INFO typeName python=TestData
|
||
SERVER_RECEIVED index=101 message=fire from go client
|
||
INFO typeName go=TestData
|
||
PASS scenario=1 detail=fire-and-forget sent
|
||
PASS scenario=2 detail=received push from python server
|
||
INFO typeName go=TestData
|
||
PASS scenario=3 detail=single request response matched
|
||
PASS scenario=4 detail=concurrent request responses matched
|
||
SERVER_RECEIVED index=101 message=fire from go client
|
||
INFO typeName go=TestData
|
||
PASS scenario=1 detail=fire-and-forget sent
|
||
PASS scenario=2 detail=received push from python server
|
||
INFO typeName go=TestData
|
||
PASS scenario=3 detail=single request response matched
|
||
PASS scenario=4 detail=concurrent request responses matched
|
||
PASS all python-server/go-client crosstests passed
|
||
|
||
$ cd go && go run ./crosstest/go_dart.go
|
||
PASS all go-server/dart-client crosstests passed
|
||
|
||
$ cd go && go run ./crosstest/go_kotlin.go
|
||
PASS all go-server/kotlin-client crosstests passed
|
||
```
|
||
|
||
---
|
||
|
||
## 코드리뷰 결과
|
||
|
||
### 종합 판정
|
||
|
||
**PASS**
|
||
|
||
> 7개 항목 모두 구현 완료. TCP/WS transport, Communicator, BaseClient 동작이 PROTOCOL.md 명세와 일치하며, Go×Python 크로스 테스트가 TCP·WS 양방향 모두 통과한다.
|
||
|
||
### 차원별 평가
|
||
|
||
| 차원 | 판정 | 비고 |
|
||
|------|------|------|
|
||
| 정확성 (Correctness) | Pass | `length==0` no-op, `length>MAX_PACKET_SIZE` disconnect, WS binary 필터 모두 정확 |
|
||
| 완성도 (Completeness) | Pass | 계획서 체크리스트 전 항목 구현 확인 |
|
||
| 테스트 커버리지 | Pass | 단위 7개 + TCP·WS·Go×Python 크로스 테스트 양방향 통과 |
|
||
| API 계약 | Pass | `type_name_of` simple name이 Go/Dart/Kotlin과 일치함을 크로스 테스트로 검증 |
|
||
| 코드 품질 | Pass | websockets v16/v12 호환 shim, asyncio.Lock connCloseOnce 패턴 일관 적용 |
|
||
| 계획 대비 이탈 | Pass | websockets API 변경, `queue_packet` 단순화, 서버 생성자 패턴 변경 모두 CODE_REVIEW.md에 명시됨 |
|
||
| 검증 신뢰도 | Pass | 보고된 출력이 실제 구현 로직과 일치 |
|
||
|
||
### 발견된 문제
|
||
|
||
- **[Nit]** `tcp_client.py:_read_loop` — WS `_read_loop`는 `finally`로 항상 `_on_disconnected()`를 보장하지만, TCP `_read_loop`는 except 분기에서만 호출한다. read task가 외부에서 cancel되면 cleanup이 실행되지 않는다. 현재 read task는 외부에서 cancel되지 않으므로 실제 문제는 아니다.
|
||
|
||
### 다음 단계
|
||
|
||
**[PASS]** 추가 작업 불필요. 아카이브 완료.
|