search

G-GCELL / api-server

0 stars 0 forks source link

Feat : 2, 3주차 기능 개발 - [merged] #91

Closed TaeYeongKwak closed 1 year ago

TaeYeongKwak commented 1 year ago

Merges dev -> main

1. 엑셀 파일 삭제 기능

2. 엑셀 파일 검색 기능

3. 엑셀 파일 생성 진행률 전송 기능

4. API 서버 로그 출력 기능

5. 엑셀 파일 생성 기능

6. 메시지 브로커 리팩토링

Issues

18

23

25

26

27

28

30

31

32

34

35

36

38

39

Bugs

29

37

@mentor/mentor_2023.01

TaeYeongKwak commented 1 year ago

In GitLab by @Charlie on Feb 27, 2023, 09:50

Commented on src/main/java/com/gabia/weat/gcellapiserver/dto/MessageDto.java line 11

API 서버와 Excel 서버 모두 공용으로 사용하는 DTO 가 될 것 같은데요. API 서버와 엑셀 서버 간 통신에서 핵심적인 부분이기 때문에 서로 어긋나지 않도록 관리하는 게 중요합니다.

DTO 뿐만 아니라 Repository 나 Entity, 비지니스 로직 등 다른 공통적인 부분들도 마찬가지로요.

지금은 API 서버, Excel 서버 각각 이들을 관리하고 있지만, 만일 이 공통적인 코드에 변화가 있으면 양쪽 다 변경해줘야하고 호환이 잘 맞도록 개발자가 주의해서 개발해야 하는 문제가 있을 것 같은데요.

이런 문제를 개선할 수 있는 방법은 어떤 게 있을까요?

TaeYeongKwak commented 1 year ago

In GitLab by @Kai on Feb 27, 2023, 12:52

Commented on src/main/java/com/gabia/weat/gcellapiserver/dto/log/LogFormatFactory.java line 16

traceId 를 부여했다는 것은 분산 서비스 간 트랜잭션 추적을 고려해서 부여한 것일텐데 server.name 을 쓰는 것도 좋지만 Spring Cloud 에서 보통 이러한 부분은 spring.application.name 을 주로 이용합니다.

나중에 범용적으로 Spring Cloud 관련 라이브러리를 사용할 수도 있으니 한번 알아보시는 것도 좋을 것 같습니다.

TaeYeongKwak commented 1 year ago

In GitLab by @Kai on Feb 27, 2023, 12:52

Commented on src/main/java/com/gabia/weat/gcellapiserver/dto/log/ErrorLogFormatDto.java line 33

이 부분을 볼 때마다 항상 약간의 아쉬움이 남는 것 같습니다. logback 을 활용해서 공통적인 부분은 뺄 수 있지 않을까 하고 말이죠. 결국 다른 코드들을 보면 traceId 를 항상 넘기게 되어 있는 것을 확인했습니다.

spring 에서 제공하는 logback appender 에 더해서 출력을 하고 싶다면 spring 에서 어떻게 logback 을 만들어주는지, 그리고 어떻게 하면 내용을 바꾸어서 보여줄 수 있을지 찾아보세요. 이를 응용한 것 중에 하나가 sleuth 라이브러리입니다.

제 생각엔 이런 공통적으로 보여줄 수 있는 부분은 logback 으로 해결할 수 있을 것 같아 보입니다. 요구사항은 이미 만족한 것으로 보이니 남는 시간에 키워드로 MDC, EnvironmentPostProcessor, DefaultLogbackConfiguration 으로 알아보시면 될 것 같습니다.

그리고 예전에 sleuth 에서는 이런 구현이 어렵다고 들었는데 어떤 점이 어려웠던 것인지 들어보고 싶네요.

TaeYeongKwak commented 1 year ago

찾아보니 멀티 모듈이라는 방식이 있는 것 같아 해당 내용을 공부해본 후 적용시켜보도록 하겠습니다.

TaeYeongKwak commented 1 year ago

네 해당 키워드에 대해서 공부한 후 적용시켜보도록 하겠습니다.

sleuth를 사용하지 않은 이유는 처음에 sleuth를 적용시켜보려고 했을 때 traceId가 제대로 생성되지 않는 이슈가 있었습니다. 그래서 개인적인 판단으로 spring cloud가 아닌 spring boot 환경에서는 제대로 적용되지 않는다고 생각하여 직접 구현하게 되었습니다. 해당 부분은 제가 잘못 판단한 것 같아서 다시 한번 확인해보도록 하겠습니다.

TaeYeongKwak commented 1 year ago

In GitLab by @Charlie on Feb 27, 2023, 13:23

Commented on src/main/java/com/gabia/weat/gcellapiserver/dto/MessageDto.java line 11

이번에 적용하실 필욘 없습니다.

이와 같은 이슈를 개선하기 위한 방법은 여러가지가 있을테니, 관련 내용만 정리하시고 나중에 시간 있을 때 적용해보시는 게 어떨까 싶네요.

TaeYeongKwak commented 1 year ago

네 일단 내용 정리만 하고 후에 시간이 남는다면 적용해보도록 하겠습니다.

TaeYeongKwak commented 1 year ago

In GitLab by @Charlie on Feb 27, 2023, 13:38

Commented on src/main/java/com/gabia/weat/gcellapiserver/dto/MessageDto.java line 11

~CHECK

TaeYeongKwak commented 1 year ago

In GitLab by @Charlie on Feb 27, 2023, 13:38

approved this merge request

TaeYeongKwak commented 1 year ago

In GitLab by @Kai on Feb 27, 2023, 13:41

approved this merge request

TaeYeongKwak commented 1 year ago

In GitLab by @Pablo on Feb 27, 2023, 14:11

Commented on src/main/java/com/gabia/weat/gcellapiserver/service/MinioService.java line 49

~QUESTION
minioClient에 올라간 Excel file 삭제를 위한 메서드 같은데
@Transactional 어노테이션이 필요한 이유가 있나요?

TaeYeongKwak commented 1 year ago

In GitLab by @Kai on Feb 27, 2023, 14:23

Commented on src/main/java/com/gabia/weat/gcellapiserver/config/RabbitmqProperty.java line 11

제 생각에는 관리할 부분만 따로 Property 로 빼내면 좋지 않을까 생각합니다.

RabbitAutoConfiguration 을 보면 이미 property 를 만들어주고 있는 것으로 보여집니다. 기본 설정은 설정대로 두고, 따로 하위의 property 를 추가해서 관리하고 싶은 부분만 관리하는 방법으로 진행하는 것은 어떻게 생각하시나요?

TaeYeongKwak commented 1 year ago

In GitLab by @tommysgit on Feb 27, 2023, 14:31

Commented on src/main/java/com/gabia/weat/gcellapiserver/service/MinioService.java line 49

ExcelInfoService에서 삭제하는 로직에 사용되는데 @Transactional을 붙여야 트랜잭션 전파가 되는 줄 알았으나, 다시 찾아보니 붙이지 않아도 기본적으로 전파가 된다고 하여 해당 어노테이션을 제거하여 다시 올리겠습니다!

TaeYeongKwak commented 1 year ago

In GitLab by @muyongKim on Feb 27, 2023, 15:01

Commented on src/main/java/com/gabia/weat/gcellapiserver/config/RabbitmqProperty.java line 11

추후 VM에 배포할 때 기본 설정(특히, guest/guest가 default 값인 username, password) 모두 변경이 필요할 것으로 생각하여 함께 관리하고자 AutoConfiguration에서 제공하는 property를 사용하지 않았습니다!

그러나 말씀해주신대로 port는 변경 없이 그대로 사용할 것으로 보여 해당 Property 클래스에서 제외하고 기본 설정을 사용해도 될 것 같습니다.

TaeYeongKwak commented 1 year ago

In GitLab by @muyongKim on Feb 27, 2023, 15:17

Commented on src/main/java/com/gabia/weat/gcellapiserver/dto/log/LogFormatFactory.java line 16

각자 구현 파트에서 애플리케이션 이름을 부여할 때 spring.application.name, server.name을 중복으로 사용하고 있는 걸 발견했고, 팀원끼리 이를 통일하는 과정에서 의미상으로(?) server.name이 적절하다고 판단했던 것 같습니다.

앞으로 property를 사용할 때 해당 property가 의미하는 바와 쓰임새를 한 번 더 확인해보고 사용하도록 하겠습니다!

TaeYeongKwak commented 1 year ago

In GitLab by @Kai on Feb 27, 2023, 15:19

Commented on src/main/java/com/gabia/weat/gcellapiserver/config/RabbitmqProperty.java line 11

다시 보니 Henry 가 얘기했었던 부분이 반영되었던거군요. 제가 말하는 기본설정이라는 것은 AutoConfiguration 과 함께 제공되는 RabbitProperties(spring.rabbitmq.*) 를 의미합니다.

TaeYeongKwak commented 1 year ago

In GitLab by @tommysgit on Feb 28, 2023, 09:56

Commented on src/main/java/com/gabia/weat/gcellapiserver/service/MinioService.java line 49

changed this line in version 2 of the diff

TaeYeongKwak commented 1 year ago

In GitLab by @tommysgit on Feb 28, 2023, 09:56

added 11 commits

Compare with previous version

TaeYeongKwak commented 1 year ago

In GitLab by @tommysgit on Feb 28, 2023, 09:57

Commented on src/main/java/com/gabia/weat/gcellapiserver/service/MinioService.java line 49

말씀하신 부분 반영하였습니다!

TaeYeongKwak commented 1 year ago

In GitLab by @Luke on Feb 28, 2023, 11:18

approved this merge request

TaeYeongKwak commented 1 year ago

In GitLab by @Pablo on Mar 3, 2023, 13:11

Commented on src/main/java/com/gabia/weat/gcellapiserver/service/MinioService.java line 49

~CHECK

TaeYeongKwak commented 1 year ago

In GitLab by @Pablo on Mar 3, 2023, 13:11

resolved all threads

TaeYeongKwak commented 1 year ago

In GitLab by @Pablo on Mar 3, 2023, 13:11

approved this merge request

TaeYeongKwak commented 1 year ago

mentioned in commit 2b43edbbe43077fda41811f9f5837b33d8fb63cf