cache resource providers to reduce expensive api call #20050
Conversation
There was a problem hiding this comment.
hi @ms-henglu
Thanks for this PR - I've taken a look through and left some comments inline to make this consistent with how caching for the Locations are done, if we can make those changes so this becomes consistent then we should be able to get this merged.
Thanks!
internal/provider/provider.go
Outdated
| @@ -416,14 +416,13 @@ func providerConfigure(p *schema.Provider) schema.ConfigureContextFunc { | |||
| if !skipProviderRegistration { | |||
| // List all the available providers and their registration state to avoid unnecessary | |||
| // requests. This also lets us check if the provider credentials are correct. | |||
| providerList, err := client.Resource.ProvidersClient.List(ctx, nil, "") | |||
| availableResourceProviders, err := resourceproviders.CacheSupportedProviders(ctx, client.Resource.ProvidersClient) | |||
There was a problem hiding this comment.
we should use the same approach we do for Cached Locations here?
There was a problem hiding this comment.
Got it, but the location cache is part of the go-azure-helper, I can make another PR for it, but I think it will be a breaking change of go-azure-helper.
There was a problem hiding this comment.
that sounds like the place for it to be, How come it would be a breaking change?
There was a problem hiding this comment.
Hi @katbyte , not sure I followed, do you mean that we should move the resource provider cache logic to go-azure-helper?
There was a problem hiding this comment.
yes? as tom suggested we should follow the same approach we used for location
There was a problem hiding this comment.
The move-cache-to-go-azure-helper has been made here: hashicorp/go-azure-helpers#158. Please help review that and after that appreciate ms-henglu could continue here. @tombuildsstuff @katbyte
ms-henglu
changed the title
|
Hi @tombuildsstuff , Thanks for the code review! I changed the Would you please take another look? thanks! |
ms-henglu
changed the title
…API Version `2022-09-01` This PR supersedes both #20050 and hashicorp/go-azure-helpers#158 by refactoring this to use a cache and then migrating this across to `hashicorp/go-azure-sdk`, since this needs to be migrated away from using `Azure/azure-sdk-for-go` anyway.
|
hey @ms-henglu Apologies for the delayed reply to this - it's taken a little time to review the PR's on I spent some time digging into this one last week, and whilst this PR looks good - in retrospect it appears that we're actually going to need to move this logic from Whilst having this located within Because this logic needs to be migrated across to using As such whilst I'd like to thank you for this contribution, I'm going to close this PR in favour of #21695 which implements this functionality using Thanks! |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
When running azurerm provider initialize, there're two
GET https://management.azure.com/subscriptions/{subscription_id}/providers?api-version=2016-02-01calls, which are very time consuming, each can take about 7 seconds.The two API calls are used to ensure resource providers are registered and enhance resource provider's name validation.
This PR cached the results of the first request, so even with enabling the above 2 features, there'll only be one API request.
After:
Run terraform plan command with service principal authentication
There's only one
/providersAPI callBefore:
Run terraform plan command with service principal authentication
There're two
/providersAPI call