Skip to content
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

Add deploy config and set contaner status #44

Merged
merged 5 commits into from
Sep 19, 2023

Conversation

yoojinko
Copy link
Collaborator

deploy config 추가하여 config data 로 app 생성하도록 변경하였습니다.

  • config: modelName, billingConfig, serviceUrl (test deploy용. dev 이후 사라질 수 있음.)
  • modelName == serviceName 입니다.
  • deploy 함수에서 데이터를 받아 createApp으로 넘겨줍니다.

setContainerStatus 추가하였습니다.

  • deploy 시 createApp 에서 자동으로 RUNNING 상태로 기록합니다.
  • stop 시 appController.setContainerStatus(modelName, ContainerStatus.STOP) 호출하면 됩니다.

Copy link
Collaborator

@akastercomcom akastercomcom left a comment

Choose a reason for hiding this comment

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

LGTM with some comments
Thanks!

modelName: string,
billingConfig?: appBillingConfig,
serviceUrl?: string,
Copy link
Collaborator

Choose a reason for hiding this comment

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

현재는 serviceUrl필수로 해두는게 맞는 것 같습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

저희 테스트 용으로 임시로 두었습니다. logic에는 주석 썼는데 type에 안써놨네요. 주석 추가하겠습니다.

}
}
// NOTE(yoojin): For test. We make fixed url on service.
if (!serviceUrl) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

현재로썬 저 규칙을 안따를 가능성이 높기 때문에 그냥 필수로 하는게 더 좋아 보입니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ditto

@yoojinko
Copy link
Collaborator Author

Thanks for the review!

@yoojinko yoojinko merged commit 766c2b8 into develop Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants