resource/aws_api_gateway_rest_api: Allowed for binary media types update #1600
Conversation
Ninir
added
the
enhancement
Ninir
force-pushed
the
f-apigw-binarymediatype-update
branch
from
95551a0 to
ee02249
Compare
| // Remove every binary media types. Simpler to remove and add new ones, | ||
| // since there are no replacings. | ||
| for _, v := range old { | ||
| m := v.(string) |
There was a problem hiding this comment.
Nitpick, but I'd call this variable mt (assuming it means "media type").
| m := v.(string) | ||
| operations = append(operations, &apigateway.PatchOperation{ | ||
| Op: aws.String("remove"), | ||
| Path: aws.String(fmt.Sprintf("/%s/%s", prefix, strings.Replace(m, "/", "~1", -1))), |
There was a problem hiding this comment.
Do you think it's worth decoupling the escaping logic into a separate function, e.g. to structures.go? I did something very similar in the K8S provider:
hashicorp/terraform-provider-kubernetes@c2d73ca
| }) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Computing the difference, instead of removing all old ones & adding all new ones would be a bit more efficient, but the user probably won't ever have too long list of types, so it probably doesn't matter too much - so feel free to ignore me.
thinking out laud 🤔
|
Just pushed changes: If it's ok for you, feel free to merge, or comment and I'll do 😄 Thanks Radek! |
…update resource/aws_api_gateway_rest_api: Allowed for binary media types update
|
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. Thanks! |
ghost
locked and limited conversation to collaborators
Fixes #1594
Tests