Skip to content

Conversation

@wibaek
Copy link
Member

@wibaek wibaek commented Dec 14, 2024

작업 내용

도메인 전환에 따라 하드코딩 되어 있는 url을 수정했습니다. 이후 수정이 필요해 보입니다.

Nginx를 도커로 올리지 않고 서버에서 상시로 작동하게 변경했습니다. 이에 따라 미사용 nginx.conf 파일을 docs로 이동했고 workflow, docker-compose.yml을 수정했습니다.

workflow의 docker-compose -> docker compose로 명령이 변경되어 최신화 했습니다.

특이 사항

리뷰 요구사항 (선택)

@wibaek wibaek self-assigned this Dec 14, 2024
@nayonsoso
Copy link
Collaborator

nayonsoso commented Dec 14, 2024

몇가지 궁금한게 있습니다😊

1/ 엔진엑스를 도커를 사용하지 않고 띄우도록 바꾸신 이유가 있나요?
엔진엑스를 사용한 무중단 배포를 할 때도 도커를 사용할 수 있는 걸로 압니다.

2/

도메인 전환에 따라 하드코딩 되어 있는 url을 수정했습니다. 이후 수정이 필요해 보입니다.

동의합니다!
application.yml 에 선언하고, 주입받아 사용하면 될 것 같습니다.
그리고 환경변수를 @Value로 주입받는게 아니라 @ConfigurationProperties 를 사용해서 주입하는게 더 좋아보입니다. 참고
혹시 이 부분 변경하실 때, 따로 PR 파서 @ConfigurationProperties가 코드 전체적으로 적용되도록 해주실 수 있나요?🥹

3/

workflow의 docker-compose -> docker compose로 명령이 변경되어 최신화 했습니다

이것도 잘 변경하신 것 같습니다👍

@wibaek
Copy link
Member Author

wibaek commented Dec 14, 2024

몇가지 궁금한게 있습니다😊

1/ 엔진엑스를 도커를 사용하지 않고 띄우도록 바꾸신 이유가 있나요? 엔진엑스를 사용한 무중단 배포를 할 때도 도커를 사용할 수 있는 걸로 압니다.

제가 생각하는 Docker의 주요 장점은 환경과 무관하게 컨테이너의 독립성을 확보할 수 있다는 점입니다. 그러나 Nginx는 SSL 인증서와 같은 파일 시스템 리소스에 직접 접근해야 하기 때문에, 이 경우 Docker를 사용하는 이점이 줄어든다고 느꼈습니다. Docker 볼륨을 통해 파일 시스템 접근이 가능하긴 하지만, 위치가 변경될 수도 있고 추가적인 설정이 필요해 종속성이 완전히 제거되지는 않는다고 느꼈습니다.

그리고 무중단 배포를 진행한다면 Nginx는 계속 앞단의 로드밸런싱을 해줘야하기 때문에 중단될 수 없는데, 이런 상황에서는 도커의 빠른 재시작 및 컨테이너 교체와 같은 주요 장점이 크게 발휘되지 않는다고 생각했습니다.

또한, SSL 인증서 자동 갱신을 설정할 때 Nginx가 서버에 직접 설치되어 있는 경우 관리가 더 간단하고 설정 작업이 줄어들어 선택했습니다.

Copy link
Collaborator

@nayonsoso nayonsoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

말씀해주신 근거가 타당하다 생각해 approve 합니다😊

추가로, 위에 제안드린 configurationProperties에 대해서 어떻게 하고 싶으신지 알려주심 감사하겠습니다!

@wibaek
Copy link
Member Author

wibaek commented Dec 14, 2024

말씀해주신 근거가 타당하다 생각해 approve 합니다😊

추가로, 위에 제안드린 configurationProperties에 대해서 어떻게 하고 싶으신지 알려주심 감사하겠습니다!

cors allowed origins을 yml로 분리하는 건도 완료했습니다!

미리 말을 드리고 작업했어야 했는데 늦어서 죄송합니다!

@nayonsoso
Copy link
Collaborator

WebConfig 파일의 todo 도 지우면 더 완벽할 것 같습니다!

@wibaek
Copy link
Member Author

wibaek commented Dec 15, 2024

WebConfig 파일의 todo 도 지우면 더 완벽할 것 같습니다!

반영했습니다! 머지 하겠습니다!

@wibaek wibaek merged commit c0426c9 into solid-connection:main Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants