Add fetch instance vnc details api in civigo library #213
Conversation
|
@alessandroargentieri @uzaxirr Please review this PR 👀 |
instance.go
Outdated
| @@ -380,3 +391,15 @@ func (c *Client) SetInstanceFirewall(id, firewallID string) (*SimpleResponse, er | |||
| response, err := c.DecodeSimpleResponse(resp) | |||
| return response, err | |||
| } | |||
|
|
|||
| // GetInstanceVNCDetails gets the VNC details for an instance | |||
| func (c *Client) GetInstanceVNCDetails(id string) (*InstanceVNCDetails, error) { | |||
There was a problem hiding this comment.
May we name it like InitInstanceVNC?
Because it's a PUT and not really a get
There was a problem hiding this comment.
Makes more sense now.
I was intrigued by the naming as it felt like it's an API that gives you the VNC details. (A fetch API hence the naming as GET)
But I guess in backend, it initiates -> then gives back the result I guess.
I will make these changes.
| resp, err := c.SendPutRequest(fmt.Sprintf("/v2/instances/%s/vnc", id), "") | ||
| if err != nil { | ||
| return nil, decodeError(err) | ||
| } |
There was a problem hiding this comment.
did you tested it locally without mocking the API?
I don't think it will work rather it would throw that instance not found in this region error i assume
|
I tested your code locally by pulling up your branch and building the SDK locally, and as i said it was not working and threw an Bad Request error, Refer to the screenshot below. 
This happened bcz you didn't passed the region in request params. Thus the default region will always be considered as NYC1 even if the SDK Client is initialised with a different region. I have raised a PR with correct code, please refer to this: https://github.com/civo/civogo/pull/214/files#diff-f4581317ccb65231765f0fa1aec4aa2bb1c08b09fe9dc1faf095d9039483e4fdR273-R276 |
|
@dipu989 if you are keen on contributing you can take a look at: |
|
@uzaxirr I only relied UTs for the test. |
Ticket link
Description -
This PR aims to introduce an API for fetching the VNC details of an instance. This API will be used internally in the command: civo instance vnc INSTANCE-ID/NAME, which will be implemented in future PRs.
Please note:
This is the first part of a broader workflow. I have submitted this PR to gather incremental feedback as I continue to work on the next phase, which involves integrating the command into the CLI tool.