fix(kafka create): sync marketplace provider with cloud provider #1792
Conversation
pkg/cmd/kafka/create/create.go
Outdated
| return nil, errors.New("standard instances are unavailable for the cloud provider, try another provider") | ||
| } | ||
|
|
||
| fmt.Println("Supported Billing Models - ", availableBillingModels) |
There was a problem hiding this comment.
Debugging print statements 🙃
pkg/cmd/kafka/create/create.go
Outdated
| availableBillingModels := FetchSupportedBillingModels(orgQuota, answers.CloudProvider) | ||
|
|
||
| if len(availableBillingModels) == 0 && len(orgQuota.MarketplaceQuotas) > 0 { | ||
| return nil, errors.New("standard instances are unavailable for the cloud provider, try another provider") |
There was a problem hiding this comment.
These error messages should be added to our lang files right? Also one line 263
pkg/shared/accountmgmtutil/ams.go
Outdated
| } | ||
|
|
||
| if len(orgQuota.MarketplaceQuotas) == 0 { | ||
| return nil, errors.New("no marketplace quota available for given provider") |
pkg/shared/accountmgmtutil/ams.go
Outdated
| func uniqueQuota(s []QuotaSpec) []QuotaSpec { | ||
| inResult := make(map[QuotaSpec]bool) | ||
| var result []QuotaSpec | ||
| for _, quota := range s { | ||
| if _, ok := inResult[quota]; !ok { | ||
| inResult[quota] = true | ||
| result = append(result, quota) | ||
| } | ||
| } | ||
| return result | ||
| } |
There was a problem hiding this comment.
This is a small thing but I had to read this a few times to figure out what was happening (we are removing all duplicates from the slice). Maybe just giving this a rename would make it clearer for code reading in the future.
There was a problem hiding this comment.
Something like, removeDuplicateQuotaSpec maybe?
There was a problem hiding this comment.
Having unique as prefix makes more sense imo, I have added an inline description for the method. Can you check it and let me know wdyt?
There was a problem hiding this comment.
Yeah, the comment is fine, I tend to rather document with the naming of things then comments as they can be outdated and not read most of the time.
I have a small rule, if my comment isnt 3 or more lines long it probably means the code can be made clearer without it (or there is something very niche happening that is just difficult so its fine).
But that is just how I go about it so this is fine 👍
Selection of marketplace provider should be dependent on the cloud provider.
Verification Steps
gcpas marketplace whenawsis chosen as the cloud provider:Type of change