9조 BE 코드리뷰 1회차 #10
9조 BE 코드리뷰 1회차 #10
Conversation
…erfile Dockerfile 작성
Dockerfile 작성
Revert "Dockerfile 작성"
| compileOnly 'org.projectlombok:lombok' | ||
| runtimeOnly 'com.mysql:mysql-connector-j' | ||
| annotationProcessor 'org.projectlombok:lombok' | ||
| runtimeOnly 'com.h2database:h2' |
There was a problem hiding this comment.
h2db는 테스트 목적으로 사용하시는거면 testImplementation 혹은 testRuntimeOnly로도 사용할수 있을것 같아요
| - name: Create application.yaml | ||
| run: | | ||
| mkdir -p src/main/resources | ||
| echo "${{ secrets.APPLICATION_YAML }}" > src/main/resources/application.yaml |
There was a problem hiding this comment.
보안으로 인해 코드레벨에서 관리할 수 없는 설정을 빌드할때 붙여넣는 코드일까요?
이렇게 해도 좋지만, 아래와 같은 방식으로 별도 파일로 관리해도 좋을것 같아요.
파일명을 application-secret.yaml 요런식으로 설정하고,
spring.config.additional-location=파일위치
spring.profiles.include=secret
이렇게 하면 별도 파일로 애플리케이션 설정을 관리할 수 있을것 같습니다.
| no-cache: true # 이미지 빌드 시 캐시 사용 여부 | ||
|
|
||
| # EC2 인스턴스에 Docker 이미지 배포 | ||
| - name: Deploy to AWS EC2 |
There was a problem hiding this comment.
간단하게 CI/CD를 잘 작성해주신것 같아요.
하지만 해당 스크립트로는 무중단 배포가 어렵겠네요.
아직 시간은 많이 남았으니, 앞으로는 이런 부분들에 대해서 고민해보면 어떨까요?
- 서버를 N대로 확장할 수 있어야한다.
- 배포시에, N대의 서버에 순차적으로 이미지가 업데이트 되어야한다.
- 무중단으로 배포되어야 한다.
- 트래픽이 서버 N대에 Round Robin 되어야한다.
- 배포에 문제가 생겼을시 이전 이미지 버전으로 rollback이 가능해야한다.
현재는 시작하지 얼마되지 않은 개발단계이기 때문에, 지금 수준으로도 충분하지만, 남은시간동안에는 위 부분에 대해서도 고려해보면 좋을것 같아요!!
고생하셨습니다 👍👍
| @@ -0,0 +1,39 @@ | |||
| plugins { | |||
There was a problem hiding this comment.
별건 아니지만 제 취향이긴 한데요, 저는 version과 관련된건 변수로 빼둬서 관리를 하는걸 선호해서
제가 작성하는 방식에 대해서 잠깐 알려드리면 아래처럼 사용하면 version을 변수로 사용하실 수 있어요!
buildscript {
ext {
spring_boot_version = '3.3.3'
spring_dependency_management = '1.1.6'
}
repositories {
mavenCentral()
}
}
plugins {
id 'java'
id 'org.springframework.boot' version "${spring_boot_version}"
id 'io.spring.dependency-management' version "${spring_dependency_management}"
}
| package com.helpmeCookies.product.entity; | ||
|
|
||
| public enum HashTag { | ||
| autumn("가을"), winter("겨울"); |
There was a problem hiding this comment.
ENUM은 대문자로 관리해주시는건 어떨까요? Category도 대문자이니, 한가지 방식으로 통일해주시면 좋을것 같아요!
ex)
AUTUMN,
WINTER
| import jakarta.persistence.ManyToOne; | ||
| import jakarta.persistence.OneToMany; | ||
|
|
||
| @Entity |
There was a problem hiding this comment.
요기도 다른 코드와 통일성있게 @Table 붙여주시면 좋을것 같아요~
|
|
||
| @ManyToOne | ||
| @JoinColumn(name = "user_id") | ||
| private User user; |
There was a problem hiding this comment.
Jpa를 사용할때 개인적으로 중요하다고 생각하는 지점중 하나는 객체 탐색을 명시적으로 막아야할때가 있다는점인것 같아요.
관련이 있는 Table에 대해서 객체 탐색(Lazy Loading)을 통해 데이터를 조회해오는 과정은 너무나 편리하지만, 그만큼 위험이 존재할 수 도 있는것 같아요.
객체 탐색을 막기 위해서는 User로 매핑하지않고, Long(userId)으로 매핑해서, 객체 탐색을 막는법이 있습니다. 이렇게 되면 유저의 데이터가 필요하다면, UserRepository 같은걸 통해서 데이터를 조회해야할것 같아요.
| private Long id; | ||
|
|
||
| @Column(nullable = false) | ||
| private Long totalFollowers; |
There was a problem hiding this comment.
totalFollwers, totalLikes는 데이터 정합성을 맞추기 위해서 조금의 노력이 필요할것 같아요.
비즈니스적으로 데이터 정합성이 정확하게 일치될 필요가 있는지에 대해 고민해주시면 좋을것 같고, 동시성과 관련되서도 고민해보면 좋을것 같아요.
| private String businessNumber; | ||
|
|
||
| @Column(nullable = false) | ||
| private String openDate; |
|
머지하려고 보니, CI가 실패했네요~ 이거 통과시켜주시면 머지해두겠습니다! |
…erfile 도커 파일 작성
|
안녕하세요 멘토님 @leaf-upper CI/CD 통과하도록 수정했습니다. CI/CD 관련해서 말씀해주신 부분들 고려해보겠습니다. 감사합니다! |
PR
✨ 작업 내용
✨ 참고 사항
⏰ 현재 버그
✏ Git Close