-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
책을 검색한다. #83
책을 검색한다. #83
Conversation
Query Parameter
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!
clients/build.gradle
Outdated
implementation 'io.github.openfeign:feign-hc5' | ||
implementation 'io.github.openfeign:feign-micrometer' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 의존성들은 어디에서 사용되는지 궁금합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
엇 의존성 찾아보면서 추가하다가 지워야되는걸 까먹었네요 😅
그런김에 찾아봤어요!
1. Apache HttpClient 5
implementation 'io.github.openfeign:feign-hc5'
이 의존성은 Feign에서 Apache HttpClient 5를 사용하는 의존성인거 같아요.
관련해서 찾아보니 토스에 이런 글이 있더라구요 (https://toss.tech/article/engineering-note-3)
요약하자면, Feign 내부적으로 기본 HTTP Client로 HttpURLConnection
을 사용하고 있어 동시성 문제가 발생할 수 있다고 하는데, JDK17 버전을 쓰고있는 상황에서는 이 문제가 해결되어서 괜찮다고 하네요 !
2. Micrometer
implementation 'io.github.openfeign:feign-micrometer'
이 의존성은 Feign 클라이언트의 메트릭을 수집하고 모니터링을 쉽게 적용할 수 있게 해주는 의존성이라고 해요.
Spring Boot Actuator와 함께 사용할 경우에 자동으로 애플리케이션 메트릭을 수집하고 보여준다고 하는데, 필요한 시점에 도입하는게 좋아보기인 합니다
두 의존성 모두 필요한 시점에 도입하는게 좋을 것 같아요! 의존성 제외하도록 하겠습니다 ~
String publisher, | ||
List<String> authors, | ||
String thumbnail, | ||
OffsetDateTime datetime, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저희 Time도 필요한가요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 말하고자 한 것은 OffsetDateTime 클래스가 LocalDateTime에 Offset을 추가한 것인데, 여기서 LocalDate는 필요하지만 LocalTime도 필요한지가 궁금하다는 의미였습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 KakaoSearchResponseDTO 자체는 단순히 카카오에서 넘겨주는 정보들을 매핑하는 형태의 DTO로서 클라이언트에게 응답할 때는 api 모듈의 SearchBookResponse 에서 필요한 값과 형태로만 빼오는 방식으로 구성을 했어요
[카카오 응답] "datetime": "2014-11-17T00:00:00.000+09:00",
만약, 요구사항이 변경되어서 필요한 값이 추가된다면 단순히 api 모듈 내의 Reponse 객체만 수정하면 되도록 구성하는게 더 낫지 않을까? 라고 생각을 했습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 확인했습니다!
연관된 이슈
resolves #16
작업 내용
리뷰 요구사항
현재 상태는 단순 호출만 가능한 상태입니다.