Repository navigation
적대적 코드 리뷰: src/kma 정확성·보안·성능 수정 + CI 추가 - #25
Merged
Merged
Conversation
… performance fixes
4 independent reviewer subagents (correctness/concurrency, security, API design,
performance/reliability) audited the hand-written src/kma files (apihub_endpoints.py,
a 13,589-line auto-generated wrapper file, was excluded from scope); every finding
was adversarially re-verified by a separate skeptic agent (reading the actual code +
reproducing the failure) before being applied. 34 raw findings, 4 refuted.
Highlights:
- pagination.py: iter_pages() trusted the response body's self-reported pageNo to
compute the next page instead of the page it actually requested, so an operation
that doesn't echo pageNo correctly (or always returns a fixed value) made iter_pages
stall on one page and silently re-fetch/duplicate it up to max_pages times, returned
to the caller as if they were distinct subsequent pages -- now increments a
locally-tracked page number instead
- pagination.py: pageNo/numOfRows/totalCount that arrive as decimal-formatted numbers
("10.0") failed int() parsing and silently fell back to a sentinel default, which
could flip has_next_page() to False and truncate a real result set with zero
indication -- now tolerant of decimal-formatted numeric fields; added an explicit
PaginationLimitWarning when max_pages is hit while more pages were genuinely
available (previously fully silent); added an async aiter_pages() counterpart, and
the same fixes applied to ApiHubClient's iter_pages/aiter_pages
- client.py: a JSON envelope with header: null (or any non-mapping header) crashed
parsing with a raw, unhandled AttributeError instead of the library's typed
KmaParseError
- apihub.py: ApiHubResponse.text decoded via httpx's default charset guess instead of
the response's actual declared Content-Type charset; open_api()/aopen_api() returned
error responses as success without checking the embedded result code; base_url was
accepted without validation (added an apihub.kma.go.kr host allowlist)
Also adds .github/workflows/ci.yml (lint/typecheck/test on Python 3.10-3.13).
Verified against real servers: `KMA_RUN_LIVE=1 pytest -m integration` with the
data.go.kr and APIHub keys present in this checkout's .env/.env.local -- 9 passed,
3 skipped (service key not subscribed to those specific data.go.kr operations, not a
regression).
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01XF9V2q4mAmhmXn6t5G9Hfe
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
개요
src/kma의 손으로 작성한 로직 파일(src/kma/apihub_endpoints.py, tools/update_apihub_endpoints.py가 생성하는 13,589줄짜리 자동생성 470개 endpoint wrapper 파일은 범위에서 제외)을 4명의 독립된 전문 리뷰어 서브에이전트(정확성/동시성, 보안, API 설계, 성능/신뢰성)가 적대적으로 리뷰했습니다. 각 발견은 별도 검증 에이전트가 실제 코드를 다시 읽고 반박을 시도한 뒤 확정했습니다. (방법론: digitie/python-mois-api#9, digitie/python-krforest-api#10)가장 중요한 수정 — pagination 데이터 손실/중복
pagination.py:iter_pages()가 응답 body가 자체 보고하는pageNo를 그대로 신뢰해 다음 페이지를 계산했습니다. 어떤 operation이pageNo를 제대로 echo하지 않거나(또는 항상 고정값을 돌려주면),iter_pages()는 같은 페이지에서 멈춘 채max_pages번까지 같은 페이지를 반복 재요청하면서 호출자에게는 서로 다른 후속 페이지인 것처럼 반환했습니다. → 서버가 실제로 요청받은 페이지 번호를 로컬에서 직접 추적하도록 수정.pagination.py:pageNo/numOfRows/totalCount가"10.0"처럼 소수점 형태 숫자로 오면int()파싱이 실패해 조용히 기본값(sentinel)으로 대체됐고, 이 값이has_next_page()를False로 뒤집어 더 가져올 페이지가 있는데도 아무 표시 없이 순회가 중단됐습니다. → 소수점 형태 숫자 파싱을 허용.max_pages에 도달했는데 실제로 더 가져올 페이지가 남아있으면 이제PaginationLimitWarning을 발생시킵니다(이전엔 완전히 무음). 비동기aiter_pages()신규 추가,ApiHubClient의iter_pages/aiter_pages에도 동일 수정 적용.그 외 주요 수정
client.py:header: null(또는 Mapping이 아닌 header)인 JSON envelope가 typedKmaParseError대신 처리되지 않은AttributeError로 크래시하던 문제apihub.py:ApiHubResponse.text가 실제 응답Content-Type의 charset이 아니라 httpx의 기본 추정 charset으로 디코딩되던 문제apihub.py:open_api()/aopen_api()가 결과 코드를 확인하지 않고 오류 응답을 성공으로 그대로 반환하던 문제apihub.py:base_url을 검증 없이 받아들이던 문제 →apihub.kma.go.kr호스트 allowlist 추가검증
python -m pytest -q -m "not integration": 149 passed, 12 deselectedKMA_RUN_LIVE=1 python -m pytest -m integration(이 체크아웃의.env/.env.local에 있는 data.go.kr·APIHub 키 사용) → 9 passed, 3 skipped(서비스키가 구독하지 않은 특정 data.go.kr operation, 회귀 아님)python -m ruff check .: 통과python -m mypy src/kma: 통과 (strict)추가: CI 워크플로
.github/workflows/ci.yml신규 추가 —lint/typecheck/test(Python 3.10/3.11/3.12/3.13,not integration마커 제외) 3개 job.🤖 Generated with Claude Code
Co-Authored-By: Claude Sonnet 5 [email protected]