feat: add active-active subscription and region models, and fetch methods#260
feat: add active-active subscription and region models, and fetch methods#260tstoykov-redis wants to merge 1 commit intomainfrom
Conversation
|
|
🛡️ Jit Security Scan Results
✅ No security findings were detected in this PR
Security scan by Jit
|
|
You'll need to bump the go toolchain in order to pass the vuln check. Check my PR for an example, it's one of the commits |
burythehammer
left a comment
burythehammer
left a comment
There was a problem hiding this comment.
Some small comments. Implement them if you wish. Nothing is a blocker or even a bug, except perhaps the nil pointer issue
| } | ||
| var activeActiveSubscriptions []*ActiveActiveSubscription | ||
| for _, sub := range response.Subscriptions { | ||
| if *sub.DeploymentType == "active-active" { |
There was a problem hiding this comment.
you'll probably want to do a nilcheck on DeploymentType before referencing it
if sub.DeploymentType != nil && *sub.DeploymentType == SubscriptionDeploymentTypeActiveActive {
| // ListActiveActive will list all of the current account's active-active subscriptions. | ||
| func (a *API) ListActiveActive(ctx context.Context) ([]*ActiveActiveSubscription, error) { | ||
| var response listActiveActiveSubscriptionResponse | ||
| err := a.client.Get(ctx, "list subscriptions", "/subscriptions", &response) |
There was a problem hiding this comment.
should change the comment to "list active active subscriptions" to be explicit
| // GetActiveActive will retrieve an existing active-active subscription. | ||
| func (a *API) GetActiveActive(ctx context.Context, id int) (*ActiveActiveSubscription, error) { | ||
| var response ActiveActiveSubscription | ||
| err := a.client.Get(ctx, fmt.Sprintf("retrieve subscription %d", id), fmt.Sprintf("/subscriptions/%d", id), &response) |
There was a problem hiding this comment.
same issue here - "retrieve active-active subscription"
| DeploymentCIDR *string `json:"deploymentCIDR,omitempty"` | ||
| DryRun *bool `json:"dryRun,omitempty"` | ||
| RespVersion *string `json:"respVersion,omitempty"` | ||
| VpcId *string `json:"vpcId,omitempty"` |
There was a problem hiding this comment.
just noticed with this PR that we're being inconsistent across the codebase with how we capitalise ID in the various fields. No change necessarily needed with this PR but it might be nice to go across the codebase, pick a convention, and stick to it (in future)
3 participants
No description provided.