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

refactor: unnecessary API call in instance reconciliation #386

Merged
merged 2 commits into from
Mar 13, 2023

Conversation

apricote
Copy link
Member

@apricote apricote commented Mar 9, 2023

Calls to InstanceV2.InstanceMetadata() caused 2 API calls, one to retrieve the server and the second to check if the configured network exists.

The second call is not necessary because we already know all networks that the server is attached to exist. By just comparing the nameOrID locally we can achieve the same result.

Related to #384

@apricote apricote added the enhancement New feature or request label Mar 9, 2023
@apricote apricote self-assigned this Mar 9, 2023
@apricote apricote requested a review from a team as a code owner March 9, 2023 12:53
Copy link
Contributor

@samcday samcday left a comment

Choose a reason for hiding this comment

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

gr8! 👍

@apricote
Copy link
Member Author

The network name is not being returned in the GET v1/servers/:id call, so I had to pass the network ID from newCloud(). This is cleaner anyway, because now we read the environment variable only in one location.

@apricote apricote requested a review from samcday March 13, 2023 15:11
Calls to InstanceV2.InstanceMetadata() caused 2 API calls, one
to retrieve the server and the second to check if the configured
network exists.

The second call is not necessary because we already know all
networks that the server is attached to exist. By just comparing
the nameOrID locally we can achieve the same result.
nodeAddresses does no longer require access to the HCloud API, and
we can move it to a standalone function. This simplifies the tests
and improves their performance as we no longer need the test env.
@apricote apricote merged commit 3e55a01 into main Mar 13, 2023
@apricote apricote deleted the network-api-call branch March 13, 2023 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants