civo / civogo

Golang client to interact with Civo's API
https://www.civo.com/
MIT License
37 stars 23 forks source link

Add fetch instance vnc details api in civigo library #213

Closed dipu989 closed 3 weeks ago

dipu989 commented 3 weeks ago

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.

dipu989 commented 3 weeks ago

@alessandroargentieri @uzaxirr Please review this PR 👀

uzaxirr commented 3 weeks ago

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.

Screenshot 2024-09-26 at 10 46 16 AM

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

uzaxirr commented 3 weeks ago

@dipu989 if you are keen on contributing you can take a look at:

dipu989 commented 3 weeks ago

@uzaxirr I only relied UTs for the test. Thanks and will note this for my reference on how to test it in local 👍 Yup, I will start with one of the above.