Feat/49/painting-process-data-simulator-service: 전착도장 데이터 전송 시뮬레이터 기능 구현#53
Feat/49/painting-process-data-simulator-service: 전착도장 데이터 전송 시뮬레이터 기능 구현#53TaeHyunaivle merged 14 commits intomainfrom
Conversation
기존 시뮬레이터에서 더 이상 사용되지 않는 파일 및 테스트 코드를 정리
기존에 사용하던 print문을 적절한 로깅으로 대체, 기존 시뮬레이터의 settings.py를 변경
기존 시뮬레이터에서 모델 및 데이터 파일을 주고받는 로직을 제거하고 Spring Boot 서버에 데이터를 전송하도록 기능을 변경
수정에 따른 README 및 requirements 변경. .env.example 을 사용함으로써 수월하게 .env 사용 가능(Azure 액세스 키만 직접 추가하면 됨)
API 라우터, 스케줄러, Azure Storage 서비스, 백엔드 클라이언트의 핵심 로직에 대한 단위 테스트를 추가. 테스트를 위한 라이브러리 추가
파이썬 3.10버전에 맞는 numpy버전을 2.3.2에서 2.2.6으로 수정하고 .env파일에 따옴표를 빼서 오류를 방지
WalkthroughReorients the simulator to fetch CSV rows from Azure Blob Storage on a seconds-based schedule and POST them to a backend; adds centralized logging and an async backend HTTP client; updates settings, main app endpoints, scheduler, and Azure service; removes model-client, anomaly logger, diagnostic routes, and many legacy tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant API as FastAPI (/simulator)
participant Scheduler
participant Azure as AzureStorageService
participant Backend as BackendClient
participant Spring as Spring Boot Backend
User->>API: POST /simulator/start
API->>Scheduler: start(interval=seconds)
Scheduler-->>API: started
loop Every interval_seconds
Scheduler->>Azure: simulate_real_time_data()
Azure-->>Scheduler: simulated_data or None
alt Data available
Scheduler->>Backend: send_to_backend(data, backend_service_url)
Backend->>Spring: HTTP POST JSON
Spring-->>Backend: 2xx / error
Backend-->>Scheduler: success / failure
else No data
Scheduler-->>Scheduler: skip
end
end
User->>API: POST /simulator/stop
API->>Scheduler: stop()
Scheduler-->>API: stopped
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Out-of-scope changes
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
🤖 Smart FAST 자동 코드 리뷰📦 변경된 서비스 (1개):
🎯 PR 제목에서 서비스 자동 감지됨
📊 전체 검사 결과:🧪 테스트 & 코드 품질 결과:
🔒 보안 스캔 결과:
✅ 검사 체크리스트:
🚀 배포 정보:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🔭 Outside diff range comments (2)
services/painting-process-data-simulator-service/app/routers/simulator_router.py (1)
20-21: Don’t return raw exception messages to clientsReturning str(e) can leak internals. Log the exception server-side and return a generic message.
- except Exception as e: - raise HTTPException(status_code=500, detail=f"시뮬레이터 시작 실패: {str(e)}") from e + except Exception as e: + logger.exception("시뮬레이터 시작 실패") + raise HTTPException(status_code=500, detail="시뮬레이터 시작 실패") from eAdd this at the top of the module (outside the selected lines):
import logging logger = logging.getLogger(__name__)services/painting-process-data-simulator-service/app/main.py (1)
57-62: Expose the actual backend target instead of the legacy model name.Replace the legacy "target_model" with the configured backend URL for clarity and correctness.
Apply this diff:
- "target_model": "painting-process-equipment-defect-detection", + "target_backend": settings.backend_service_url,
🧹 Nitpick comments (15)
services/painting-process-data-simulator-service/requirements.txt (1)
34-35: Consider moving test-only dependencies to a dev requirements filepytest and pytest-asyncio inflate the runtime image and attack surface. If tests aren’t executed inside the production container, move them to requirements-dev.txt and keep this file runtime-only.
Apply this diff to slim the runtime requirements (only if tests are not run in the production image):
-pytest==8.4.1 -pytest-asyncio==1.1.0Additionally, add a dev requirements file at services/painting-process-data-simulator-service/requirements-dev.txt:
pytest==8.4.1 pytest-asyncio==1.1.0And update your CI to install both:
- pip install -r services/painting-process-data-simulator-service/requirements.txt
- pip install -r services/painting-process-data-simulator-service/requirements-dev.txt
services/painting-process-data-simulator-service/.env.example (2)
9-9: Azure Storage 연결 문자열이 비어있습니다Azure Storage 연결 문자열은 시뮬레이터의 핵심 기능인 CSV 데이터 읽기를 위해 필수입니다. 예제 파일이지만 사용자가 설정해야 할 필수 값임을 명확히 하는 것이 좋습니다.
-AZURE_CONNECTION_STRING= +AZURE_CONNECTION_STRING=<your-azure-storage-connection-string-here>
35-36: 백엔드 서비스 URL 설정 명확화 필요두 개의 백엔드 URL이 제공되어 혼란을 줄 수 있습니다. 주석을 더 명확히 수정하여 각각의 사용 시나리오를 구분하는 것이 좋습니다.
-# 도커 사용시 시뮬레이터가 데이터를 전송할 Spring Boot 백엔드 서비스의 전체 URL -BACKEND_SERVICE_URL=http://paintingprocessmonitoring:8087/equipment-data -# BACKEND_SERVICE_URL=http://localhost:8087/equipment-data # uvicorn으로 실행할때 사용 +# Spring Boot 백엔드 서비스의 전체 URL +# Docker Compose 환경에서 실행 시: +BACKEND_SERVICE_URL=http://paintingprocessmonitoring:8087/equipment-data +# 로컬 개발 환경(uvicorn)에서 실행 시: +# BACKEND_SERVICE_URL=http://localhost:8087/equipment-dataservices/painting-process-data-simulator-service/app/services/azure_storage.py (1)
79-82: 필수 컬럼 검증 로직 개선 필요DataFrame에서
iloc로 선택한 row는 Series 객체이므로index속성 확인이 올바른 방법입니다. 하지만 더 명확한 에러 메시지를 위해 실제 존재하는 컬럼도 함께 출력하는 것이 좋습니다.# 필요한 모든 열이 존재하는지 확인 required_columns = ['machineId', 'timeStamp', 'Thick', 'PT_jo_V_1', 'PT_jo_A_Main_1', 'PT_jo_TP'] missing_columns = [col for col in required_columns if col not in data_row.index] if missing_columns: - raise ValueError(f"Missing required columns: {missing_columns}") + available_columns = list(data_row.index) + raise ValueError(f"Missing required columns: {missing_columns}. Available columns: {available_columns}")services/painting-process-data-simulator-service/README.md (1)
1-1: 서비스 명칭 일관성 확인 필요README 제목은 "Backend Simulator Service"이지만 프로젝트 구조에서는 "backend-simulator-service"로 표기되어 있습니다. 또한 PR 목적과 일치하는지 확인이 필요합니다.
-# Painting Process Backend Simulator Service +# Painting Process Data Simulator Serviceservices/painting-process-data-simulator-service/app/services/backend_client.py (1)
9-9: Consider reusing a long-lived AsyncClient to reduce connection overheadCreating a new client per call prevents connection pooling and TLS reuse. Prefer a single client per service lifetime and close it on shutdown (e.g., FastAPI lifespan event). I can help wire this if you want.
services/painting-process-data-simulator-service/tests/test_simulator_router.py (2)
13-15: Align mocked status shape with actual scheduler.get_status output or relax assertionsReal get_status includes interval_seconds and backend_service_url. Your mock returns only two keys, which may hide schema drift. Either add the fields or assert only on required keys.
Apply this diff to relax assertions (alternative provided in next comment):
- mock.get_status.return_value = {"is_running": False, "next_run": None} + mock.get_status.return_value = {"is_running": False, "next_run": None}If you prefer full alignment, import settings and include:
- "interval_seconds": settings.scheduler_interval_seconds
- "backend_service_url": settings.backend_service_url
39-44: Also assert that start() was awaited in the error pathConfirms the endpoint actually awaited the coroutine before producing 500.
response = client.post("/simulator/start") assert response.status_code == 500 assert "시뮬레이터 시작 실패" in response.json()["detail"] + mock_scheduler.start.assert_awaited_once()services/painting-process-data-simulator-service/tests/test_azure_storage.py (2)
25-34: Cover index progression and wrap-around (optional)With a single-row fixture, index wrap-around isn’t observable. Consider extending the fixture to two rows and asserting the second call advances selection. If you’d like, I can provide a patch to expand the fixture and assertions.
36-51: Assert that _load_dataframe resets current_index to 0Matches production behavior and guards against regressions.
await service._load_dataframe() mock_list.assert_awaited_once() mock_read.assert_awaited_once_with('test.csv') assert service.cached_df is not None + assert service.current_index == 0services/painting-process-data-simulator-service/tests/test_scheduler_service.py (1)
27-27: Add idempotency test for start() (optional)start() should no-op when already running. A small test will guard against regressions (e.g., double add_job/start). I can draft it if you want.
services/painting-process-data-simulator-service/app/services/scheduler_service.py (2)
54-55: Use timezone-aware ISO 8601 timestamps in logs.Current log uses a naive local time string, which hampers cross-system correlation. Prefer ISO 8601 with timezone.
Apply this diff:
-from datetime import datetime +from datetime import datetime, timezone- logger.info(f"🔄 데이터 시뮬레이션 및 전송 시작 - {datetime.now().strftime('%Y-%m-%d %H:%M:%S')}") + logger.info(f"🔄 데이터 시뮬레이션 및 전송 시작 - {datetime.now(timezone.utc).isoformat(timespec='seconds')}")Also applies to: 1-1
62-63: Propagate configurable HTTP timeout to backend calls.httpx has a default timeout, but you already have settings.http_timeout. Wire it through to prevent indefinite waits and align with environment configuration.
Apply this diff locally:
- await backend_client.send_to_backend(simulated_data, settings.backend_service_url) + await backend_client.send_to_backend( + simulated_data, + settings.backend_service_url, + timeout=settings.http_timeout + )And update BackendClient accordingly (in app/services/backend_client.py):
# outside the selected range; for reference from typing import Any, Dict, Optional import httpx import logging logger = logging.getLogger(__name__) class BackendClient: async def send_to_backend(self, data: Dict[str, Any], url: str, timeout: Optional[float] = None): async with httpx.AsyncClient(timeout=timeout) as client: try: response = await client.post(url, json=data) response.raise_for_status() logger.info(f"✅ 데이터 전송 성공: {response.status_code}") except httpx.HTTPStatusError as e: logger.error(f"❌ 데이터 전송 실패: {e.response.status_code} - {e.response.text}") except Exception as e: logger.error(f"❌ 데이터 전송 중 예외 발생: {e}") backend_client = BackendClient()services/painting-process-data-simulator-service/app/main.py (1)
41-44: Align service description with the new backend data-sending purpose.The description still mentions a “defect detection model” simulator. Update to reflect the backend data transmission simulator.
Apply this diff:
- description="도장 공정 설비 결함 탐지 모델을 위한 실시간 데이터 시뮬레이터", + description="도장 공정 설비 데이터를 백엔드로 전송하는 실시간 데이터 시뮬레이터",services/painting-process-data-simulator-service/app/config/settings.py (1)
6-7: Optional: Treat secrets as secrets.Consider using SecretStr for azure_connection_string to avoid accidental logging/printing. This requires minor usage changes (call .get_secret_value() where needed).
Example change in this file:
- azure_connection_string: str + azure_connection_string: SecretStrAnd usage (outside this file), e.g., in azure client initialization:
# outside the selected range; for reference conn_str = settings.azure_connection_string.get_secret_value()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
services/painting-process-data-simulator-service/.env.example(1 hunks)services/painting-process-data-simulator-service/README.md(3 hunks)services/painting-process-data-simulator-service/app/config/logging_config.py(1 hunks)services/painting-process-data-simulator-service/app/config/settings.py(1 hunks)services/painting-process-data-simulator-service/app/main.py(1 hunks)services/painting-process-data-simulator-service/app/routers/connection_test_router.py(0 hunks)services/painting-process-data-simulator-service/app/routers/simulator_router.py(2 hunks)services/painting-process-data-simulator-service/app/services/azure_storage.py(2 hunks)services/painting-process-data-simulator-service/app/services/backend_client.py(1 hunks)services/painting-process-data-simulator-service/app/services/model_client.py(0 hunks)services/painting-process-data-simulator-service/app/services/scheduler_service.py(2 hunks)services/painting-process-data-simulator-service/app/utils/logger.py(0 hunks)services/painting-process-data-simulator-service/pytest.ini(0 hunks)services/painting-process-data-simulator-service/requirements.txt(2 hunks)services/painting-process-data-simulator-service/tests/test_azure_storage.py(1 hunks)services/painting-process-data-simulator-service/tests/test_backend_client.py(1 hunks)services/painting-process-data-simulator-service/tests/test_logger.py(0 hunks)services/painting-process-data-simulator-service/tests/test_main.py(0 hunks)services/painting-process-data-simulator-service/tests/test_model_client.py(0 hunks)services/painting-process-data-simulator-service/tests/test_scheduler_service.py(1 hunks)services/painting-process-data-simulator-service/tests/test_settings.py(0 hunks)services/painting-process-data-simulator-service/tests/test_simulator_router.py(1 hunks)
💤 Files with no reviewable changes (8)
- services/painting-process-data-simulator-service/pytest.ini
- services/painting-process-data-simulator-service/tests/test_main.py
- services/painting-process-data-simulator-service/tests/test_logger.py
- services/painting-process-data-simulator-service/app/services/model_client.py
- services/painting-process-data-simulator-service/app/utils/logger.py
- services/painting-process-data-simulator-service/tests/test_model_client.py
- services/painting-process-data-simulator-service/app/routers/connection_test_router.py
- services/painting-process-data-simulator-service/tests/test_settings.py
🧰 Additional context used
🧬 Code Graph Analysis (7)
services/painting-process-data-simulator-service/tests/test_backend_client.py (1)
services/painting-process-data-simulator-service/app/services/backend_client.py (2)
BackendClient(7-17)send_to_backend(8-17)
services/painting-process-data-simulator-service/app/services/scheduler_service.py (2)
services/painting-process-data-simulator-service/app/services/azure_storage.py (1)
simulate_real_time_data(65-101)services/painting-process-data-simulator-service/app/services/backend_client.py (1)
send_to_backend(8-17)
services/painting-process-data-simulator-service/tests/test_scheduler_service.py (3)
services/painting-process-data-simulator-service/app/services/scheduler_service.py (4)
SimulatorScheduler(11-79)start(16-39)stop(41-49)_simulate_and_send_data(51-65)services/painting-process-data-simulator-service/app/services/azure_storage.py (1)
simulate_real_time_data(65-101)services/painting-process-data-simulator-service/app/services/backend_client.py (1)
send_to_backend(8-17)
services/painting-process-data-simulator-service/tests/test_simulator_router.py (1)
services/painting-process-data-simulator-service/app/services/scheduler_service.py (3)
get_status(67-79)start(16-39)stop(41-49)
services/painting-process-data-simulator-service/tests/test_azure_storage.py (1)
services/painting-process-data-simulator-service/app/services/azure_storage.py (3)
AzureStorageService(10-122)simulate_real_time_data(65-101)_load_dataframe(103-122)
services/painting-process-data-simulator-service/app/routers/simulator_router.py (1)
services/painting-process-data-simulator-service/app/services/scheduler_service.py (1)
get_status(67-79)
services/painting-process-data-simulator-service/app/main.py (1)
services/painting-process-data-simulator-service/app/config/logging_config.py (1)
setup_logging(6-15)
🪛 dotenv-linter (3.3.0)
services/painting-process-data-simulator-service/.env.example
[warning] 17-17: [ExtraBlankLine] Extra blank line detected
[warning] 28-28: [ExtraBlankLine] Extra blank line detected
[warning] 33-33: [ExtraBlankLine] Extra blank line detected
[warning] 38-38: [ExtraBlankLine] Extra blank line detected
🔇 Additional comments (10)
services/painting-process-data-simulator-service/requirements.txt (5)
12-12: Bump to charset-normalizer 3.4.3 looks goodThis aligns with requests 2.32.x constraints (<4.0). No compatibility concerns on this pin.
49-49: yarl 1.20.1 pin is appropriate for aiohttp 3.12.xThis version pairing is commonly used and should be compatible with aiohttp 3.12.15.
1-49: No known vulnerabilities detected in key dependenciesA quick OSV scan of FastAPI, Starlette, HTTPX, Requests, and Cryptography returned zero known issues for the pinned versions.
Optional best practices to further harden your supply chain:
- Generate a lock/constraints file with hashes (e.g., via pip-compile or pip-tools) and install using --require-hashes
- Schedule periodic CVE audits to catch newly disclosed vulnerabilities
24-26: Confirmed Python 3.10 Base Image Meets NumPy 2.2 Requirement
NumPy 2.2.x requires Python >=3.10, andservices/painting-process-data-simulator-service/DockerfileusesFROM python:3.10. No changes required.
15-16: Dependencies pins satisfy upstream constraintsA programmatic check against PyPI’s requires_dist for fastapi 0.116.1, httpx 0.28.1, azure-storage-blob 12.26.0, and aiohttp 3.12.15 (covering their Starlette, Pydantic, HTTPCore, AnyIO, SniffIO, etc. requirements) returned “OK”—no version conflicts detected. No changes required.
services/painting-process-data-simulator-service/app/services/azure_storage.py (1)
3-3: 비동기 import 사용되었으나 동기 클라이언트 사용 중
azure.storage.blob.aio에서BlobServiceClient를 import하고 있으나, 실제로는 Line 16에서 동기 방식의from_connection_string을 사용하고 있습니다. 비동기 처리를 위해서는 올바른 import와 초기화가 필요합니다.-from azure.storage.blob.aio import BlobServiceClient +from azure.storage.blob.aio import BlobServiceClient as AsyncBlobServiceClient그리고 초기화 부분도 수정이 필요합니다:
- self.blob_service_client = BlobServiceClient.from_connection_string(self.connection_string) + self.blob_service_client = AsyncBlobServiceClient.from_connection_string(self.connection_string)Likely an incorrect or invalid review comment.
services/painting-process-data-simulator-service/tests/test_azure_storage.py (1)
3-3: All async declarations verifiedBoth
list_data_files(line 22) andread_csv_data(line 43) inazure_storage.pyare declared asasync defand are correctly awaited by callers (lines 106 and 115). No changes to the tests are needed.services/painting-process-data-simulator-service/tests/test_scheduler_service.py (3)
14-16: Good isolation of scheduler internals via patchingPatching add_job/start/shutdown keeps this a pure unit test without touching APScheduler internals. Assertions are sufficient.
28-43: Robust and focused verification of data path (azure -> backend)Nice use of AsyncMock to assert awaited calls and correct URL propagation.
45-59: Good negative-path testVerifies that no backend call happens when no data is available.
services/painting-process-data-simulator-service/app/config/logging_config.py
Show resolved
Hide resolved
services/painting-process-data-simulator-service/app/routers/simulator_router.py
Outdated
Show resolved
Hide resolved
services/painting-process-data-simulator-service/app/routers/simulator_router.py
Outdated
Show resolved
Hide resolved
services/painting-process-data-simulator-service/app/services/scheduler_service.py
Show resolved
Hide resolved
services/painting-process-data-simulator-service/app/services/scheduler_service.py
Show resolved
Hide resolved
| with patch('httpx.AsyncClient') as mock_async_client: | ||
| mock_client_instance = AsyncMock() | ||
| mock_async_client.return_value.__aenter__.return_value = mock_client_instance | ||
|
|
||
| await client.send_to_backend(test_data, test_url) | ||
|
|
||
| mock_client_instance.post.assert_awaited_once_with(test_url, json=test_data) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
HTTP 에러 상황에 대한 테스트 추가 필요
현재 테스트는 성공 케이스만 검증하고 있습니다. BackendClient의 에러 처리 로직도 테스트해야 합니다.
HTTP 에러나 네트워크 예외 상황에 대한 추가 테스트 케이스를 생성하시겠습니까? 예를 들어:
- HTTP 500 에러 응답 처리
- 네트워크 연결 실패
- 타임아웃 발생
다음과 같은 테스트를 추가할 수 있습니다:
@pytest.mark.asyncio
async def test_send_to_backend_http_error():
"""Test that the backend client handles HTTP errors correctly."""
client = BackendClient()
test_data = {"test_key": "test_value"}
test_url = "http://test-backend.com/api"
with patch('httpx.AsyncClient') as mock_async_client:
mock_client_instance = AsyncMock()
mock_async_client.return_value.__aenter__.return_value = mock_client_instance
# Simulate HTTP 500 error
mock_response = AsyncMock()
mock_response.raise_for_status.side_effect = httpx.HTTPStatusError(
"Server Error", request=None, response=AsyncMock(status_code=500, text="Internal Server Error")
)
mock_client_instance.post.return_value = mock_response
# Should not raise exception but log the error
await client.send_to_backend(test_data, test_url)
mock_client_instance.post.assert_awaited_once_with(test_url, json=test_data)🤖 Prompt for AI Agents
In services/painting-process-data-simulator-service/tests/test_backend_client.py
around lines 12 to 18, add tests covering error paths for
BackendClient.send_to_backend: create separate async test cases that patch
'httpx.AsyncClient' similar to the existing success test but simulate (1) an
HTTP error by making the mocked post() return a response whose
raise_for_status() raises httpx.HTTPStatusError (with a mock
response.status_code 500), (2) a network/connect failure by having post() raise
httpx.ConnectError, and (3) a timeout by having post() raise httpx.ReadTimeout
or httpx.TimeoutException; in each test assert the mocked post was awaited with
the expected url and json and that send_to_backend does not propagate the
exception (i.e., completes) and optionally asserts the logger was called with an
error message.
| def test_get_simulator_status(mock_scheduler): | ||
| """Test GET /simulator/status endpoint.""" | ||
| response = client.get("/simulator/status") | ||
|
|
||
| # Then | ||
| assert response.status_code == 200 | ||
| data = response.json() | ||
| assert data["is_running"] is False | ||
| assert "model_services_health" not in data | ||
| assert response.json() == {"is_running": False, "next_run": None} | ||
| mock_scheduler.get_status.assert_called_once() | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make the status assertion resilient to additional fields
Avoid strict equality so additions to the status schema don’t break the test.
- assert response.json() == {"is_running": False, "next_run": None}
- mock_scheduler.get_status.assert_called_once()
+ payload = response.json()
+ assert payload.get("is_running") is False
+ assert "next_run" in payload
+ mock_scheduler.get_status.assert_called_once()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_get_simulator_status(mock_scheduler): | |
| """Test GET /simulator/status endpoint.""" | |
| response = client.get("/simulator/status") | |
| # Then | |
| assert response.status_code == 200 | |
| data = response.json() | |
| assert data["is_running"] is False | |
| assert "model_services_health" not in data | |
| assert response.json() == {"is_running": False, "next_run": None} | |
| mock_scheduler.get_status.assert_called_once() | |
| def test_get_simulator_status(mock_scheduler): | |
| """Test GET /simulator/status endpoint.""" | |
| response = client.get("/simulator/status") | |
| assert response.status_code == 200 | |
| payload = response.json() | |
| assert payload.get("is_running") is False | |
| assert "next_run" in payload | |
| mock_scheduler.get_status.assert_called_once() |
🤖 Prompt for AI Agents
In
services/painting-process-data-simulator-service/tests/test_simulator_router.py
around lines 18 to 24, the test uses strict equality on the entire response JSON
which will break if extra fields are added; change the assertion to only verify
the expected keys and their values (e.g., assert
response.json().get("is_running") is False and response.json().get("next_run")
is None) or assert that the expected items are a subset of
response.json().items(), so the test remains resilient to additional fields
while still validating the important values.
TaeHyunaivle
changed the title
TaeHyunaivle
changed the title
래빗코드의 조언에 따라 코드를 리팩토링하고 잠재된 문제 및 오류들을 감지하고 대응
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (1)
services/painting-process-data-simulator-service/app/routers/simulator_router.py (1)
23-24: Hide internal error details in /start response (mirror /stop).Currently returns the exception message to clients. Log internally and return a generic message instead.
Apply this diff:
- except Exception as e: - raise HTTPException(status_code=500, detail=f"시뮬레이터 시작 실패: {str(e)}") from e + except Exception: + logger.exception("시뮬레이터 시작 실패") + raise HTTPException(status_code=500, detail="시뮬레이터 시작 실패")
♻️ Duplicate comments (2)
services/painting-process-data-simulator-service/app/config/settings.py (1)
2-2: Good adoption of URL typing and interval validation (matches prior feedback).Using AnyHttpUrl for backend_service_url and validating scheduler_interval_seconds > 0 address configuration safety concerns raised earlier.
Also applies to: 13-13, 16-18, 25-31
services/painting-process-data-simulator-service/README.md (1)
16-16: 이전 리뷰 사항 반영 확인: 디렉토리명 일치 OK.프로젝트 구조에 "painting-process-data-simulator-service/"로 수정되어 이전 불일치 이슈가 해결되었습니다. 좋습니다.
🧹 Nitpick comments (10)
services/painting-process-data-simulator-service/app/config/settings.py (1)
5-7: Treat the Azure connection string as a secret to avoid accidental logging.Typing azure_connection_string as SecretStr helps prevent accidental exposure in logs and reprs.
Apply this diff:
-from pydantic import AnyHttpUrl, field_validator +from pydantic import AnyHttpUrl, field_validator, SecretStr @@ - azure_connection_string: str + azure_connection_string: SecretStrservices/painting-process-data-simulator-service/app/config/logging_config.py (1)
10-16: Optional: add log rotation to prevent unbounded log growth.RotatingFileHandler avoids filling the disk in long-running containers.
Example (outside current hunk):
from logging.handlers import RotatingFileHandler handlers = [logging.StreamHandler(sys.stdout)] try: handlers.append( RotatingFileHandler( f"{settings.log_directory}/service.log", maxBytes=5_000_000, backupCount=5, ) ) except OSError as e: logging.warning("RotatingFileHandler creation failed; using stdout-only logging. (%s)", e)services/painting-process-data-simulator-service/app/services/backend_client.py (1)
7-26: Optional: reuse a single AsyncClient for connection pooling.Creating a new AsyncClient per request prevents keep-alive reuse and adds overhead. Manage a client lifecycle on app startup/shutdown (FastAPI lifespan) or expose aclose() on the client.
Example refactor (outside current hunk):
class BackendClient: def __init__(self) -> None: self._client = httpx.AsyncClient(timeout=httpx.Timeout(settings.http_timeout)) async def aclose(self) -> None: await self._client.aclose() async def send_to_backend(self, data: Dict[str, Any], url: str) -> bool: try: response = await self._client.post(url, json=data) response.raise_for_status() logger.info("✅ 데이터 전송 성공: %s", response.status_code) return True except httpx.HTTPStatusError as e: logger.error("❌ 데이터 전송 실패(HTTP %s)", getattr(e.response, "status_code", "unknown")) return False except httpx.RequestError as e: logger.error("❌ 네트워크 오류로 데이터 전송 실패: %r", e) return False except Exception: logger.exception("❌ 데이터 전송 중 예외 발생") return FalseWire BackendClient.aclose() into the app’s lifespan to cleanly close on shutdown.
services/painting-process-data-simulator-service/app/services/scheduler_service.py (3)
29-31: Externalize misfire_grace_time to settings for configurability.Hardcoding 30 makes tuning harder across environments.
Apply this diff here:
- misfire_grace_time=30, # seconds to allow a delayed run + misfire_grace_time=settings.scheduler_misfire_grace_seconds, # secondsAnd add to Settings (in app/config/settings.py, example outside current hunk):
# 스케줄러 설정 scheduler_misfire_grace_seconds: int = 30
66-67: Surface send outcome to logs/metrics.send_to_backend returns a bool but the result is ignored. Log failures (or consider a retry/backoff strategy later).
Apply this diff:
- await backend_client.send_to_backend(simulated_data, settings.backend_service_url) + ok = await backend_client.send_to_backend(simulated_data, settings.backend_service_url) + if not ok: + logger.warning("⚠️ 백엔드 전송 실패 (재시도 미구현)")
38-40: Minor: avoid logging full backend URL unless necessary.The router already strips backend_service_url from public responses; consider avoiding logging the full URL here if logs may be exposed. Log host or path only.
Example:
from urllib.parse import urlparse target = urlparse(str(settings.backend_service_url)) logger.info("🎯 대상 백엔드 서비스: %s://%s%s", target.scheme, target.netloc, target.path)services/painting-process-data-simulator-service/README.md (4)
9-11: 라이브러리 표기 및 정확성 정리(HTTPX → httpx).공식 패키지명은 소문자 "httpx"입니다. README의 표기와 requirements.txt 내용이 일치하도록 맞춰주세요.
적용 제안(diff):
-- **Spring Boot 백엔드 연동**: `HTTPX` 클라이언트를 사용하여 Spring Boot 백엔드의 API를 비동기적으로 호출합니다. +- **Spring Boot 백엔드 연동**: `httpx` 클라이언트를 사용하여 Spring Boot 백엔드의 API를 비동기적으로 호출합니다.
36-46: venv 활성화 명령 주석 제거.Linux/macOS 활성화 명령이 주석 처리되어 있어 혼란을 줄 수 있습니다.
적용 제안(diff):
- # macOS/Linux - # source venv/bin/activate + # macOS/Linux + source venv/bin/activate
66-72: 엔드포인트 사용 예시(cURL)와 요청 스키마를 명확히 해주세요./simulator/start의 요청 본문(필요한 경우), 각 엔드포인트의 응답 스키마/예시를 제공하면 소비자 측 혼선을 줄일 수 있습니다. 또한 존재한다면 /health 엔드포인트도 함께 문서화하세요.
추가 제안(diff; 표 아래 예시 삽입):
| `GET` | `/simulator/status`| 현재 스케줄러의 상태를 확인합니다. | +#### 사용 예시 +```bash +# Swagger UI +open http://localhost:8011/docs + +# 시뮬레이터 시작 (요청 본문이 필요 없다면 -d 생략) +curl -X POST http://localhost:8011/simulator/start \ + -H "Content-Type: application/json" \ + -d '{"intervalSeconds": 5}' + +# 상태 확인 +curl http://localhost:8011/simulator/status + +# 시뮬레이터 중지 +curl -X POST http://localhost:8011/simulator/stop +``` + +> 참고: intervalSeconds, 대상 Blob 범위 등 런타임 파라미터를 API로 설정할 수 없는 경우, 환경 변수로 구성하세요(위 .env 섹션 참조). + +> (선택) 헬스체크 엔드포인트(`/health` 또는 `/`)가 존재하면 문서화해 주세요.
74-81: 로그 보존/회전 정책 및 경로 설정 명시.로그 파일이 무한 성장하지 않도록 회전 정책(예: RotatingFileHandler)과 LOG_DIRECTORY 설정/권한 이슈를 문서화하세요. 컨테이너에서는 volume 마운트를 권장합니다.
추가 제안(diff):
- **파일**: 로그는 `logs/service.log` 파일에도 저장되어, 서비스 실행 이력을 확인할 수 있습니다. + - **회전 정책**: 로그 파일 크기/개수 제한을 설정하여 디스크 점유를 방지하세요(예: 10MB × 5개 순환). + - **경로 설정**: `LOG_DIRECTORY` 환경 변수로 로그 디렉토리를 지정할 수 있습니다. 컨테이너 사용 시 `-v $(pwd)/logs:/app/logs`로 호스트에 마운트하는 것을 권장합니다.확인 요청:
- logging_config.py에서 회전 핸들러 사용 여부와 LOG_DIRECTORY 반영 여부를 확인해 README와 일치시켜 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
services/painting-process-data-simulator-service/README.md(3 hunks)services/painting-process-data-simulator-service/app/config/logging_config.py(1 hunks)services/painting-process-data-simulator-service/app/config/settings.py(2 hunks)services/painting-process-data-simulator-service/app/main.py(1 hunks)services/painting-process-data-simulator-service/app/routers/simulator_router.py(2 hunks)services/painting-process-data-simulator-service/app/services/azure_storage.py(2 hunks)services/painting-process-data-simulator-service/app/services/backend_client.py(1 hunks)services/painting-process-data-simulator-service/app/services/scheduler_service.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- services/painting-process-data-simulator-service/app/main.py
- services/painting-process-data-simulator-service/app/services/azure_storage.py
🧰 Additional context used
🧬 Code Graph Analysis (2)
services/painting-process-data-simulator-service/app/services/scheduler_service.py (2)
services/painting-process-data-simulator-service/app/services/azure_storage.py (1)
simulate_real_time_data(71-107)services/painting-process-data-simulator-service/app/services/backend_client.py (1)
send_to_backend(8-24)
services/painting-process-data-simulator-service/app/routers/simulator_router.py (1)
services/painting-process-data-simulator-service/app/services/scheduler_service.py (1)
get_status(71-83)
🪛 Ruff (0.12.2)
services/painting-process-data-simulator-service/app/routers/simulator_router.py
36-36: Undefined name logger
(F821)
🔇 Additional comments (2)
services/painting-process-data-simulator-service/app/routers/simulator_router.py (1)
9-12: Good: internal URL removed from public /status.Popping backend_service_url before returning avoids leaking internal endpoints.
services/painting-process-data-simulator-service/app/services/scheduler_service.py (1)
51-53: Non-blocking shutdown looks good.Using asyncio.to_thread avoids blocking the event loop when shutting down the scheduler.
services/painting-process-data-simulator-service/app/config/logging_config.py
Show resolved
Hide resolved
services/painting-process-data-simulator-service/app/config/settings.py
Outdated
Show resolved
Hide resolved
| except Exception as e: | ||
| logger.exception("시뮬레이터 중지 실패") | ||
| raise HTTPException(status_code=500, detail="시뮬레이터 중지 실패") from e No newline at end of file |
There was a problem hiding this comment.
Fix NameError: logger is undefined.
Static analysis flagged logger as undefined. Define a module logger to use logger.exception safely.
Add these lines near the top of the file (after imports):
import logging
logger = logging.getLogger(__name__)🧰 Tools
🪛 Ruff (0.12.2)
36-36: Undefined name logger
(F821)
🤖 Prompt for AI Agents
In
services/painting-process-data-simulator-service/app/routers/simulator_router.py
around lines 35 to 37, the exception handler uses logger.exception but no module
logger is defined; add an import for the logging module and define a
module-level logger (e.g., import logging and logger =
logging.getLogger(__name__)) near the top of the file after the existing imports
so logger.exception can be called safely.
services/painting-process-data-simulator-service/app/services/backend_client.py
Outdated
Show resolved
Hide resolved
| @@ -1,35 +1,30 @@ | |||
| # Painting Process Equipment Simulator Service | |||
| # Painting Process Backend Simulator Service | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
문서 타이틀을 서비스 명칭과 일치시키세요.
PR/디렉토리명은 "painting-process-data-simulator-service"인데, 제목은 "Backend Simulator"로 되어 있습니다. 명칭 혼용은 혼란을 유발합니다.
적용 제안(diff):
-# Painting Process Backend Simulator Service
+# Painting Process Data Simulator Service📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Painting Process Backend Simulator Service | |
| # Painting Process Data Simulator Service |
🤖 Prompt for AI Agents
services/painting-process-data-simulator-service/README.md lines 1-1: README 제목이
디렉토리 및 서비스명과 일치하지 않아 혼동을 유발합니다; 파일 상단의 제목을 현재 디렉토리명/서비스명인
"painting-process-data-simulator-service" 또는 더 읽기 쉬운 서비스명(예: "Painting Process
Data Simulator Service")로 변경하여 PR 및 디렉토리명과 일치시키세요.
| │ │ ├── settings.py # Pydantic-settings를 이용한 환경 변수 및 설정 관리 | ||
| │ │ └── logging_config.py # 서비스 로깅 설정 | ||
| │ ├── routers/ | ||
| │ │ ├── simulator_router.py # 시뮬레이터 시작/중지/상태 확인 API | ||
| │ │ └── test_connection_router.py # 외부 서비스(Azure, 모델) 연결 테스트 API | ||
| │ ├── services/ | ||
| │ │ ├── scheduler_service.py # APScheduler를 사용한 핵심 스케줄링 로직 | ||
| │ │ ├── model_client.py # 모델 예측 서비스 API 호출 클라이언트 | ||
| │ │ └── azure_storage.py # Azure Blob Storage 데이터 처리 서비스 | ||
| │ └── utils/ | ||
| │ └── logger.py # 이상 및 정상 로그 기록 유틸리티 | ||
| ├── logs/ # 시뮬레이션 결과 로그가 저장되는 디렉토리 | ||
| │ │ └── simulator_router.py # 시뮬레이터 시작/중지/상태 확인 API | ||
| │ └── services/ | ||
| │ ├── scheduler_service.py # APScheduler를 사용한 핵심 스케줄링 로직 | ||
| │ ├── backend_client.py # Spring Boot 백엔드 API 호출 클라이언트 | ||
| │ └── azure_storage.py # Azure Blob Storage 데이터 처리 서비스 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
설정 항목(환경 변수) 문서화 보강 제안.
settings.py에서 관리하는 주요 설정값(예: scheduler_interval_seconds, backend_service_url, log_directory 등)의 키명/기본값/유효성 제약을 README에 명시해 주세요. 운영/도커 배포 시 필수 정보입니다.
원하는 경우, .env 섹션 아래에 "환경 변수 요약" 표를 추가하는 패치를 제안할 수 있습니다.
🤖 Prompt for AI Agents
In services/painting-process-data-simulator-service/README.md around lines 20 to
27, add documentation for the key environment settings managed in settings.py:
list each env var name (e.g., scheduler_interval_seconds, backend_service_url,
log_directory, azure_blob_connection_string, storage_container,
scheduler_max_workers), their default values, expected types/validation rules
(ranges, required formats, URL pattern, integer min/max), and whether they are
mandatory for production/docker; include an ".env" subsection with a concise "환경
변수 요약" table showing key, default, required, and brief description so operators
can copy into Docker or deployment manifests.
| │ ├── scheduler_service.py # APScheduler를 사용한 핵심 스케줄링 로직 | ||
| │ ├── backend_client.py # Spring Boot 백엔드 API 호출 클라이언트 | ||
| │ └── azure_storage.py # Azure Blob Storage 데이터 처리 서비스 | ||
| ├── .env # Azure 연결 문자열 등 민감한 환경 변수 파일 |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
보안/운영 혼동 방지: .env 대신 .env.example 표시 권장.
프로젝트 트리에 .env를 직접 표기하면 커밋 오해를 유발합니다. 예시 파일(.env.example)을 트리에 노출하고, .env는 gitignore 대상임을 명시하세요.
적용 제안(diff):
-├── .env # Azure 연결 문자열 등 민감한 환경 변수 파일
+├── .env.example # 환경 변수 예시 파일 (.env는 git에 커밋하지 않음)또한 .gitignore에 .env가 포함되어 있는지 확인 부탁드립니다.
🏁 Script executed:
#!/bin/bash
# Locate all .gitignore files and check for .env entries
find . -type f -name ".gitignore" | while read gitignore; do
echo "Checking $gitignore"
if grep -qE '(^|/)\.env($|[[:space:]])' "$gitignore"; then
echo " ✅ .env is already listed in $gitignore"
else
echo " ❌ .env is NOT listed in $gitignore"
fi
doneLength of output: 267
🏁 Script executed:
#!/bin/bash
# Locate the README for the painting-process-data-simulator-service
target=$(find . -type f -name README.md | grep painting-process-data-simulator-service)
echo "Found README at: $target"
echo "Lines with '.env':"
grep -n "├── .env" "$target" || echo "No matches found"Length of output: 521
Replace .env entry in service README with .env.example
The .env file is already listed in .gitignore, so please update the tree diagram in the service’s README to refer to an example file instead:
• File: services/painting-process-data-simulator-service/README.md
– Line 28: change the .env entry to .env.example
Suggested patch:
--- a/services/painting-process-data-simulator-service/README.md
+++ b/services/painting-process-data-simulator-service/README.md
@@ -28,1 +28,1 @@
-├── .env # Azure 연결 문자열 등 민감한 환경 변수 파일
+├── .env.example # 환경 변수 예시 파일 (.env는 git에 커밋하지 않음)Ensure you also include a real services/painting-process-data-simulator-service/.env.example with placeholder values.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ├── .env # Azure 연결 문자열 등 민감한 환경 변수 파일 | |
| ├── .env.example # 환경 변수 예시 파일 (.env는 git에 커밋하지 않음) |
🤖 Prompt for AI Agents
In services/painting-process-data-simulator-service/README.md around line 28,
the tree diagram currently shows a private `.env` file; change that entry to
`.env.example` and update the README accordingly, and add a new
services/painting-process-data-simulator-service/.env.example file containing
placeholder values for each required environment variable (do not include real
secrets), ensuring the example matches the variables used by the service and
that `.env` remains gitignored.
| 3. **.env 파일 설정**: | ||
| 프로젝트 루트에 `.env` 파일을 생성하고, Azure Storage 연결 문자열을 추가합니다. | ||
| 프로젝트 루트에 `.env` 파일을 생성하고, Azure Storage 연결 문자열과 백엔드 서비스 URL을 추가합니다. | ||
| ```env | ||
| AZURE_CONNECTION_STRING="<Your_Azure_Storage_Connection_String>" | ||
| BACKEND_SERVICE_URL="<Your_Backend_Service_URL>" | ||
| ``` |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
.env 예시에 필수/권장 변수 추가.
스케줄러 주기와 로그 디렉토리 등 핵심 설정을 함께 예시로 제공하세요. 운영 시 초기 장애를 줄여줍니다.
적용 제안(diff):
AZURE_CONNECTION_STRING="<Your_Azure_Storage_Connection_String>"
BACKEND_SERVICE_URL="<Your_Backend_Service_URL>"
+ # 전송 주기(초). 예: 5
+ SCHEDULER_INTERVAL_SECONDS=5
+ # 로그 디렉토리(컨테이너/호스트 모두 쓰기 가능 경로)
+ LOG_DIRECTORY="logs"
+ # (선택) Azure Blob 선택을 위한 컨테이너/경로 설정이 필요한 경우 아래 키 사용
+ # AZURE_BLOB_CONTAINER="your-container"
+ # AZURE_BLOB_PREFIX="path/prefix/"확인 요청:
- settings.py에서 실제 사용하는 키명이 위와 동일한지 검증 필요합니다. 불일치 시 README를 settings와 동일하게 맞춰주세요.
🏁 Script executed:
#!/bin/bash
# Locate all settings.py files in the repo
echo "=== settings.py files ==="
fd --hidden --glob settings.py
# Search for environment variable accesses in the simulator service
echo "=== getenv and environ usage in service folder ==="
grep -R --color=always -nE "os\.getenv|os\.environ" services/painting-process-data-simulator-service
# Search specifically for scheduler and log-related keys
echo "=== Scheduler & Log keys ==="
grep -R -nE "SCHEDULER|LOG_DIRECTORY|BLOB_CONTAINER|BLOB_PREFIX" -n services/painting-process-data-simulator-serviceLength of output: 731
🏁 Script executed:
#!/bin/bash
# Display the settings.py for painting-process-data-simulator-service
sed -n '1,200p' services/painting-process-data-simulator-service/app/config/settings.pyLength of output: 1066
.env 예시: 필수/권장 변수 추가 및 settings.py 키 이름 동기화
README의 .env 예시에 services/painting-process-data-simulator-service/app/config/settings.py에서 사용하는 모든 환경 변수 키를 반영해주세요. 주요 변경 사항:
AZURE_CONTAINER_NAME,PAINTING_DATA_FOLDER,BATCH_SIZE,HTTP_TIMEOUT항목을 추가하여 기본값과 함께 예시 제공- 기존 제안된
AZURE_BLOB_CONTAINER/AZURE_BLOB_PREFIX대신, 실제 사용하는AZURE_CONTAINER_NAME으로 맞춤 SCHEDULER_INTERVAL_SECONDS와LOG_DIRECTORY는 기본값 30초/“logs”로 유지
적용할 diff 예시 (파일: services/painting-process-data-simulator-service/README.md, 해당 섹션 교체):
AZURE_CONNECTION_STRING="<Your_Azure_Storage_Connection_String>"
BACKEND_SERVICE_URL="<Your_Backend_Service_URL>"
+# Azure Blob 컨테이너 이름 (기본: "simulator-data")
+AZURE_CONTAINER_NAME="simulator-data"
+# 페인팅 프로세스 데이터 저장 폴더 (기본: "painting-process-equipment")
+PAINTING_DATA_FOLDER="painting-process-equipment"
+# 스케줄러 주기(초) (기본: 30)
+SCHEDULER_INTERVAL_SECONDS=30
+# 배치 사이즈 (기본: 10)
+BATCH_SIZE=10
+# HTTP 타임아웃(초) (기본: 30)
+HTTP_TIMEOUT=30
+# 로그 디렉토리 (기본: "logs")
+LOG_DIRECTORY="logs"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 3. **.env 파일 설정**: | |
| 프로젝트 루트에 `.env` 파일을 생성하고, Azure Storage 연결 문자열을 추가합니다. | |
| 프로젝트 루트에 `.env` 파일을 생성하고, Azure Storage 연결 문자열과 백엔드 서비스 URL을 추가합니다. | |
| ```env | |
| AZURE_CONNECTION_STRING="<Your_Azure_Storage_Connection_String>" | |
| BACKEND_SERVICE_URL="<Your_Backend_Service_URL>" | |
| ``` | |
| 3. **.env 파일 설정**: | |
| 프로젝트 루트에 `.env` 파일을 생성하고, Azure Storage 연결 문자열과 백엔드 서비스 URL을 추가합니다. |
🤖 Prompt for AI Agents
In services/painting-process-data-simulator-service/README.md around lines 52 to
57, the .env example is missing several variables used by
app/config/settings.py; update the example to replace
AZURE_BLOB_CONTAINER/AZURE_BLOB_PREFIX with AZURE_CONTAINER_NAME and add
AZURE_CONTAINER_NAME, PAINTING_DATA_FOLDER, BATCH_SIZE, HTTP_TIMEOUT (with
sensible defaults), and keep SCHEDULER_INTERVAL_SECONDS=30 and
LOG_DIRECTORY="logs"; ensure the README .env sample keys exactly match the names
used in settings.py and include brief default values for each variable.
| uvicorn app.main:app --reload --port 8011 | ||
| ``` | ||
| 실행 후 `http://localhost:8011/docs`에서 API 문서를 확인할 수 있습니다. | ||
|
|
||
| ### 4.2. Docker를 이용한 실행 | ||
|
|
||
| **사전 준비:** `painting-process-equipment-defect-detection-model-service`가 `model-service`라는 컨테이너 이름으로 동일한 Docker 네트워크(`smart-fast-net`)에서 실행 중이어야 합니다. | ||
|
|
||
| 1. **Docker 네트워크 생성** (이미 생성했다면 생략): | ||
| ```bash | ||
| docker network create smart-fast-net | ||
| ``` | ||
|
|
||
| 2. **모델 서비스 실행** (이미 실행 중이라면 생략): | ||
| ```bash | ||
| # 모델 서비스 디렉토리에서 실행 | ||
| docker build -t model-service . | ||
| docker run --name model-service --network smart-fast-net -p 8001:8001 model-service | ||
| ``` | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Docker 실행 가이드 추가 제안.
PR 요약에 “Docker 환경(포트 8011)”이 강조되었으나, README에는 Docker 빌드/실행 예시가 없습니다. 도커 사용자 가이드가 필요합니다.
추가 제안(diff; 섹션 하단에 삽입):
uvicorn app.main:app --reload --port 8011
}
+### Docker로 실행
+
+```bash
+# 이미지 빌드
+docker build -t painting-process-data-simulator:latest .
+
+# (권장) .env 사용 시
+docker run --rm -p 8011:8011 --env-file ./.env \
+ -v $(pwd)/logs:/app/logs \
+ --name painting-data-simulator \
+ painting-process-data-simulator:latest
+
+# (대안) 개별 환경변수 전달
+# docker run --rm -p 8011:8011 \
+# -e AZURE_CONNECTION_STRING="..." \
+# -e BACKEND_SERVICE_URL="..." \
+# -e SCHEDULER_INTERVAL_SECONDS=5 \
+# -e LOG_DIRECTORY="logs" \
+# -v $(pwd)/logs:/app/logs \
+# painting-process-data-simulator:latest
+```🤖 Prompt for AI Agents
In services/painting-process-data-simulator-service/README.md around lines
61-63, the README lacks the Docker build and run examples referenced in the PR;
add a new subsection at the bottom showing how to build the image and run it
with port 8011 mapped, an example using --env-file and mounting ./logs to
/app/logs, and an alternative showing individual -e environment variables for
AZURE_CONNECTION_STRING, BACKEND_SERVICE_URL, SCHEDULER_INTERVAL_SECONDS and
LOG_DIRECTORY; ensure commands include docker build -t
painting-process-data-simulator:latest ., docker run --rm -p 8011:8011
--env-file ./.env -v $(pwd)/logs:/app/logs --name painting-data-simulator
painting-process-data-simulator:latest and commented example of docker run with
explicit -e variables.
래빗코드의 조언에 따라 코드를 리팩토링하고 잠재된 문제 및 오류들을 감지하고 대응
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
services/painting-process-data-simulator-service/app/config/settings.py (2)
22-24: Make sure the log directory exists and consider rotationEnsure your logging bootstrap creates settings.log_directory (and sets sane rotation/retention) to avoid runtime failures and log bloat.
Example (outside this file, e.g., in logging_config.py):
from pathlib import Path def ensure_log_dir(path: str) -> None: Path(path).mkdir(parents=True, exist_ok=True)
33-38: Make the shared validator’s error message field-specificThe current message mentions both fields even when only one fails. Use FieldValidationInfo to tailor the message.
Apply this diff:
- @field_validator("batch_size", "http_timeout") - @classmethod - def _positive_ints(cls, v: int) -> int: - if v <= 0: - raise ValueError("batch_size and http_timeout must be > 0") - return v + @field_validator("batch_size", "http_timeout") + @classmethod + def _positive_ints(cls, v: int, info: "FieldValidationInfo") -> int: + if v <= 0: + raise ValueError(f"{info.field_name} must be > 0") + return vAdd this import outside the selected range:
from pydantic import FieldValidationInfo
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
services/painting-process-data-simulator-service/app/config/logging_config.py(1 hunks)services/painting-process-data-simulator-service/app/config/settings.py(2 hunks)services/painting-process-data-simulator-service/app/services/backend_client.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- services/painting-process-data-simulator-service/app/config/logging_config.py
- services/painting-process-data-simulator-service/app/services/backend_client.py
🔇 Additional comments (2)
services/painting-process-data-simulator-service/app/config/settings.py (2)
2-2: Strong typing for URL and validator import — LGTMGood move switching to AnyHttpUrl and adopting field_validator; this tightens config correctness at load time.
13-15: ✅ All painting-process-data-simulator references updated to seconds-based intervalVerified in services/painting-process-data-simulator-service that:
- app/config/settings.py defines only
scheduler_interval_seconds- app/main.py logs use
settings.scheduler_interval_seconds- app/services/scheduler_service.py uses
IntervalTrigger(seconds=…)and returns"interval_seconds".env.examplecontainsSCHEDULER_INTERVAL_SECONDS=…and no minute-based keys remainNo further changes required.
| # Backend 서비스 설정 | ||
| backend_service_url: AnyHttpUrl = "http://localhost:8087/equipment-data" | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure backend endpoint path isn’t double-appended
Default value includes a path (/equipment-data). Confirm the HTTP client doesn’t also append the same segment, which would yield duplicate paths.
Run:
If you intend backend_service_url to include the full endpoint (vs. just base URL), consider renaming to backend_endpoint_url for clarity.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Where is backend_service_url used?"
rg -n -S 'backend_service_url' -A3 -B3
echo
echo "Where does '/equipment-data' appear (possible hard-coded appends)?"
rg -n -S '/equipment-data' -A2 -B2Length of output: 6466
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Search for backend_client definition/import:"
rg -n --color always 'backend_client' -A5 -B5 services/painting-process-data-simulator-service/appLength of output: 4266
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Displaying backend_client.py:"
sed -n '1,200p' services/painting-process-data-simulator-service/app/services/backend_client.pyLength of output: 1261
backend_service_url is used verbatim—no double-appended path
We confirmed in services/backend_client.py that send_to_backend calls client.post(url, …) without modifying the URL, so supplying "http://localhost:8087/equipment-data" will not lead to duplicated segments.
• No code concatenates "/equipment-data" onto settings.backend_service_url.
• The default value already includes the full endpoint path.
For clarity (optional):
- Rename
settings.backend_service_url→settings.backend_endpoint_urlto signal it holds the full URL, not just a base.
🤖 Prompt for AI Agents
In services/painting-process-data-simulator-service/app/config/settings.py
around lines 16-18, the setting name backend_service_url is misleading because
it contains the full endpoint path; rename it to backend_endpoint_url and keep
the default value "http://localhost:8087/equipment-data". Update every
usage/import across the codebase (including services/backend_client.py, env/CI
config, tests, and docs) to reference settings.backend_endpoint_url, and update
any environment variable names or README references accordingly to avoid
breaking runtime configuration.
| # Validators | ||
| @field_validator("scheduler_interval_seconds") | ||
| @classmethod | ||
| def _positive_interval(cls, v: int) -> int: | ||
| if v <= 0: | ||
| raise ValueError("scheduler_interval_seconds must be > 0") | ||
| return v | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add Azure container name validation to fail fast
A malformed container name will cause runtime errors with Azure. Validate per Azure rules (3–63 chars; lowercase letters, numbers, hyphens; no leading/trailing hyphen; no consecutive hyphens).
Add a validator (and import re) alongside the existing validators:
import re
@field_validator("azure_container_name")
@classmethod
def _valid_container_name(cls, v: str) -> str:
# must be 3-63 chars, lowercase letters/numbers/hyphens, no leading/trailing hyphen, no consecutive hyphens
if not re.match(r'^(?!-)(?!.*--)[a-z0-9-]{3,63}(?<!-)$', v):
raise ValueError("azure_container_name must be 3-63 chars, use lowercase letters/numbers/hyphens, no leading/trailing '-' or consecutive '-'")
return vOptionally, alias the connection string to the common Azure env name for convenience:
# Example (requires pydantic Field):
# azure_connection_string: str = Field(validation_alias="AZURE_STORAGE_CONNECTION_STRING")🤖 Prompt for AI Agents
In services/painting-process-data-simulator-service/app/config/settings.py
around lines 25 to 32, there is no validation for azure_container_name which can
lead to runtime Azure errors; add an import for re at the top and implement a
field_validator for "azure_container_name" that uses a regex to enforce 3–63
characters, only lowercase letters/numbers/hyphens, no leading/trailing hyphen
and no consecutive hyphens (raise ValueError with a clear message on failure),
and optionally add a Field with
validation_alias="AZURE_STORAGE_CONNECTION_STRING" for the connection string if
you want the env var alias.
래빗코드가 리팩토링한 코드 중 시뮬레이터의 backend_client.py 파일에서 httpx 라이브러리를 호출할 때,
{predictions: [result]} 으로 출력하지 않고 시뮬레이터처럼 단순 json로 출력되게 수정
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/painting-process-equipment-defect-detection-model-service/app/routers/predict.py (1)
25-37: Offload blocking inference to a threadpool to avoid starving the event loop
run_analysislooks CPU/blocking (model inference). Calling it directly in an async route will block the event loop and degrade throughput. FastAPI recommendsrun_in_threadpoolfor this case.Apply:
@@ - try: - # run_analysis 함수를 호출하여 분석을 수행합니다. - result = run_analysis(input_data, model, explainer, config) + try: + # run_analysis 함수를 호출하여 분석을 수행합니다. + result = await run_in_threadpool(run_analysis, input_data, model, explainer, config)Add this import once:
from starlette.concurrency import run_in_threadpool
🧹 Nitpick comments (5)
services/painting-process-equipment-defect-detection-model-service/app/routers/predict.py (5)
114-119: Avoid blocking the event loop while iterating rows — use run_in_threadpool per invocationRow-by-row synchronous calls inside an async endpoint will block the loop for large files. Offloading to the threadpool keeps the server responsive.
- result = run_analysis(input_data, model, explainer, config) + result = await run_in_threadpool(run_analysis, input_data, model, explainer, config)
62-67: Case-insensitive CSV extension checkMinor: accept uppercase extensions.
- if not file.filename.endswith('.csv'): + if not file.filename.lower().endswith('.csv'):
99-103: Guard against header/row length mismatches to avoid IndexErrorUsing
enumerate(row)withmapped_header[i]will raise if a row has fewer columns than the header. Usezipand optionally log a warning.- row_data = {mapped_header[i]: val for i, val in enumerate(row)} + row_data = {col: val for col, val in zip(mapped_header, row)} + if len(row) != len(mapped_header): + logger.warning(f"{index}행: 헤더/행 길이 불일치. header={len(mapped_header)}, row={len(row)}")
33-37: Replace print statements with structured loggingUse the centralized logger for consistency and observability. Map severity to context (error/exception for failures, info for progress, warning for partial row failures).
Example changes:
- print(f"분석 중 오류 발생: {e}") + logger.exception("분석 중 오류 발생") - print(f"이슈 로그를 저장하는 중에 오류가 발생했습니다: {e}") + logger.warning(f"이슈 로그 저장 실패: {e}") - print(f"{file.filename} 파일 처리를 시작합니다...") + logger.info(f"{file.filename} 파일 처리를 시작합니다...") - print(f"{index}행 처리 중 HTTPException 발생: {e.detail}") + logger.warning(f"{index}행 처리 중 HTTPException 발생: {e.detail}") - print(f"{index}행 처리 중 오류 발생: {e}") + logger.warning(f"{index}행 처리 중 오류 발생: {e}") - print(f"파일 처리 실패: {e}") + logger.exception(f"파일 처리 실패: {e}")If not already configured, add at module top:
import logging logger = logging.getLogger(__name__)Also applies to: 47-47, 97-97, 123-126, 136-136
48-50: No internal callers expect the oldpredictionswrapper—response schema change is safeWe ran ripgrep across the repo for both
['predictions']andget('predictions')and found no references in Python files.
- If you have external API consumers, make sure to update your API docs or bump the version to avoid surprises.
- Optional: to surface whether logs were persisted and switch to structured logging, you can apply the following refactor in
services/painting-process-equipment-defect-detection-model-service/app/routers/predict.pyaround lines 48–50:@@ services/painting-process-equipment-defect-detection-model-service/app/routers/predict.py:48 - # 이슈가 감지되면 로그를 저장하고 결과를 반환합니다. - try: - save_issue_log(result, config) - except Exception as e: - print(f"이슈 로그를 저장하는 중에 오류가 발생했습니다: {e}") - - return result + # 이슈가 감지되면 로그를 저장하고 결과를 반환합니다. + from fastapi.responses import JSONResponse + import logging + logger = logging.getLogger(__name__) + + log_saved = True + try: + save_issue_log(result, config) + except Exception as e: + logger.warning(f"이슈 로그 저장 실패: {e}") + log_saved = False + + return JSONResponse(content=result, headers={"X-Log-Save": "1" if log_saved else "0"})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
services/painting-process-equipment-defect-detection-model-service/app/routers/predict.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
services/painting-process-equipment-defect-detection-model-service/app/routers/predict.py (1)
services/press-defect-detection-model-service/app/routers/predict.py (1)
predict_inspection_batch(132-166)
spring boot 포트를 8091로 변경한것을 반영
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
services/painting-process-data-simulator-service/.env.example (5)
8-16: Clarify Azure auth and add a concrete example (keep secrets out of VCS).Recommend adding a commented example connection string for quick local setup, and clarifying that non-local environments should prefer managed identity or AAD where possible. This improves DX without risking secret leakage.
Apply this non-breaking doc-only enhancement:
# Azure Storage 계정의 연결 문자열 (필수) AZURE_CONNECTION_STRING= +# 예시(로컬 개발용, 실제 키는 절대 커밋 금지): +# DefaultEndpointsProtocol=https;AccountName=<account>;AccountKey=<key>;EndpointSuffix=core.windows.net +# 비-로컬 환경에서는 Azure AD/Managed Identity 사용을 권장합니다. # 데이터를 읽어올 컨테이너 이름 AZURE_CONTAINER_NAME=simulator-data
34-36: Localhost will not work inside a container; provide a Docker Compose-friendly example.Using localhost from the simulator container will point to itself, not the backend. Add a commented alternative using the Compose service name to prevent common misconfig.
Proposed tweak:
-# 도커 사용시 시뮬레이터가 데이터를 전송할 Spring Boot 백엔드 서비스의 전체 URL -BACKEND_SERVICE_URL=http://localhost:8091/equipment-data +# 백엔드 서비스의 전체 URL +# - 로컬(호스트에서 백엔드 실행): localhost 사용 +BACKEND_SERVICE_URL=http://localhost:8091/equipment-data +# - Docker Compose 사용(서비스명이 backend 인 경우 예시): +# BACKEND_SERVICE_URL=http://backend:8091/equipment-dataIf your Compose file uses a different service name/port, update the commented example accordingly.
17-17: Trim extra blank lines flagged by dotenv-linter.Keep a single blank line between sections for readability; remove extras to satisfy linting.
- - +# (하나의 공백 줄만 유지) #============================== # 스케줄러 설정 #============================== @@ - - +# (하나의 공백 줄만 유지) #============================== # 백엔드 서비스 설정 #============================== - - +# (하나의 공백 줄만 유지) @@ - +# (하나의 공백 줄만 유지)Also applies to: 28-28, 33-33, 37-37
22-27: Field Validators Present for Scheduler Parameters
The
Settingsclass in
services/painting-process-data-simulator-service/app/config/settings.py
already defines@field_validatormethods to ensure:
scheduler_interval_seconds> 0batch_size> 0If you still want to guard against excessively large values, you can optionally introduce upper-bound checks (e.g. via
le=in the Field or additional validators).
1-44: Env var names align with Settings – no mismatches foundAll environment variable names in
.env.examplemap exactly to the Pydantic Settings attributes (uppercase names vs. snake_case fields), so there’s no risk of silent defaults or runtime failures due to naming mismatches.• AZURE_CONNECTION_STRING →
azure_connection_string
• AZURE_CONTAINER_NAME →azure_container_name
• PAINTING_DATA_FOLDER →painting_data_folder
• SCHEDULER_INTERVAL_SECONDS →scheduler_interval_seconds
• BATCH_SIZE →batch_size
• BACKEND_SERVICE_URL →backend_service_url
• HTTP_TIMEOUT →http_timeoutHowever, note that the default port for
backend_service_urldiffers between.env.example(:8091) andsettings.py(:8087). You may want to standardize one of these defaults for consistency.Since there are currently no tests covering loading of these vars in
painting-process-data-simulator-service, consider adding a simple test to verify that required env vars are loaded (and that invalid values are rejected by your validators).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
services/painting-process-data-simulator-service/.env.example(1 hunks)
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
services/painting-process-data-simulator-service/.env.example
[warning] 17-17: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
[warning] 28-28: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
[warning] 33-33: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
[warning] 37-37: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
🔇 Additional comments (2)
services/painting-process-data-simulator-service/.env.example (2)
1-3: Good starter header and guidance.Clear instruction to copy as .env for local development. No issues.
42-43: Timeout is sensible and scoped; LGTM.30s is a reasonable default; can be tuned per environment.
서버가 아닌 게이트웨이와 연결
1 participant
✨ Description
✅ Task
✅ Test Result
📸 Screenshot
📌 Etc
Summary by CodeRabbit
New Features
Changes
Documentation
Tests
Chores