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

Update API docs for MCIS control #1539

Closed
wants to merge 1 commit into from

Conversation

yunkon-kim
Copy link
Member

The MCIS control APIs return a string type as a success response as follows:
image
image

The API docs for MCIS control need to be updated. (This PR should be closed if common.SimpleMsg is the correct response.)

The APIs:

  • GET /ns/{nsId}/control/mcis/{mcisId}
  • GET /ns/{nsId}/control/mcis/{mcisId}/vm/{vmId}

* Use string type for success responses of
  - `GET /ns/{nsId}/control/mcis/{mcisId}`
  - `GET /ns/{nsId}/control/mcis/{mcisId}/vm/{vmId}`
@seokho-son
Copy link
Member

@yunkon-kim 제안 감사합니다.

{object} common.SimpleMsg 로 리턴하는 것은 오류는 아니며 명확한 의도가 있습니다.
REST API를 활용하는 클라이언트(웹툴 등)의 입장에서는
리턴바디의 오브젝트 타입이 다양한 경우, 각각 맞춰서 처리하는 것이 까다롭고 코드상 일관성이 떨어진다고 합니다.

해당 SimpleMsg 는 String 타입 리턴을 JSON 오브젝트로 제공하여,
다른 API의 응답들과 마찬가지로 JSON으로 룩업할 수 있게 지원하는 사항입니다.

혹시 다른 의견이나 개선 방법이 있으실까요?

@yunkon-kim
Copy link
Member Author

@seokho-son

PR의 의도가 잘 전달 되지 않았던 것 같습니다 ^^;;

현재, MCIS 제어 API를 호출했을 때, 200 OK 인 경우 string 타입의 응답을 리턴하고 있는데요. API 문서 상에는 common.SimpleMsg로 규격이 나타나있어 이를 string 으로 정정하는 PR 이었습니다.

설명주신 내용 대로라면, string 리턴을 common.SimpleMsg로 리턴하도록 수정하는 것이 맞는 방향이겠네요?

@seokho-son
Copy link
Member

@yunkon-kim 네네! 제가 오해했습니다. 리턴이 string으로 나오고 있었네요 -_-.. 코드상 오류가 맞습니다.
마침 제가 코드를 보고 있으니, 정정 PR을 올리도록 하겠습니다. :)

@yunkon-kim
Copy link
Member Author

yunkon-kim commented May 3, 2024

@seokho-son

억.. commit을 잘못 찍었습니다 ㅠ (원래대로 force-push 했습니다. )

@seokho-son
Copy link
Member

@yunkon-kim #1542 will fix the issue. Thanks.

@seokho-son seokho-son closed this May 3, 2024
@yunkon-kim yunkon-kim deleted the 240503-11 branch June 10, 2024 02:00
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