Support for additional arguments on azurerm_api_management_api_diagnostic #9172
Conversation
sirlatrom
force-pushed
the
fix-9169
branch
8 times, most recently
from
9fda0c3 to
4807ec2
Compare
Signed-off-by: Sune Keller <absukl@almbrand.dk>
Signed-off-by: Sune Keller <absukl@almbrand.dk>
Signed-off-by: Sune Keller <absukl@almbrand.dk>
|
Pinging @jackofallops as you were the reviewer on the original resource PR. The original PR only implemented the required arguments. This PR adds the other arguments configurable through the Portal and ARM API. |
There was a problem hiding this comment.
Hi @sirlatrom, thanks for this. Just a couple small suggestions, if you could answer those then this LGTM! :)
azurerm/internal/services/apimanagement/api_management_api_diagnostic_resource.go
Outdated
Show resolved
Hide resolved
Co-authored-by: Tom Bamford <tom@bamford.io>
…gnostic_resource.go Co-authored-by: Tom Bamford <tom@bamford.io>
|
Oops, I should learn to stop coding directly on GitHub; extraneous comma at https://github.com/terraform-providers/terraform-provider-azurerm/pull/9172/files#diff-c5799762db005774a01a8b87d865af5879b7e92cf5ed980bb2f60c8718976214R326 |
Signed-off-by: Sune Keller <absukl@almbrand.dk>
@manicminer That's taken care of now :) |
azurerm/internal/services/apimanagement/api_management_api_diagnostic_resource.go
Outdated
Show resolved
Hide resolved
…gnostic_resource.go Co-authored-by: Tom Bamford <tom@bamford.io>
| if frontendRequest, ok := d.GetOk("frontend_request"); ok { | ||
| parameters.Frontend.Request = expandApiManagementApiDiagnosticHTTPMessageDiagnostic(frontendRequest.([]interface{})) | ||
| } | ||
|
|
||
| if frontendResponse, ok := d.GetOk("frontend_response"); ok { | ||
| parameters.Frontend.Response = expandApiManagementApiDiagnosticHTTPMessageDiagnostic(frontendResponse.([]interface{})) | ||
| } | ||
|
|
||
| if backendRequest, ok := d.GetOk("backend_request"); ok { | ||
| parameters.Backend.Request = expandApiManagementApiDiagnosticHTTPMessageDiagnostic(backendRequest.([]interface{})) | ||
| } | ||
|
|
||
| if backendResponse, ok := d.GetOk("backend_response"); ok { | ||
| parameters.Backend.Response = expandApiManagementApiDiagnosticHTTPMessageDiagnostic(backendResponse.([]interface{})) | ||
| } | ||
|
|
There was a problem hiding this comment.
@sirlatrom Getting a crash here, looks like parameters.Frontend / parameters.Backend are nil
There was a problem hiding this comment.
Thanks, I'll look into it and ensure they are initialized correctly.
There was a problem hiding this comment.
It should be handled better now in 3dbe558.
Signed-off-by: Sune Keller <absukl@almbrand.dk>
|
@sirlatrom This is unfortunately blocked by Azure/azure-sdk-for-go#13604 |
We can either await its resolution or I can extract the sampling part into another PR? |
|
@sirlatrom Sure, that'll work, you can remove that attribute for now. Thanks! |
See hashicorp#9172 (comment) and Azure/azure-sdk-for-go#13604. Signed-off-by: Sune Keller <absukl@almbrand.dk>
Done! |
See hashicorp#9172 (comment) and Azure/azure-sdk-for-go#13604. Signed-off-by: Sune Keller <absukl@almbrand.dk>
@manicminer I created #9321 to follow up on this when the SDK issue is resolved. |
|
Test results |
|
This has been released in version 2.37.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example: provider "azurerm" {
version = "~> 2.37.0"
}
# ... other configuration ... |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks! |
ghost
locked as resolved and limited conversation to collaborators
Closes #9169.