[FEAT] elasticsearch에도 변경된 경매 상태 반영#12
Open
imprity wants to merge 1 commit into
Open
Conversation
WalkthroughAuctionSqsLambda에 Elasticsearch 경매 상태 동기화 기능 추가. 환경변수 기반으로 HttpClient와 Basic Auth를 설정한 후, 경매의 START/END 처리 시점에 경매 상태를 Elasticsearch /_update 엔드포인트로 POST 전송하고 응답/예외 로깅. ChangesElasticsearch 경매 상태 동기화
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/com/example/lambda/AuctionSqsLambda.java (2)
318-326: 💤 Low valueInterruptedException 처리 개선 고려
httpClient.send()는InterruptedException을 던질 수 있습니다. 현재 코드에서는 이를 일반Exception으로 catch하여 로그만 남기고 있는데, 인터럽트 상태를 복원하지 않으면 상위 호출자가 인터럽트를 감지할 수 없습니다.Lambda 환경에서는 심각한 문제가 아니지만, 베스트 프랙티스를 위해 고려할 수 있습니다.
♻️ InterruptedException 처리 제안
- } catch (Exception e) { + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + context.getLogger().log("[ES] status update 인터럽트: auctionId=%d, %s".formatted( + auctionId, e.getMessage())); + } catch (Exception e) { context.getLogger().log("[ES] status update 예외: auctionId=%d, %s".formatted( auctionId, e.getMessage())); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/com/example/lambda/AuctionSqsLambda.java` around lines 318 - 326, The catch-all Exception block around the httpClient.send call swallows InterruptedException and doesn't restore the thread's interrupt status; update AuctionSqsLambda's httpClient.send handling by adding a specific catch (InterruptedException ie) that logs the interruption (using context.getLogger().log with auctionId) and then calls Thread.currentThread().interrupt() before exiting/returning, and keep the existing catch (Exception e) for other exceptions so normal errors are still logged via the same pattern.
50-60: 💤 Low value환경 변수 null 체크 고려
ES_USERNAME또는ES_PASSWORD가 설정되지 않은 경우System.getenv()가null을 반환하고, Java에서null + ":"는"null:"문자열이 됩니다. 이로 인해 잘못된 Basic Auth 헤더가 생성되어 ES에서 401 오류가 발생합니다.배포 시 환경 변수 누락을 조기에 감지하려면 null 체크를 추가하는 것이 좋습니다.
🛡️ 방어적 null 체크 제안
private static final String ES_URL = System.getenv("ELASTICSEARCH_URIS"); private static final String ES_AUCTION_INDEX = System.getenv("ELASTICSEARCH_AUCTION_INDEX"); private static final String ES_USERNAME = System.getenv("ELASTICSEARCH_USERNAME"); private static final String ES_PASSWORD = System.getenv("ELASTICSEARCH_PASSWORD"); - private static final String ES_BASIC_AUTH = "Basic " + java.util.Base64.getEncoder() - .encodeToString((ES_USERNAME + ":" + ES_PASSWORD).getBytes(java.nio.charset.StandardCharsets.UTF_8)); + private static final String ES_BASIC_AUTH; + static { + if (ES_URL == null || ES_AUCTION_INDEX == null || ES_USERNAME == null || ES_PASSWORD == null) { + throw new IllegalStateException("Elasticsearch 환경 변수가 설정되지 않았습니다."); + } + ES_BASIC_AUTH = "Basic " + java.util.Base64.getEncoder() + .encodeToString((ES_USERNAME + ":" + ES_PASSWORD).getBytes(java.nio.charset.StandardCharsets.UTF_8)); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/com/example/lambda/AuctionSqsLambda.java` around lines 50 - 60, The static initialization currently builds ES_BASIC_AUTH from ES_USERNAME and ES_PASSWORD without null checks; add defensive validation in the AuctionSqsLambda static init (or a small helper like validateEnvVar) to assert ES_USERNAME and ES_PASSWORD are non-null/non-empty and if they are missing throw an IllegalStateException (or log and abort startup) with a clear message naming the missing env var(s); then compute ES_BASIC_AUTH using the validated values so you never encode "null:password" or "username:null".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/com/example/lambda/AuctionSqsLambda.java`:
- Around line 318-326: The catch-all Exception block around the httpClient.send
call swallows InterruptedException and doesn't restore the thread's interrupt
status; update AuctionSqsLambda's httpClient.send handling by adding a specific
catch (InterruptedException ie) that logs the interruption (using
context.getLogger().log with auctionId) and then calls
Thread.currentThread().interrupt() before exiting/returning, and keep the
existing catch (Exception e) for other exceptions so normal errors are still
logged via the same pattern.
- Around line 50-60: The static initialization currently builds ES_BASIC_AUTH
from ES_USERNAME and ES_PASSWORD without null checks; add defensive validation
in the AuctionSqsLambda static init (or a small helper like validateEnvVar) to
assert ES_USERNAME and ES_PASSWORD are non-null/non-empty and if they are
missing throw an IllegalStateException (or log and abort startup) with a clear
message naming the missing env var(s); then compute ES_BASIC_AUTH using the
validated values so you never encode "null:password" or "username:null".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eca4bcd3-b4c5-43a4-bd26-7513e1e56b5e
📒 Files selected for processing (1)
src/com/example/lambda/AuctionSqsLambda.java
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.
📝 작업 내용
경매 상태가 변경될 때 elasticsearch에도 변경된 경매 상태를 반영하도록 수정
✅ 체크리스트 (Checklist)
💬 기타 사항
제대로된 테스트를 진행하지는 못했지만 elasticsearch에 http 요청을 보내는 부분만 떼어내어 실행해 본 결과 정상 작동함을 확인하였습니다.
Summary by CodeRabbit
변경 사항