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

load balancers: introduce new type field #618

Merged
merged 1 commit into from
Jul 11, 2023

Conversation

asaha2
Copy link
Contributor

@asaha2 asaha2 commented Jul 10, 2023

Introduce new load balancer Type field to allow specifying the type of load balancer to be created:

  • REGIONAL: supports target droplets within the same region/vpc
  • GLOBAL: supports target droplets across multiple regions/vpcs

Omitting the Type field in the Create request implies REGIONAL by default.

@asaha2 asaha2 marked this pull request as ready for review July 10, 2023 13:29
Copy link
Member

@bentranter bentranter left a comment

Choose a reason for hiding this comment

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

For the new Type field, it looks like it can either be REGIONAL or GLOBAL.

Could you include these as constants? Something like,

const (
    LoadBalancerTypeGlobal = "GLOBAL"
    LoadBalancerTypeRegional = "REGIONAL"
)

@asaha2
Copy link
Contributor Author

asaha2 commented Jul 10, 2023

For the new Type field, it looks like it can either be REGIONAL or GLOBAL.

Could you include these as constants? Something like,

const (
    LoadBalancerTypeGlobal = "GLOBAL"
    LoadBalancerTypeRegional = "REGIONAL"
)

We allow case insensitive type parameter: hence all of REGIONAL, regional or reGIOnAL would be accepted.

@bentranter
Copy link
Member

@asaha2 It would still make it easier for users to discover and to pass one the correct load balancer types if we can provide some kind of constant – it doesn't necessarily have to be uppercase.

@asaha2 asaha2 requested a review from bentranter July 10, 2023 19:04
load_balancers.go Outdated Show resolved Hide resolved
@asaha2 asaha2 requested a review from bentranter July 11, 2023 13:03
Copy link
Member

@bentranter bentranter 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 @asaha2. If you want you can merge now, or if you want you can wait for @bbassingthwaite to give it a review.

@bentranter bentranter merged commit 43f27da into digitalocean:main Jul 11, 2023
7 checks passed
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.

None yet

3 participants