New Datasource: aws_network_interface #2316
Conversation
| page_title: "AWS: aws_network_interface" | ||
| sidebar_current: "docs-aws-datasource-network-interface" | ||
| description: |- | ||
| Get information on anNetwork Interface resource. |
There was a problem hiding this comment.
Hey @atsushi-ishibashi
thanks for the PR.
I left you some comments there, the most important ones about schema (List vs Set). Let me know what you think. The code looks otherwise good and tests are passing.
| Required: true, | ||
| }, | ||
| "association": &schema.Schema{ | ||
| Type: schema.TypeSet, |
There was a problem hiding this comment.
Since there's only ever going to be a single association I think this field should be TypeList so it's possible to reference it like data.aws_network_interface.name.association.0.public_ip etc.
| }, | ||
| }, | ||
| "attachment": &schema.Schema{ | ||
| Type: schema.TypeSet, |
There was a problem hiding this comment.
Since there's only ever going to be a single attachment I think this field should be TypeList so it's possible to reference it like data.aws_network_interface.name.attachment.0.instance_id etc.
| Computed: true, | ||
| }, | ||
| "private_ips": &schema.Schema{ | ||
| Type: schema.TypeSet, |
There was a problem hiding this comment.
Any reason to keep this list of IPs Set? It looks like a field people would like to reference and Set would prevent them in doing so due to computed index. That said I'm not sure if the ordering of IPs has any meaning - i.e. if they're sorted by networking interfaces (primary first, then others)?
There was a problem hiding this comment.
I used Set because I thought the ordering has no meaning.
I agree with your suggestion👍
| return fmt.Errorf("no matching network interface found") | ||
| } | ||
| if len(resp.NetworkInterfaces) > 1 { | ||
| return fmt.Errorf("multiple network interfaces matched") |
There was a problem hiding this comment.
Do you mind improving the error message here to also include the ID we looked for? e.g. multiple network interfaces matched %s", d.Id()
radeksimko
added
the
waiting-response
|
radeksimko
removed
the
waiting-response
* New Datasource: aws_network_interface * Reflect reviews
|
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
#2306