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 support for the type field when creating nodebalancers #669

Merged
merged 1 commit into from
Feb 28, 2025

Conversation

eljohnson92
Copy link
Contributor

📝 Description

What does this PR do and why is this change necessary?
This PR adds support for a new type of nodebalancer called premium

✔️ How to Test

What are the steps to reproduce the issue or verify the changes?
This can be tested by sending a request for an explicit common or premium nodebalancer type
How do I run the relevant unit/integration tests?

@eljohnson92 eljohnson92 requested a review from a team as a code owner February 4, 2025 04:11
@eljohnson92 eljohnson92 requested review from ykim-akamai and yec-akamai and removed request for a team February 4, 2025 04:11
Copy link
Contributor

@rahulait rahulait left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@yec-akamai yec-akamai left a comment

Choose a reason for hiding this comment

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

@eljohnson92 Thank you for the contribution! Can you add test cases, i.e. integration test, to test through the new field?

There are examples you can find here: https://github.com/linode/linodego/tree/main/test/integration
And you can test and generate the fixtures by running make TEST_ARGS="-run $TestCaseName" fixtures

@eljohnson92
Copy link
Contributor Author

@yec-akamai I can definitely add this, the only issue is currently the Linode API does not return what the type is from a created NB so we can test that submission works but not validate it is the right type, do you think just submission is good for now?

Copy link
Contributor

@yec-akamai yec-akamai left a comment

Choose a reason for hiding this comment

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

Tests all passed on my end, nice work!

Copy link
Contributor

@ykim-akamai ykim-akamai left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the contribution

@ykim-akamai ykim-akamai merged commit 41a199a into linode:main Feb 28, 2025
10 checks passed
@zliang-akamai zliang-akamai added the new-feature for new features in the changelog. label Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature for new features in the changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants